This fixes a bug in creating rpaths relative to $ORIGIN on linux. (#5726)

* This fixes a bug in creating rpaths relative to  on linux.

* fix for macOS as well

* found in testing

* flake8

* fix testing on macOS

* flake8

3 files changed, 34 insertions, 29 deletions

diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py
index f915e3a0d0..2e30cdcae7 100644
--- a/lib/spack/spack/binary_distribution.py
+++ b/lib/spack/spack/binary_distribution.py
@@ -240,24 +240,24 @@ def build_tarball(spec, outdir, force=False, rel=False, yes_to_all=False,
         else:
             raise NoOverwriteException(str(specfile_path))
     # make a copy of the install directory to work with
-    prefix = join_path(outdir, os.path.basename(spec.prefix))
-    if os.path.exists(prefix):
-        shutil.rmtree(prefix)
-    install_tree(spec.prefix, prefix)
+    workdir = join_path(outdir, os.path.basename(spec.prefix))
+    if os.path.exists(workdir):
+        shutil.rmtree(workdir)
+    install_tree(spec.prefix, workdir)
 
     # create info for later relocation and create tar
-    write_buildinfo_file(prefix)
+    write_buildinfo_file(workdir)
 
     # optinally make the paths in the binaries relative to each other
     # in the spack install tree before creating tarball
     if rel:
-        make_package_relative(prefix)
+        make_package_relative(workdir, spec.prefix)
     # create compressed tarball of the install prefix
     with closing(tarfile.open(tarfile_path, 'w:gz')) as tar:
-        tar.add(name='%s' % prefix,
-                arcname='%s' % os.path.basename(prefix))
+        tar.add(name='%s' % workdir,
+                arcname='%s' % os.path.basename(workdir))
     # remove copy of install directory
-    shutil.rmtree(prefix)
+    shutil.rmtree(workdir)
 
     # get the sha256 checksum of the tarball
     checksum = checksum_tarball(tarfile_path)
@@ -327,15 +327,16 @@ def download_tarball(spec):
     return None
 
 
-def make_package_relative(prefix):
+def make_package_relative(workdir, prefix):
     """
     Change paths in binaries to relative paths
     """
-    buildinfo = read_buildinfo_file(prefix)
+    buildinfo = read_buildinfo_file(workdir)
     old_path = buildinfo['buildpath']
     for filename in buildinfo['relocate_binaries']:
-        path_name = os.path.join(prefix, filename)
-        relocate.make_binary_relative(path_name, old_path)
+        orig_path_name = os.path.join(prefix, filename)
+        cur_path_name = os.path.join(workdir, filename)
+        relocate.make_binary_relative(cur_path_name, orig_path_name, old_path)
 
 
 def relocate_package(prefix):
diff --git a/lib/spack/spack/relocate.py b/lib/spack/spack/relocate.py
index 999dc23a02..41c979fae3 100644
--- a/lib/spack/spack/relocate.py
+++ b/lib/spack/spack/relocate.py
@@ -158,7 +158,8 @@ def macho_replace_paths(old_dir, new_dir, rpaths, deps, idpath):
     return nrpaths, ndeps, id
 
 
-def modify_macho_object(path_name, old_dir, new_dir, relative):
+def modify_macho_object(cur_path_name, orig_path_name, old_dir, new_dir,
+                        relative):
     """
     Modify MachO binary path_name by replacing old_dir with new_dir
     or the relative path to spack install root.
@@ -170,14 +171,14 @@ def modify_macho_object(path_name, old_dir, new_dir, relative):
     install_name_tool  -rpath old new binary
     """
     # avoid error message for libgcc_s
-    if 'libgcc_' in path_name:
+    if 'libgcc_' in cur_path_name:
         return
-    rpaths, deps, idpath = macho_get_paths(path_name)
+    rpaths, deps, idpath = macho_get_paths(cur_path_name)
     id = None
     nrpaths = []
     ndeps = []
     if relative:
-        nrpaths, ndeps, id = macho_make_paths_relative(path_name,
+        nrpaths, ndeps, id = macho_make_paths_relative(orig_path_name,
                                                        old_dir, rpaths,
                                                        deps, idpath)
     else:
@@ -185,13 +186,13 @@ def modify_macho_object(path_name, old_dir, new_dir, relative):
                                                  deps, idpath)
     install_name_tool = Executable('install_name_tool')
     if id:
-        install_name_tool('-id', id, path_name, output=str, err=str)
+        install_name_tool('-id', id, cur_path_name, output=str, err=str)
 
     for orig, new in zip(deps, ndeps):
-        install_name_tool('-change', orig, new, path_name)
+        install_name_tool('-change', orig, new, cur_path_name)
 
     for orig, new in zip(rpaths, nrpaths):
-        install_name_tool('-rpath', orig, new, path_name)
+        install_name_tool('-rpath', orig, new, cur_path_name)
     return
 
 
@@ -256,17 +257,18 @@ def relocate_binary(path_name, old_dir, new_dir):
         tty.die("Relocation not implemented for %s" % platform.system())
 
 
-def make_binary_relative(path_name, old_dir):
+def make_binary_relative(cur_path_name, orig_path_name, old_dir):
     """
     Make RPATHs relative to old_dir in given elf or mach-o file path_name
     """
     if platform.system() == 'Darwin':
         new_dir = ''
-        modify_macho_object(path_name, old_dir, new_dir, relative=True)
+        modify_macho_object(cur_path_name, orig_path_name, old_dir, new_dir,
+                            relative=True)
     elif platform.system() == 'Linux':
-        orig_rpaths = get_existing_elf_rpaths(path_name)
-        new_rpaths = get_relative_rpaths(path_name, old_dir, orig_rpaths)
-        modify_elf_object(path_name, orig_rpaths, new_rpaths)
+        orig_rpaths = get_existing_elf_rpaths(cur_path_name)
+        new_rpaths = get_relative_rpaths(orig_path_name, old_dir, orig_rpaths)
+        modify_elf_object(cur_path_name, orig_rpaths, new_rpaths)
     else:
         tty.die("Prelocation not implemented for %s" % platform.system())
 
diff --git a/lib/spack/spack/test/packaging.py b/lib/spack/spack/test/packaging.py
index 2277ec891d..52e66c99d2 100644
--- a/lib/spack/spack/test/packaging.py
+++ b/lib/spack/spack/test/packaging.py
@@ -291,11 +291,13 @@ def test_relocate_macho():
     assert (needs_binary_relocation('Mach-O') is True)
     macho_get_paths('/bin/bash')
     shutil.copyfile('/bin/bash', 'bash')
-    modify_macho_object('bash', '/usr', '/opt', False)
-    modify_macho_object('bash', '/usr', '/opt', True)
+    modify_macho_object('bash', '/bin/bash', '/usr', '/opt', False)
+    modify_macho_object('bash', '/bin/bash', '/usr', '/opt', True)
     shutil.copyfile('/usr/lib/libncurses.5.4.dylib', 'libncurses.5.4.dylib')
-    modify_macho_object('libncurses.5.4.dylib', '/usr', '/opt', False)
-    modify_macho_object('libncurses.5.4.dylib', '/usr', '/opt', True)
+    modify_macho_object('libncurses.5.4.dylib',
+                        '/usr/lib/libncurses.5.4.dylib', '/usr', '/opt', False)
+    modify_macho_object('libncurses.5.4.dylib',
+                        '/usr/lib/libncurses.5.4.dylib', '/usr', '/opt', True)
 
 
 @pytest.mark.skipif(sys.platform != 'linux2',