Binary caching: remove symlinks, copy files instead #9747
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@scheibelp This looks like what we talked about and makes sense to me. I suppose if I want to try out this change, I no longer want #9733, is that correct? 👍 |
Yes (strictly speaking it doesn't conflict with this, but it isn't necessary if you are using this) |
|
Absolute symlinks are a pain. Can they be converted to relative symlinks? Are the links to file in the install tree? |
|
You could try something like this https://github.com/gartung/cmssw-spack/blob/master/common/scrampackage.py#L16 |
|
@scheibelp can you look into the testing errors on this? I just ran into this bug in trying to put together the tutorial materials, so I'll put a priority on reviewing it. |
|
@becker33 it is ready now - the build failure was a timeout in one of the build tests. I restarted it and it has now completed. |
They could and that could be a better option longer-term.
I'm not sure what you mean by this. If you mean "are the links pointing to files that exist in the install prefix" then yes (at least in the case of |
becker33
pushed a commit
that referenced
this pull request
Nov 10, 2018
This reverts commit 058cf81.
tgamblin
pushed a commit
that referenced
this pull request
Nov 12, 2018
This reverts commit 058cf81.
tgamblin
pushed a commit
that referenced
this pull request
Nov 12, 2018
This reverts commit 058cf81.
tgamblin
pushed a commit
that referenced
this pull request
Nov 12, 2018
This reverts commit 058cf81.
tgamblin
pushed a commit
that referenced
this pull request
Nov 13, 2018
This reverts commit 058cf81.
pramodk
reviewed
Dec 8, 2018
| install_tree(spec.prefix, workdir, symlinks=True) | ||
| # set symlinks=False here to avoid broken symlinks when archiving and | ||
| # moving the package | ||
| install_tree(spec.prefix, workdir, symlinks=False) |
There was a problem hiding this comment.
In the context of #10062 (comment), I wonder why symlinks=False is required.
There was a problem hiding this comment.
Some of the links in the bzip2 package used absolute paths. When deploying the buildcache, link creation would fail because the target paths were missing. This was a temporary solution. The correct solution would be to make the links relative when creating the buildcache.
There was a problem hiding this comment.
I don't think above is solving bzip2 issue with buildcache. I had trouble yesterday while testing buildcache. (bzip2 binary linked to libbz2.so is installed without rpath)
The correct solution would be to make the links relative when creating the buildcache
agree.
scheibelp
pushed a commit
that referenced
this pull request
Jan 11, 2019
Binary caches of packages with absolute symlinks had broken symlinks. As a stopgap measure, #9747 addressed this by replacing symlinks with copies of files when creating binary cached packages. This reverts #9747 and instead, either relative-izes the symlink or rewrites the target. If the binary cache is created using '--rel' (as in "spack buildcache create --rel...") then absolute symlinks will be replaced with relative symlinks (in addition to making RPATHs relative as before); otherwise they are rewritten (when the binary cache is unpacked and installed).
likask
added a commit
to likask/spack
that referenced
this pull request
Apr 18, 2019
* master_upstream: (518 commits) bugfix: bring in .travis.yml from develop version bump: v0.12.1 tutorial basics section: fix gcc install version bugfix: add back r's for invalid regexes Make Spack relocate text files in build caches with relative binaries Revert "Binary caching: remove symlinks, copy files instead (spack#9747)" Fix typos in Build Systems Tutorial (spack#9822) docs: update tutorial slide link for SC18 version bump: v0.12.0 docs: add environments tutorial Updates to Configuration Tutorial for SC18 (spack#9811) Consistent capitalization of Build Caches docs for SC18 (spack#9810) Minor changes to Build Settings docs (spack#9808) Minor changes to Basic Settings docs for SC18 (spack#9809) Update basic usage and modules tutorial (spack#9807) Advanced packaging tutorial: reorganize for binary caches (spack#9804) env: uninstall just removes specs that are still needed by other envs bugfix: preserve abstract specs when installing an environment env: fix display of concretized specs in `spack find` env: make `spack config edit` and `spack config get` environment-aware ... # Conflicts: # var/spack/repos/builtin/packages/diffutils/package.py
adamjstewart
added a commit
that referenced
this pull request
Jan 9, 2020
* fixes #967 * Version bump to 0.9.1 - Bugfixes for spack find - 0.9.1 can read specs from current develop. * Don't assume spack is in the path when building docs. * Quick fix for relocation issues. * elf relocation fix: cherry-picked from develop branch (#6889) * Revert "Quick fix for relocation issues." This reverts commit 57608a6. * Buildcache: relocate fixes (#6512) * Updated function which checks if a binary file needs relocation. Previously this was incorrectly identifying ELF binaries as symbolic links (so they were being excluded from relocation). Added test to check that ELF binaries are not considered symlinks. * relocate_text was not replacing paths in text files. Added test to check that text files are relocated properly (i.e. paths in the file are converted to the new prefix). * Exclude backup files created by filter_file when installing from binary cache. * Update write_buildinfo_file method signature to distinguish between the spec prefix and the working directory for the binary cache package. * Final changes for v0.11.0 (#6318) * Fix logo link in README.md to point to the develop branch. (#6969) * Compiler flag handlers (#6415) This adds the ability for packages to apply compiler flags in one of three ways: by injecting them into the compiler wrapper calls (the default in this PR and previously the only automated choice); exporting environment variable definitions for variables with corresponding names (e.g. CPPFLAGS=...); providing them as arguments to the build system (e.g. configure). When applying compiler flags using build system arguments, a package must implement the 'flags_to_build_system_args" function. This is provided for CMake and autotools packages, so for packages which subclass those build systems, they need only update their flag handler method specify which compiler flags should be specified as arguments to the build system. Convenience methods are provided to specify that all flags be applied in one of the 3 available ways, so a custom implementation is only required if more than one method of applying compiler flags is needed. This also removes redundant build system definitions from tutorial examples * Fix type issues with setting flag handlers (#6960) The flag_handlers method was being set as a bound method, but when reset in the package.py file it was being set as an unbound method (all python2 issues). This gets the underlying function information, which is the same in either case. The bug was uncovered for parmetis in #6858. This is a partial fix. Included are changes to the parmetis package.py file to make use of flag_handlers. * Bump version to 0.11.1 * Added flags to unit tests + OSX build done once per day (#6988) * Adding flags to codecov reports * OSX builds are triggered once a day * Pull R list_urls from upstream. * travis: removed /usr/local/include/c++ before installing gcc on OSX (#6515) (#7027) "brew install gcc" fails for travis build because of an existing /usr/local/include/c++. This commit removes the offending file as suggested by brew. * Fix gfortran 7 detection (#7017) * Add NameError to exceptions caught from configure_args in module generation (#7173) * Revert "Binary caching: remove symlinks, copy files instead (#9747)" This reverts commit 058cf81. * Make Spack relocate text files in build caches with relative binaries * add the tfel package * fix the tfel package * fix the tfel package * fix the tfel package * Taking Adam J. Steward' remarks into account * fixes trailing white spaces * Update description * Update dependencies following @adamjstewart adices * Style fixes * Style fixes * Add java optional support * add the maintainers attribute (following @alalazo advice), disable interface not selected (following @adamjstewart advice) * flake8 fixes * Fix Cast3M and python-bindings support. Python detection is made compatible with cmake'FindPythonLibs module (at least how it is used in TFEL) * Style fixes * Style fixes * Fix test on python version * Follow @adamjstewart advices: code is much cleaner and readable * Small fix * Small fix * Add comment * Small fix in cmake option * try again (trying to overcome Travis CI unstable build process) * Add support for the MFrontGenericInterfaceSupport project (MGIS) * Style fixes * Package documentation update * Package documentation update * Fix a typo thanks to Andreas Baumbach review * Follow Adam J. Stewart advices * Fix type * bugfix: add back r's for invalid regexes * tutorial basics section: fix gcc install version * version bump: v0.12.1 * bugfix: bring in .travis.yml from develop * Add new TFEL' versions (3.0.4, 3.1.4 and 3.2.1). Add new MGIS version (1.0.1). Fix MGIS dependency * merge with spack:develop * add missing dependency * new versions of and * Fix MGIS url. Fix duplicate variant in TFEL * Fix tfel packaging according to Adam J. Stewart' advices * Fix flake8 warning Co-authored-by: Massimiliano Culpo <massimiliano.culpo@gmail.com> Co-authored-by: Todd Gamblin <tgamblin@llnl.gov> Co-authored-by: Peter Scheibel <scheibel1@llnl.gov> Co-authored-by: Greg Becker <becker33@llnl.gov> Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>
JBlaschke
pushed a commit
to JBlaschke/spack
that referenced
this pull request
Jun 1, 2020
…)" This reverts commit 058cf81.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@scottwittenburg @gartung
@becker33 this will fix some broken symlinks for bzip2, for example. However, it may not be essential, as from what I can tell most packages don't use the extra symlinks (e.g.
bzegrep) that are broken.Binary caches of packages with absolute symlinks had broken symlinks. From what I can tell, tar doesn't support any notion of matching source/destination roots when unpacking an archive with absolute symlinks. Therefore, this commit just makes a copy of any file that is a symlink while creating a binary cache of a package.