Patchwork package.bbclass: search for dangling links in installation directory

login
register
mail settings
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

Enrico Scholz - Oct. 13, 2012, 12:12 p.m.
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(-)
Chris Larson - Oct. 14, 2012, 1:46 a.m.
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.
Enrico Scholz - Oct. 14, 2012, 9:51 a.m.
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
Phil Blundell - Oct. 14, 2012, 2:58 p.m.
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.
Enrico Scholz - Oct. 29, 2012, 3:20 p.m.
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: