Install: Add use-buildcache option to install by kwryankrattiger · Pull Request #32537 · spack/spack

kwryankrattiger · 2022-09-06T21:35:23Z

Allow differentiating between top level packages and dependencies when determining whether to install from the cache or not.

@scottwittenburg

Implements this feature request.

kwryankrattiger · 2022-09-07T01:47:22Z

@spackbot fix style

spackbot-app · 2022-09-07T01:47:24Z

Let me see if I can fix that for you!

spackbot-app · 2022-09-07T01:49:13Z

I was able to run spack style --fix for you!

spack style --fix

==> Running style checks on spack
  selected: isort, mypy, black, flake8
==> Modified files
  lib/spack/spack/cmd/install.py
  lib/spack/spack/installer.py
==> Running isort checks
Fixing /tmp/tmpvw9rga1h/spack/lib/spack/spack/cmd/install.py
  isort checks were clean
==> Running mypy checks
Success: no issues found in 558 source files
  mypy checks were clean
==> Running black checks
reformatted lib/spack/spack/cmd/install.py
All done! ✨ 🍰 ✨
1 file reformatted, 1 file left unchanged.
  black checks were clean
==> Running flake8 checks
lib/spack/spack/cmd/install.py:44: [E713] test for membership should be 'not in'
lib/spack/spack/cmd/install.py:47: [E501] line too long (123 > 99 characters)
  flake8 found errors

Keep in mind that I cannot fix your flake8 or mypy errors, so if you have any you'll need to fix them and update the pull request. If I was able to push to your branch, if you make further changes you will need to pull from your updated branch before pushing again.

I've updated the branch with isort fixes.

scottwittenburg · 2022-09-09T23:02:41Z

@kwryankrattiger Can you rebase this on the latest develop and force-push? I'm hoping it will then get past the failing checks we're seeing currently.

scottwittenburg · 2022-09-12T18:24:18Z

@spackbot fix style

spackbot-app · 2022-09-12T18:24:21Z

Let me see if I can fix that for you!

spackbot-app · 2022-09-12T18:26:10Z

I was able to run spack style --fix for you!

spack style --fix

==> Running style checks on spack
  selected: isort, mypy, black, flake8
==> Modified files
  lib/spack/spack/cmd/install.py
  lib/spack/spack/installer.py
  lib/spack/spack/test/cmd/install.py
==> Running isort checks
Fixing /tmp/tmpm_1rhp_4/spack/lib/spack/spack/test/cmd/install.py
  isort checks were clean
==> Running mypy checks
Success: no issues found in 558 source files
  mypy checks were clean
==> Running black checks
reformatted lib/spack/spack/test/cmd/install.py
All done! ✨ 🍰 ✨
1 file reformatted, 2 files left unchanged.
  black checks were clean
==> Running flake8 checks
  flake8 checks were clean
==> spack style checks were clean

Keep in mind that I cannot fix your flake8 or mypy errors, so if you have any you'll need to fix them and update the pull request. If I was able to push to your branch, if you make further changes you will need to pull from your updated branch before pushing again.

I've updated the branch with isort fixes.

lib/spack/spack/cmd/install.py

scottwittenburg

Thanks @kwryankrattiger,! This is looking pretty good, but I left a comment or two to to be addressed, along with a question about how the shorthand is intended to work.

Also, could you provide a brief description of the change? Even just a pointer to the issue this addresses could help other reviewers know what they're looking at

lib/spack/spack/cmd/install.py

scottwittenburg · 2022-09-13T22:26:21Z

I don't mind running the style fix for you, but you should at least pull the changes and incorporate them before you force push again 😜

scottwittenburg · 2022-09-13T22:26:37Z

@spackbot fix style

spackbot-app · 2022-09-13T22:26:40Z

Let me see if I can fix that for you!

spackbot-app · 2022-09-13T22:28:25Z

I was able to run spack style --fix for you!

spack style --fix

==> Running style checks on spack
  selected: isort, mypy, black, flake8
==> Modified files
  lib/spack/spack/cmd/install.py
  lib/spack/spack/installer.py
  lib/spack/spack/test/cmd/install.py
