-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bugfix: ensure proper dependency handling for package-only installs #15197
bugfix: ensure proper dependency handling for package-only installs #15197
Conversation
@scottwittenburg Could you try this fix and let me know if it resolves your issue? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tldahlgren: this looks great, but we need to get the coverage up -- there are some key pieces of the code still not covered by tests.
Thanks for the confirmation that you're fine with this approach.
I agree. I was half expecting the patch coverage numbers but must've missed them when I selected reviewers. As for increasing coverage of some key pieces, it would be easier with refactoring (e.g., |
This PR restores functionality for our pipeline @tldahlgren @tgamblin @scottwittenburg |
BUT, I am still seeing the same issue using
Output (truncated to only show final error):
The KEY is that libiconv IS INSTALLED, despite the above error message:
|
Thanks for clearing that up. I plan to work on this after my morning meetings. |
@eugeneswalker Not sure what is going on. I followed the directions above but get the same failure, which appears to be tied to curl refusing connections (code 7). This happens whether I use the
Using |
Thanks for your help setting up the environment to reproduce the errors you were seeing. I created a separate issue, #15219 , for it. I was able to determine that the |
In conjunction with PR #15220, the pipeline seems to be working now! Thank you! |
@tgamblin Can this be merged now? @eugeneswalker mentioned in slack that this was also needed to resolve his issues with the distributed build |
See #15237 for work on additional |
…15197) The distributed build PR (#13100) -- did not check the install status of dependencies when using the `--only package` option so would refuse to install a package with the claim that it had uninstalled dependencies whether that was the case or not. - [x] add install status checks for the `--only package` case. - [x] add initial set of tests
…upstream_master * commit 'e2b1737a42c9c0c796671f9dd0c39f623e4c91c0': (1343 commits) update CHANGELOG.md for 0.14.1 version bump: 0.14.1 multiprocessing: allow Spack to run uninterrupted in background (spack#14682) Cray bugfix: TERM missing while reading default target (spack#15381) Upstreams: don't write metadata directory to upstream DB (spack#15526) Creating versions from urls doesn't modify class attributes (spack#15452) bugfix: fix install_missing_compilers option bug from v0.14.0 (spack#15416) bugfix: installer.py shouldn't be executable (spack#15386) Add function replace_prefix_nullterm for use on mach-o rpaths. (spack#15347) ArchSpec: fix semantics of satisfies when not concrete and strict is true (spack#15319) suite-sparse: fix installation for v5.X (spack#15326) testing: increase installer coverage (spack#15237) bugfix: resolve undefined source_pkg_dir failure (spack#15339) Bugfix: resolve StopIteration message attribute failure (spack#15341) Recover coverage from subprocesses during unit tests (spack#15354) Correct pytest.raises matches to match (spack#15346) bugfix: Add dependents when initializing spec from yaml (spack#15220) Uniquify suffixes added to module names (spack#14920) bugfix: ensure proper dependency handling for package-only installs (spack#15197) Fix for being able to 'spack load' packages that have been renamed. (spack#14348) ... # Conflicts: # .travis.yml # lib/spack/spack/modules/common.py # var/spack/repos/builtin/packages/mofem-cephas/package.py # var/spack/repos/builtin/packages/mofem-fracture-module/package.py # var/spack/repos/builtin/packages/mofem-users-modules/package.py # var/spack/repos/builtin/packages/python/package.py
Fixes #15196
The recent distributed build PR -- #13100 -- did not check the install status of dependencies when using the
--only package
option so would refuse to install a package with the claim that it had uninstalled dependencies whether that was the case or not.This PR adds install status checks for the
--only package
case.