Buildache install fix: replace placeholder and origin install paths in rpath when relocating binaries by gartung · Pull Request #10287 · spack/spack

gartung · 2019-01-09T03:48:42Z

This fixes a problem where the placeholder path was not in the first rpath entry. This was seen in c++ libraries and binaries because the compiler was outside the spack install base path and always appears first in the rpath.

Instead of checking the first rpath entry, all rpaths have the placeholder path and the old install path (if it exists) replaced with the new install path.

The second commit changes the check for old install path strings by using the install prefix instead of the install root.

gartung · 2019-01-09T13:20:45Z

@tgamblin

gartung · 2019-01-09T16:38:10Z

Pending merge of
#10073

…n rpath when relocating binaries

This is similar to `spack spec -I` but highlights which nodes in a DAG are relocatable and which are not. spec.tree has been generalized a little to accept a status function, instead of always showing the install status The current implementation works only for ELF, and needs to be generalized to other platforms.

This test requires a few commands to be present in the environment. Currently it will run only under python 3.7 (which uses Xenial instead of Trusty).

…older-fix

gartung · 2019-02-05T20:19:13Z

@tgamblin

tgamblin

@gartung: this looks good to me, but could you add some more tests for this stuff? The coverage on binary caching is currently kind of low, and I'd like to see some simple tests on mock packages that build dummy libraries and relocate them. Is that feasible? It could be in a separate PR if this needs to go in soon, but it would be really nice to have better regression coverage for the bincache parts of the code.

…er-fix

gartung · 2019-02-08T15:14:52Z

@tgamblin the code coverage report link times out. How can I reproduce this on my machine?

tgamblin · 2019-02-08T17:56:08Z

@gartung: codecov seems to be having issues. I pushed a commit to see whether it'll work if we generate fewer reports (they seem to crash on merging lots of reports for PRs).

If you do want to run this locally, you can do this (from the spack root). You'll need the Python coverage tool installed.

coverage run spack test
coverage combine
coverage html

Might need some options on the above, but that is the gist of it. You should get a directory with a local HTML coverage view if you're successful. Also, if you want to zoom in on one part, you can run, e.g.: coverage run spack test packaging to focus on the biuldcache stuff.

This reverts commit 4f9d703.

…rea.

gartung · 2019-02-15T19:26:52Z

I would like to cherry-pick the changes from #9199 #9440 #9600 #10274 that involve the buildcache command and put them in this PR. I will try to resolve merge conflicts.

gartung · 2019-02-15T19:32:56Z

Close for now so every commit os not tested.

gartung · 2019-02-15T19:38:35Z

Re-open for comments.

…holder-fix

…l or external packages. (spack#229)

Conflicts: lib/spack/spack/relocate.py

…ot check with file_is_relocatable which is much faster.

gartung · 2019-02-28T03:13:19Z

This error was produced when trying to create a buildcache for [email protected] that was linked against system zlib which has an entry for a version 1.2.7 that does not exists in the zlib package.py. The real cause of the error is in spec.full_hash().

  zlib:
    paths:
      [email protected]+optimize+pic+shared arch=linux-rhel7-x86_64: /usr

==> [2019-02-27-20:52:54.622705] Warning: Missing a source id for [email protected]
Traceback (most recent call last):
  File "/build/work/spack-tools/spack/bin/spack", line 48, in <module>
    sys.exit(spack.main.main())
  File "/build/work/spack-tools/spack/lib/spack/spack/main.py", line 675, in main
    return _invoke_command(command, parser, args, unknown)
  File "/build/work/spack-tools/spack/lib/spack/spack/main.py", line 446, in _invoke_command
    return_val = command(parser, args)
  File "/build/work/spack-tools/spack/lib/spack/spack/cmd/buildcache.py", line 528, in buildcache
    args.func(args)
  File "/build/work/spack-tools/spack/lib/spack/spack/cmd/buildcache.py", line 326, in createtarball
    not args.no_rebuild_index)
  File "/build/work/spack-tools/spack/lib/spack/spack/binary_distribution.py", line 363, in build_tarball
    spec_dict['full_hash'] = spec.full_hash()
  File "/build/work/spack-tools/spack/lib/spack/spack/spec.py", line 1306, in full_hash
    package_hash = self.package.content_hash()
  File "/build/work/spack-tools/spack/lib/spack/spack/package.py", line 1092, in content_hash
    for p in self.spec.patches)
  File "/build/work/spack-tools/spack/lib/spack/llnl/util/lang.py", line 183, in __call__
    self.cache[args] = self.func(*args)
  File "/build/work/spack-tools/spack/lib/spack/spack/spec.py", line 2690, in patches
    for sha256 in self.variants['patches']._patches_in_order_of_appearance:
AttributeError: 'MultiValuedVariant' object has no attribute '_patches_in_order_of_appearance'

gartung · 2019-02-28T03:19:49Z

@tgamblin The patch coverage is at 62%. Is this OK to merge?

scheibelp · 2019-02-28T03:32:42Z

Either myself or @tgamblin can look at this by tomorrow