==> Running isort checks
Fixing /tmp/tmpnyy9ct46/spack/lib/spack/spack/test/cmd/install.py
  isort checks were clean
==> Running mypy checks
Success: no issues found in 558 source files
  mypy checks were clean
==> Running black checks
reformatted lib/spack/spack/test/cmd/install.py
All done! ✨ 🍰 ✨
1 file reformatted, 2 files left unchanged.
  black checks were clean
==> Running flake8 checks
  flake8 checks were clean
==> spack style checks were clean

Keep in mind that I cannot fix your flake8 or mypy errors, so if you have any you'll need to fix them and update the pull request. If I was able to push to your branch, if you make further changes you will need to pull from your updated branch before pushing again.

I've updated the branch with isort fixes.

scottwittenburg

This is getting closer, thanks @kwryankrattiger! It's working for my particular use case (must install deps from buildcache, never install package from buildcache), but I have a couple requests regarding the UI.

Also, it looks like some unit tests failed, which this may or may not have caused. Can you check if those failures are reproducible on your end?

lib/spack/spack/cmd/install.py

alalazo · 2022-09-22T09:03:33Z

I have a high-level question. Failed pipeline jobs let you run the build in a docker image to inspect the failure. Within that Docker image there's an install.sh script which runs a single command to install the spec that failed. An example install.sh script is, for instance:

#!/bin/bash


# spack install command
"spack" "-d" "-v" "install" "--show-log-on-error" "--keep-stage" "--no-check-signature" "--cdash-upload-url" "https://cdash.spack.io/submit.php?project=Spack+Testing" "--cdash-build" "[email protected]%[email protected] hash=36ftnpast6yvr3zawwnrcgxn5ao3t5zs arch=linux-ubuntu20.04-x86_64 (E4S OneAPI)" "--cdash-site" "Cloud Gitlab Infrastructure (uo-pegasus)" "--cdash-buildstamp" "20220919-0545-E4S OneAPI" "--no-add" "-f" "/builds/spack/spack/jobs_scratch_dir/reproduction/unifyfs.json"

I assume (let me know if this is a wrong assumption) that something similar to running install.sh happens in Gitlab. If the assumption is correct, why we discarded splitting a single spack install command into:

$ spack install --cache-only --only=dependencies <spec>
$ spack install --no-cache --only=root <spec>

to ensure that dependencies are never installed from sources? Is there any technical issue that I overlooked proceeding that way?

scottwittenburg · 2022-09-22T14:38:05Z

@alalazo We used to split it in two, but it complicated the ci module even more, as we had to make a copy of the spack.yaml before the first install, then use it to replace the version that spack clobbered before the next spack install.

Also, the install.sh script you mentioned in your question, which ends up in the reproducer docker container, is the same one that gitlab runs in the pipeline. Or did I misunderstand the question?

scottwittenburg · 2022-09-22T14:44:34Z

By the way, in my pipelines testing "rebuild everything", this PR is working as expected. @tgamblin was going to provide some insight on a better way, within the installer.py module, to distinguish dependencies from package. But from my perspective, @kwryankrattiger figured that out in a way that seems reasonable to me. So I'd be happy to accept this as-is, once the tests are passing.

alalazo · 2022-09-22T15:11:30Z

We used to split it in two, but it complicated the ci module even more, as we had to make a copy of the spack.yaml before the first install, then use it to replace the version that spack clobbered before the next spack install.

I don't think I get what you're describing above, which might be due to me being unfamiliar with ci.py and cmd/ci.py. Isn't this:

spack/lib/spack/spack/ci.py

Lines 1985 to 2005 in 0ddbb92

    
           def process_command(cmd, cmd_args, repro_dir): 
        
               """ 
        
               Create a script for and run the command. Copy the script to the 
        
               reproducibility directory. 
        
               Arguments: 
        
                   cmd (str): name of the command being processed 
        
                   cmd_args (list): string arguments to pass to the command 
        
                   repro_dir (str): Job reproducibility directory 
        
               Returns: the exit code from processing the command 
        
               """ 
        
               tty.debug("spack {0} arguments: {1}".format(cmd, cmd_args)) 
        
               # Write the command to a shell script 
        
               script = "{0}.sh".format(cmd) 
        
               with open(script, "w") as fd: 
        
                   fd.write("#!/bin/bash\n\n") 
        
                   fd.write("\n# spack {0} command\n".format(cmd)) 
        
                   fd.write(" ".join(['"{0}"'.format(i) for i in cmd_args])) 
        
                   fd.write("\n")

