| Submitter | Enrico Scholz |
|---|---|
| Date | Oct. 13, 2012, 12:12 p.m. |
| Message ID | <1350130322-3100-1-git-send-email-enrico.scholz@sigma-chemnitz.de> |
| Download | mbox | patch |
| Permalink | /patch/38137/ |
| State | New |
| Headers | show |
Comments
On Sat, Oct 13, 2012 at 5:12 AM, Enrico Scholz <enrico.scholz@sigma-chemnitz.de> wrote: > diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass > index 73c4358..30614f0 100644 > --- a/meta/classes/package.bbclass > +++ b/meta/classes/package.bbclass > @@ -1063,14 +1063,23 @@ python populate_packages () { > path = os.path.join(root, f) > rpath = path[len(inst_root):] > pkg_files[pkg].append(rpath) > + > + if not os.path.islink(path): > + continue > + > + target = os.readlink(path) > + if target[0] != '/': > + # make path absolute relative to inst_root > + target = os.path.join(root[len(inst_root):], target) > + > + # make path absolute; do not use os.path.join() here > + # because target might start with multiple '/' > + rtarget = inst_root + target I think you can use the join() which is in the oe python package for this.
Chris Larson <clarson-ZYzTKstmaoRBDgjK7y7TUQ@public.gmane.org> writes: >> + # make path absolute; do not use os.path.join() here >> + # because target might start with multiple '/' >> + rtarget = inst_root + target > > I think you can use the join() no; I used it in an earlier version of the patch rtarget = os.path.join(inst_root, target[1:]) and this fails for links like '//sbin/systemd'. I could play around with mangling 'target' through normpath() or lstrip()'ing the '/'. But concatening the two paths is the best solution because I know that 'target' is absolute and starts with the dir delimiter. Enrico
On Sun, 2012-10-14 at 11:51 +0200, Enrico Scholz wrote: > Chris Larson <clarson-ZYzTKstmaoRBDgjK7y7TUQ@public.gmane.org> writes: > > >> + # make path absolute; do not use os.path.join() here > >> + # because target might start with multiple '/' > >> + rtarget = inst_root + target > > > > I think you can use the join() > > no; I used it in an earlier version of the patch > > rtarget = os.path.join(inst_root, target[1:]) > > and this fails for links like '//sbin/systemd'. You somehow chopped off the important part of Chris's text. What he actually wrote was "... the join() which is in the oe python package", which is not the same as os.path.join. As the comment says, it is... """Like os.path.join but doesn't treat absolute RHS specially""" p.
Phil Blundell <philb-mXXj517/zsQ@public.gmane.org> writes: >> >> + # make path absolute; do not use os.path.join() here >> >> + # because target might start with multiple '/' >> >> + rtarget = inst_root + target >> > >> > I think you can use the join() > ... > You somehow chopped off the important part of Chris's text. What he > actually wrote was "... the join() which is in the oe python package", > which is not the same as os.path.join. > > As the comment says, it is... > > """Like os.path.join but doesn't treat absolute RHS specially""" ah ok... I misread 'oe' and 'os'... It is probably not the best idea, to use such similar names for functions with a so much different semantik. But there are no reasons to use one of these functions. Beside the design error of handling absolute paths specially, the purpose of 'os.path.join' is to concatenate paths with the correct delimiter ('/' vs. '\'). But because RHS is guaranteed to be absolute in this case, they can be joined directly. Enrico
Patch
diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass index 73c4358..30614f0 100644 --- a/meta/classes/package.bbclass +++ b/meta/classes/package.bbclass @@ -1063,14 +1063,23 @@ python populate_packages () { path = os.path.join(root, f) rpath = path[len(inst_root):] pkg_files[pkg].append(rpath) + + if not os.path.islink(path): + continue + + target = os.readlink(path) + if target[0] != '/': + # make path absolute relative to inst_root + target = os.path.join(root[len(inst_root):], target) + + # make path absolute; do not use os.path.join() here + # because target might start with multiple '/' + rtarget = inst_root + target try: - s = os.stat(path) + os.lstat(rtarget) except OSError, (err, strerror): if err != errno.ENOENT: raise - target = os.readlink(path) - if target[0] != '/': - target = os.path.join(root[len(inst_root):], target) dangling_links[pkg].append(os.path.normpath(target)) for pkg in package_list:
The old dangling link detection used os.stat() on the file and registered it as a dangling link when it was not found. This causes problems when the link is an absolute one because it will check files of the host system. E.g. when there is a link .../tmp/work/../etc/cron.root -> /var/spool/cron/root Then * existence of /var/spool/cron/root but not of ../tmp/work/.../var/spool/cron/root will be checked * the build aborts because a 'permission denied' exception is thrown but only 'not found' be catched. E.g. build of systemd on an SELinux enabled host aborts with: | ERROR: Error executing a python function in .../systemd/systemd_git.bb: | OSError: [Errno 13] Permission denied: '.../packages-split/systemd-initramfs/init' | ... | ERROR: 0219: try: | ERROR: *** 0220: s = os.stat(path) | ERROR: 0221: except OSError, (err, strerror): This patch uses os.lstat() to check whether source file is a link, adds it to the installation root direction and checks for the existence of this file then. As this is only a QA mechanism which does not affect the resulting package, rebuilds are not needed Signed-off-by: Enrico Scholz <enrico.scholz@sigma-chemnitz.de> --- meta/classes/package.bbclass | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)