All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Purdie <richard.purdie@linuxfoundation.org>
To: openembedded-core <openembedded-core@lists.openembedded.org>
Subject: [PATCH] chrpath: Improve crazy code
Date: Tue, 26 Nov 2013 22:14:47 +0000	[thread overview]
Message-ID: <1385504087.11246.15.camel@ted> (raw)

The current code is a little bit overcomplicated, deficient and also
possibly broken.

Issues include:

a) Not maximally optisming rpaths (e.g. a lib in usr/lib might get an
   rpath of $ORIGIN/../../usr/lib)
b) The return in the middle of the for loop look suspiciously like
   it might break on some binaries
c) The depth function, loops of "../" prepending and so on can
   be replaced with a call to os.path.relpath

This patch cleans up the above issues.

Running binaries should result in less "../" resolutions which can't
hurt performance either.

[YOCTO #3989]

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
diff --git a/meta/classes/chrpath.bbclass b/meta/classes/chrpath.bbclass
index 61a24b3..7bdb1b9 100644
--- a/meta/classes/chrpath.bbclass
+++ b/meta/classes/chrpath.bbclass
@@ -1,7 +1,7 @@
 CHRPATH_BIN ?= "chrpath"
 PREPROCESS_RELOCATE_DIRS ?= ""
 
-def process_file_linux(cmd, fpath, basedir, tmpdir, d):
+def process_file_linux(cmd, fpath, rootdir, baseprefix, tmpdir, d):
     import subprocess as sub
 
     p = sub.Popen([cmd, '-l', fpath],stdout=sub.PIPE,stderr=sub.PIPE)
@@ -15,35 +15,21 @@ def process_file_linux(cmd, fpath, basedir, tmpdir, d):
     #bb.note("Current rpath for %s is %s" % (fpath, curr_rpath.strip()))
     rpaths = curr_rpath.split(":")
     new_rpaths = []
+    modified = False
     for rpath in rpaths:
         # If rpath is already dynamic copy it to new_rpath and continue
         if rpath.find("$ORIGIN") != -1:
             new_rpaths.append(rpath.strip())
             continue
         rpath =  os.path.normpath(rpath)
-        # If the rpath shares a root with base_prefix determine a new dynamic rpath from the
-        # base_prefix shared root
-        if rpath.find(basedir) != -1:
-            depth = fpath.partition(basedir)[2].count('/')
-            libpath = rpath.partition(basedir)[2].strip()
-        # otherwise (i.e. cross packages) determine a shared root based on the TMPDIR
-        # NOTE: This will not work reliably for cross packages, particularly in the case
-        # where your TMPDIR is a short path (i.e. /usr/poky) as chrpath cannot insert an
-        # rpath longer than that which is already set.
-        elif rpath.find(tmpdir) != -1:
-            depth = fpath.rpartition(tmpdir)[2].count('/')
-            libpath = rpath.partition(tmpdir)[2].strip()
-        else:
+        if baseprefix not in rpath and tmpdir not in rpath:
             new_rpaths.append(rpath.strip())
-            return
-        base = "$ORIGIN"
-        while depth > 1:
-            base += "/.."
-            depth-=1
-        new_rpaths.append("%s%s" % (base, libpath))
+            continue
+        new_rpaths.append("$ORIGIN/" + os.path.relpath(rpath.strip(), os.path.dirname(fpath.replace(rootdir, "/"))))
+        modified = True
 
     # if we have modified some rpaths call chrpath to update the binary
-    if len(new_rpaths):
+    if modified:
         args = ":".join(new_rpaths)
         #bb.note("Setting rpath for %s to %s" %(fpath, args))
         p = sub.Popen([cmd, '-r', args, fpath],stdout=sub.PIPE,stderr=sub.PIPE)
@@ -52,7 +38,7 @@ def process_file_linux(cmd, fpath, basedir, tmpdir, d):
             bb.error("%s: chrpath command failed with exit code %d:\n%s%s" % (d.getVar('PN', True), p.returncode, out, err))
             raise bb.build.FuncFailed
 
-def process_file_darwin(cmd, fpath, basedir, tmpdir, d):
+def process_file_darwin(cmd, fpath, rootdir, baseprefix, tmpdir, d):
     import subprocess as sub
 
     p = sub.Popen([d.expand("${HOST_PREFIX}otool"), '-L', fpath],stdout=sub.PIPE,stderr=sub.PIPE)
@@ -64,26 +50,20 @@ def process_file_darwin(cmd, fpath, basedir, tmpdir, d):
         if "(compatibility" not in l:
             continue
         rpath = l.partition("(compatibility")[0].strip()
-        if rpath.find(basedir) != -1:
-            depth = fpath.partition(basedir)[2].count('/')
-            libpath = rpath.partition(basedir)[2].strip()
-        else:
+        if baseprefix not in rpath:
             continue
 
-        base = "@loader_path"
-        while depth > 1:
-            base += "/.."
-            depth-=1
-        base = base + libpath
-        p = sub.Popen([d.expand("${HOST_PREFIX}install_name_tool"), '-change', rpath, base, fpath],stdout=sub.PIPE,stderr=sub.PIPE)
+        newpath = "@loader_path/" + os.path.relpath(rpath, os.path.dirname(fpath.replace(rootdir, "/")))
+        bb.warn("%s %s %s %s" % (fpath, rpath, newpath, rootdir))
+        p = sub.Popen([d.expand("${HOST_PREFIX}install_name_tool"), '-change', rpath, newpath, fpath],stdout=sub.PIPE,stderr=sub.PIPE)
         err, out = p.communicate()
 
-def process_dir (directory, d):
+def process_dir (rootdir, directory, d):
     import stat
 
     cmd = d.expand('${CHRPATH_BIN}')
     tmpdir = os.path.normpath(d.getVar('TMPDIR'))
-    basedir = os.path.normpath(d.expand('${base_prefix}'))
+    baseprefix = os.path.normpath(d.expand('${base_prefix}'))
     hostos = d.getVar("HOST_OS", True)
 
     #bb.debug("Checking %s for binaries to process" % directory)
@@ -107,7 +87,7 @@ def process_dir (directory, d):
             continue
 
         if os.path.isdir(fpath):
-            process_dir(fpath, d)
+            process_dir(rootdir, fpath, d)
         else:
             #bb.note("Testing %s for relocatability" % fpath)
 
@@ -120,7 +100,7 @@ def process_dir (directory, d):
             else:
                 # Temporarily make the file writeable so we can chrpath it
                 os.chmod(fpath, perms|stat.S_IRWXU)
-            process_file(cmd, fpath, basedir, tmpdir, d)
+            process_file(cmd, fpath, rootdir, baseprefix, tmpdir, d)
                 
             if perms:
                 os.chmod(fpath, perms)
@@ -131,5 +111,5 @@ def rpath_replace (path, d):
     for bindir in bindirs:
         #bb.note ("Processing directory " + bindir)
         directory = path + "/" + bindir
-        process_dir (directory, d)
+        process_dir (path, directory, d)
 




                 reply	other threads:[~2013-11-26 22:15 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1385504087.11246.15.camel@ted \
    --to=richard.purdie@linuxfoundation.org \
    --cc=openembedded-core@lists.openembedded.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.