gartung · 2019-02-28T03:35:00Z

Note that I merged in pr #9199

gartung · 2019-02-28T03:37:53Z

With #9199 I am able to drop the whole replace rpaths with placeholders and check for install prefix strings in favor of @alalazo 's file_is_relocatable.

…'_patches_in_order_of_appearance'

…holder-fix

gartung · 2019-02-28T15:59:38Z

I did 'git push origin :branch' instead of 'git push origin +branch'.
I will have to resubmit but first #9199 needs to be merged.

scheibelp · 2019-02-28T18:15:58Z

I will have to resubmit but first #9199 needs to be merged.

OK I will look at that, and check this out later today.

gartung added the buildcache label Jan 9, 2019

gartung mentioned this pull request Jan 9, 2019

Release workflow #10274

Merged

3 tasks

gartung added WIP and removed WIP labels Jan 9, 2019

Buildache install fix: replace placeholder and origin install paths i…

1f9456f

…n rpath when relocating binaries

gartung force-pushed the buildcache-placeholder-fix branch from 28cffb9 to 1f9456f Compare January 11, 2019 23:26

gartung requested review from scheibelp and tgamblin January 14, 2019 15:22

alalazo and others added 6 commits January 15, 2019 17:03

Added a test to check if an executable is relocatable or not

8685d3c

This test requires a few commands to be present in the environment. Currently it will run only under python 3.7 (which uses Xenial instead of Trusty).

Added tests for the 'buildcache preview' command.

2644a44

Fixed codebase after rebase

91c3e1f

Fixed the list of apt addons for Python 3.7 in travis.yaml

255deea

Merge remote-tracking branch 'upstream/develop' into builcache-placeh…

21dd57f

…older-fix

tgamblin requested changes Feb 7, 2019

View reviewed changes

Merge remote-tracking branch 'llnl/develop' into buildcache-placehold…

a754369

…er-fix

gartung removed the request for review from scheibelp February 8, 2019 15:13

temporarily disable coverage for build tests to fix codecov

4f9d703

gartung added 2 commits February 15, 2019 12:49

Revert "temporarily disable coverage for build tests to fix codecov"

a459c55

This reverts commit 4f9d703.

Initial work to get the depency paths in the new spack installation a…

0b9f85b

…rea.

gartung closed this Feb 15, 2019

gartung reopened this Feb 15, 2019

gartung and others added 9 commits February 22, 2019 09:45

Merge remote-tracking branch 'upstream/develop' into buildcache-place…

1da09b8

…holder-fix

Merge branch 'develop' into features/buildcache_preview

ab89c04

Only check ELF executables and shared libraries. Skip checking virtua…

e68672d

…l or external packages. (spack#229)

Fixed flake8 issues

b196861

Add handling for macOS mach binaries

e291c90

Merge branch 'pr9199' into buildcache-placeholder-fix

f5e7a74

Conflicts: lib/spack/spack/relocate.py

Check the install prefix not the install root

649c8ce

Fix relocate test.

0c8f397

Replace placeholder rpath substitution and strings_contains_installro…

3df4aab

…ot check with file_is_relocatable which is much faster.

gartung mentioned this pull request Feb 28, 2019

Feature: buildcache relocation with different layout and compiler location #10522

Closed

gartung requested a review from tgamblin February 28, 2019 03:20

gartung added 3 commits February 28, 2019 09:41

Fix for AttributeError: 'MultiValuedVariant' object has no attribute …

6a4a33d

…'_patches_in_order_of_appearance'

Merge remote-tracking branch 'upstream/develop' into buildcache-place…

9156659

…holder-fix

Remove autopep8's removal of lambda's

e61b651

gartung closed this Feb 28, 2019

gartung deleted the buildcache-placeholder-fix branch February 28, 2019 15:55

gartung restored the buildcache-placeholder-fix branch February 28, 2019 15:56

gartung removed the request for review from tgamblin February 28, 2019 15:59

gartung mentioned this pull request Feb 28, 2019

Buildcache updates: Resubmit of #10287 #10756

Merged

Conversation

gartung commented Jan 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gartung commented Jan 9, 2019

Uh oh!

gartung commented Jan 9, 2019

Uh oh!

gartung commented Feb 5, 2019

Uh oh!

tgamblin left a comment

Choose a reason for hiding this comment

Uh oh!

gartung commented Feb 8, 2019

Uh oh!

tgamblin commented Feb 8, 2019

Uh oh!

gartung commented Feb 15, 2019

Uh oh!

gartung commented Feb 15, 2019

Uh oh!

gartung commented Feb 15, 2019

Uh oh!

gartung commented Feb 28, 2019

Uh oh!

gartung commented Feb 28, 2019

Uh oh!

scheibelp commented Feb 28, 2019

Uh oh!

gartung commented Feb 28, 2019

Uh oh!

gartung commented Feb 28, 2019

Uh oh!

gartung commented Feb 28, 2019

Uh oh!

scheibelp commented Feb 28, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gartung commented Jan 9, 2019 •

edited

Loading