package.bbclass: search for dangling links in installation directory

Submitted by Enrico Scholz on Oct. 13, 2012, 12:12 p.m.

Details

Message ID 1350130322-3100-1-git-send-email-enrico.scholz@sigma-chemnitz.de
State New
Headers show

Commit Message

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(-)

Patch hide | download patch | download mbox

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:

Comments

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