the only part that eventually need to change? Based on the example above wouldn't it suffice to generate something similar to:

#!/bin/bash


# spack install command
"spack" "-d" "-v" "install" "--cache-only" "--only=dependencies" "--no-add" "-f" "/builds/spack/spack/jobs_scratch_dir/reproduction/unifyfs.json"
"spack" "-d" "-v" "install" "--show-log-on-error" "--no-cache" "--only=package" "--keep-stage" "--no-check-signature" "--cdash-upload-url" "https://cdash.spack.io/submit.php?project=Spack+Testing" "--cdash-build" "[email protected]%[email protected] hash=36ftnpast6yvr3zawwnrcgxn5ao3t5zs arch=linux-ubuntu20.04-x86_64 (E4S OneAPI)" "--cdash-site" "Cloud Gitlab Infrastructure (uo-pegasus)" "--cdash-buildstamp" "20220919-0545-E4S OneAPI" "--no-add" "-f" "/builds/spack/spack/jobs_scratch_dir/reproduction/unifyfs.json"

?

scottwittenburg · 2022-09-22T15:20:51Z

That might work, without having to make a backup copy of the environment and restore it between the installs, now that we have the --no-add option. However, I would need to go back and make sure there wasn't some additional problem with the "two spack installs" approach.

Allow differentiating between top level packages and dependencies when determining whether to install from the cache or not.

A requested package is not required to be "explicit"ly requested when specifying --only package or --use-buildcache package:<...> options in install. Add a task property that returns the former.

scottwittenburg

I tested this with the other proposed changes we need for asking spackbot to rebuild everything, and it's working.

haampie

Seems like you're missing

diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py
index 7ec31f0267..1ec21be45d 100644
--- a/lib/spack/spack/installer.py
+++ b/lib/spack/spack/installer.py
@@ -2391,7 +2391,7 @@ def get_deptypes(self, pkg):
         """
         deptypes = ["link", "run"]
         include_build_deps = self.install_args.get("include_build_deps")
-        if not self.install_args.get("cache_only") or include_build_deps:
+        if not self.install_args.get("package_cache_only") or include_build_deps:
             deptypes.append("build")
         if self.run_tests(pkg):
             deptypes.append("test")

an old reference to cache_only, which now causes the installer not to prune redundant build deps.

Fixing an oversight in spack#32537 `get_deptypes` should depend on new `package/dependencies_cache_only` props.

Fixing an oversight in #32537 `get_deptypes` should depend on new `package/dependencies_cache_only` props.

@spackbot

* py-execnet: 1.9.0 (spack#33282) * py-execnet: 1.9.0 * bounds * add version 1.12.3 of parallel-netcdf (spack#33286) * Add checksum for py-psutil 5.9.2 (spack#33139) * gitlab ci: Print better information about broken specs (spack#33124) When a pipeline generation job is automatically failed because it generated jobs for specs known to be broken on develop, print better information about the broken specs that were encountered. Include at a minimum the hash and the url of the job whose failure caused it to be put on the broken specs list in the first place. * meson: remove slash in path (spack#33292) * Add checksum for py-gitpython 3.1.27 (spack#33285) * Add checksum for py-gitpython 3.1.27 * Update package.py * Update var/spack/repos/builtin/packages/py-gitpython/package.py Co-authored-by: Adam J. Stewart <[email protected]> Co-authored-by: Adam J. Stewart <[email protected]> * UPC++/GASNet-EX 2022.9.0 update (spack#33277) * gasnet: Add new release hash * upcxx: Add new release hash * gasnet: misc updates * upcxx: misc updates * Fix [email protected] sha (spack#33307) * [email protected] onwards: set prefix properly (spack#33257) * hip-set-prefix-rocm5.2.0-onwards * Update var/spack/repos/builtin/packages/hip/package.py Update description Co-authored-by: Satish Balay <[email protected]> * petsc,py-petsc4py,slepc,py-slepc4py: add version 3.18.0 (spack#32938) * petsc,py-petsc4py,slepc,py-slepc4py: add version 3.18.0 * workaround for dealii build failure [with petsc version check] * pism: add compatibility fix to for [email protected] * add in hipsolver dependency * py-libensemble: updating package for v0.9.3 (spack#33298) * commit updating py-libensemble package for 0.9.3 * removed commented-out lines * Add checksum for py-oauthlib 3.2.1 (spack#33201) * Add checksum for py-oauthlib 3.2.1 * Update package.py * [@spackbot] updating style on behalf of iarspider * Update package.py * Update package.py Co-authored-by: iarspider <[email protected]> * ninja: New version 1.11.1 (spack#33215) * seacas: update to latest release (spack#33330) Add checksum for latest tag/release * gptl: new version 8.1.1; use the correct mpi fortran compiler (spack#33235) * gptl: new version 8.1.1; use the correct `mpifc` * add `F90` and `$F77` * glib: add 2.74.0 and 2.72.4 (spack#33332) * depfile: update docs (spack#33279) * rocksdb: add 7.7.3 (spack#33341) * Add checksum for py-seaborn 0.12.0 (spack#33145) * Add checksum for py-seaborn 0.12.0 * Update package.py * Update package.py * [@spackbot] updating style on behalf of iarspider * Update package.py Co-authored-by: iarspider <[email protected]> * CI: allow multiple matches to combine tags (spack#32290) Currently "spack ci generate" chooses the first matching entry in gitlab-ci:mappings to fill attributes for a generated build-job, requiring that the entire configuration matrix is listed out explicitly. This unfortunately causes significant problems in environments with large configuration spaces, for example the environment in spack#31598 (spack.yaml) supports 5 operating systems, 3 architectures and 130 packages with explicit size requirements, resulting in 1300 lines of configuration YAML. This patch adds a configuraiton option to the gitlab-ci schema called "match_behavior"; when it is set to "merge", all matching entries are applied in order to the final build-job, allowing a few entries to cover an entire matrix of configurations. The default for "match_behavior" is "first", which behaves as before this commit (only the runner attributes of the first match are used). In addition, match entries may now include a "remove-attributes" configuration, which allows matches to remove tags that have been aggregated by prior matches. This only makes sense to use with "match_behavior:merge". You can combine "runner-attributes" with "remove-attributes" to effectively override prior tags. * meson: update OneAPI compiler support patch (spack#33293) * py-tensorflow: fix zlib (spack#33349) * py-tensorflow: fix zlib * [@spackbot] updating style on behalf of haampie Co-authored-by: haampie <[email protected]> * py-meson-python: add new versions (spack#33294) * tasmanian: disable openmp by default (spack#33345) * octopus: upgrade to 12.1 (spack#33343) * py-sphinx: add v5.3 and v5.2 (spack#33356) * py-setuptools: add v65.5.0 (spack#33353) * libblastrampoline: Add versions 5.1.1, 5.2.0 (spack#33352) * nextflow: add v20.10.0 (spack#33354) * sdl2: add v2.0.22 and v2.24.1 (spack#33351) * mariadb-c-client: add 3.3.2, 3.2.7, 3.1.18, 3.0.10 (spack#33335) * py-tensorflow-hub: zlib, again. (spack#33359) * Add checksum for py-sniffio 1.3.0 (spack#32975) * py-numpy: add v1.23.4 (spack#33260) * py-jupyterlab-pygments: install from wheel to avoid cyclic dependency (spack#33278) * py-jupyterlab-pygments: avoid cyclic dependency * Fix style * Update package.py * Update package.py * [@spackbot] updating style on behalf of iarspider * Update package.py * Flake-8 * fix Co-authored-by: iarspider <[email protected]> * installer.py: show timers for binary install (spack#33305) Print a message of the form ``` Fetch mm:ss. Build: mm:ss. Total: mm:ss ``` when installing from buildcache. Previously this only happened for source builds. * Add checksum for py-astroid 2.12.7, py-astroid 2.12.10, py-setuptools 62.6.0, py-wrapt 1.14.1, py-pylint 2.15.0 (spack#32976) * Add checksum for py-astroid 2.12.7, py-setuptools 62.6.0 * Also add checksum for py-wrapt * Update package.py * Update package.py (spack#57) * Update package.py * Update package.py * Update package.py * gitlab ci: Do not force protected build jobs to run on aws runners (spack#33314) * installer.py: fix/test get_deptypes (spack#33363) Fixing an oversight in spack#32537 `get_deptypes` should depend on new `package/dependencies_cache_only` props. * Add checksum for py-grpcio-tools 1.48.1 (spack#33358) * Add checksum for py-prompt-toolkit 3.0.31 (spack#33362) Co-authored-by: Harmen Stoppels <[email protected]> Co-authored-by: Jim Edwards <[email protected]> Co-authored-by: iarspider <[email protected]> Co-authored-by: Scott Wittenburg <[email protected]> Co-authored-by: Adam J. Stewart <[email protected]> Co-authored-by: Dan Bonachea <[email protected]> Co-authored-by: Auriane R <[email protected]> Co-authored-by: eugeneswalker <[email protected]> Co-authored-by: Satish Balay <[email protected]> Co-authored-by: John-Luke Navarro <[email protected]> Co-authored-by: iarspider <[email protected]> Co-authored-by: Erik Schnetter <[email protected]> Co-authored-by: Greg Sjaardema <[email protected]> Co-authored-by: WuK <[email protected]> Co-authored-by: Michael Kuhn <[email protected]> Co-authored-by: Jonathon Anderson <[email protected]> Co-authored-by: haampie <[email protected]> Co-authored-by: Miroslav Stoyanov <[email protected]> Co-authored-by: Hans Fangohr <[email protected]> Co-authored-by: Mosè Giordano <[email protected]> Co-authored-by: Diego Alvarez <[email protected]> Co-authored-by: Wouter Deconinck <[email protected]>

spackbot-app bot added commands core PR affects Spack core functionality labels Sep 6, 2022

kwryankrattiger force-pushed the install_use_buildcache branch 2 times, most recently from 5599f4d to f096067 Compare September 8, 2022 15:53

spackbot-app bot added the tests General test capability(ies) label Sep 9, 2022

kwryankrattiger force-pushed the install_use_buildcache branch from 9c09200 to 78d6df4 Compare September 9, 2022 21:38

kwryankrattiger force-pushed the install_use_buildcache branch from 78d6df4 to 3bc2519 Compare September 12, 2022 17:41

kwryankrattiger commented Sep 12, 2022

View reviewed changes

lib/spack/spack/cmd/install.py Outdated Show resolved Hide resolved

kwryankrattiger force-pushed the install_use_buildcache branch from d839e40 to 4851a7f Compare September 12, 2022 19:02

scottwittenburg requested changes Sep 12, 2022

View reviewed changes

lib/spack/spack/cmd/install.py Outdated Show resolved Hide resolved

lib/spack/spack/cmd/install.py Outdated Show resolved Hide resolved

lib/spack/spack/cmd/install.py Outdated Show resolved Hide resolved

alalazo assigned scottwittenburg Sep 13, 2022

kwryankrattiger force-pushed the install_use_buildcache branch 2 times, most recently from 4ac37c4 to 54634f4 Compare September 14, 2022 21:24

scottwittenburg requested changes Sep 14, 2022

View reviewed changes

lib/spack/spack/cmd/install.py Outdated Show resolved Hide resolved

lib/spack/spack/cmd/install.py Outdated Show resolved Hide resolved

lib/spack/spack/cmd/install.py Outdated Show resolved Hide resolved

kwryankrattiger force-pushed the install_use_buildcache branch 5 times, most recently from 32b6923 to 40a62fe Compare September 15, 2022 01:19

kwryankrattiger added 9 commits September 23, 2022 10:17

Install: Add use-buildcache option to install

9828e65

Allow differentiating between top level packages and dependencies when determining whether to install from the cache or not.

Add unit test for --use-buildcache

9977508

Add test of supported edge case and bug fix

590d56d

Review fixes

66fe857

Use metavar to display use-buildcache options.

550c150

Use install_mockery_mutable_config for fixture

aad8977

Update spack-completion

a2c79ae

Fix bug in distinguishing requested and explicit.

5d94032

A requested package is not required to be "explicit"ly requested when specifying --only package or --use-buildcache package:<...> options in install. Add a task property that returns the former.

Fix installer unit tests

d2a457e

kwryankrattiger force-pushed the install_use_buildcache branch from faa445c to d2a457e Compare September 23, 2022 15:17

Change name to is_root instead of is_package

3268798

scottwittenburg approved these changes Sep 28, 2022

View reviewed changes

tgamblin approved these changes Sep 29, 2022

View reviewed changes

kwryankrattiger merged commit a01c36d into spack:develop Sep 29, 2022

kwryankrattiger mentioned this pull request Oct 4, 2022

Install from source if binary cache checksum validation fails #31696

Merged

haampie reviewed Oct 16, 2022

View reviewed changes

haampie mentioned this pull request Oct 17, 2022

installer.py: fix/test get_deptypes #33363

Merged

haampie added a commit to haampie/spack that referenced this pull request Oct 17, 2022

installer.py: fix/test get_deptypes

bbbae14

Fixing an oversight in spack#32537 `get_deptypes` should depend on new `package/dependencies_cache_only` props.

haampie mentioned this pull request Oct 17, 2022

installer.py: traverse_dependencies has local deptype #33367

Merged

haampie added a commit to haampie/spack that referenced this pull request Oct 17, 2022

installer.py: fix/test get_deptypes

b11628c

Fixing an oversight in spack#32537 `get_deptypes` should depend on new `package/dependencies_cache_only` props.

haampie added a commit to haampie/spack that referenced this pull request Oct 17, 2022

installer.py: fix/test get_deptypes

9b8a4e7

Fixing an oversight in spack#32537 `get_deptypes` should depend on new `package/dependencies_cache_only` props.

haampie added a commit that referenced this pull request Oct 17, 2022

installer.py: fix/test get_deptypes (#33363)

e882583

Fixing an oversight in #32537 `get_deptypes` should depend on new `package/dependencies_cache_only` props.

tgamblin mentioned this pull request Nov 7, 2022

Better control of how package and dependencies are installed #29695

Closed

2 tasks

kwryankrattiger deleted the install_use_buildcache branch November 15, 2022 15:52

Conversation

kwryankrattiger commented Sep 6, 2022 • edited by scottwittenburg Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kwryankrattiger commented Sep 7, 2022

Uh oh!

spackbot-app bot commented Sep 7, 2022

Uh oh!

spackbot-app bot commented Sep 7, 2022

Uh oh!

scottwittenburg commented Sep 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scottwittenburg commented Sep 12, 2022

Uh oh!

spackbot-app bot commented Sep 12, 2022

Uh oh!

spackbot-app bot commented Sep 12, 2022

Uh oh!

Uh oh!

scottwittenburg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

scottwittenburg commented Sep 13, 2022

Uh oh!

scottwittenburg commented Sep 13, 2022

Uh oh!

spackbot-app bot commented Sep 13, 2022

Uh oh!

spackbot-app bot commented Sep 13, 2022

Uh oh!

scottwittenburg left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alalazo commented Sep 22, 2022

Uh oh!

scottwittenburg commented Sep 22, 2022

Uh oh!

scottwittenburg commented Sep 22, 2022

Uh oh!

alalazo commented Sep 22, 2022

Uh oh!

scottwittenburg commented Sep 22, 2022

Uh oh!

scottwittenburg left a comment

Choose a reason for hiding this comment

Uh oh!

haampie left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kwryankrattiger commented Sep 6, 2022 •

edited by scottwittenburg

Loading

scottwittenburg commented Sep 9, 2022 •

edited

Loading

scottwittenburg left a comment •

edited

Loading