Install: Add use-buildcache option to install

[kwryankrattiger]

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

@scottwittenburg

Implements this feature request.

[kwryankrattiger]

@spackbot fix style

[spackbot-app]

Let me see if I can fix that for you!

[spackbot-app]

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]

@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]

@spackbot fix style

[spackbot-app]

Let me see if I can fix that for you!

[spackbot-app]

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.

[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

[scottwittenburg]

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]

@spackbot fix style

[spackbot-app]

Let me see if I can fix that for you!

[spackbot-app]

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?

[scottwittenburg]

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]

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" "unifyfs@1.0%oneapi@2022.1.0 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]

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.

[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.