Skip to content
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

Cache Shell Script of spack load and spack env activate #47755

Draft
wants to merge 32 commits into
base: develop
Choose a base branch
from

Conversation

RikkiButler20
Copy link
Contributor

@RikkiButler20 RikkiButler20 commented Nov 23, 2024

Fixes #47603
Fixes #21413

This will fix the problem of spack load and spack env activate taking too long and currently iterating over all the package's dependency's python package files. This will also prevents the drifting of the installed spec from the current, updated version on Spack

  • Create a new hook that will save shell script at install time
  • Execute shell script when spack load is run
  • Check if there is shell script is cached and if any of the specs have changed when activated environment is installed
  • Add tests
  • Update documentation

@RikkiButler20 RikkiButler20 self-assigned this Nov 23, 2024
@spackbot-app spackbot-app bot added core PR affects Spack core functionality tests General test capability(ies) labels Nov 23, 2024
@adamjstewart
Copy link
Member

Very excited for this and happy to test on fish!

@adamjstewart adamjstewart added this to the v0.24 milestone Nov 23, 2024
@aweits
Copy link
Contributor

aweits commented Nov 24, 2024

What if we cached the environment changes at install-time instead? That would also solve issues related to allowing older installs to still function/load... (willing to help with this, would allow me to jettison some local code that is getting harder and harder to maintain)

@tgamblin
Copy link
Member

tgamblin commented Nov 24, 2024

What if we cached the environment changes at install-time instead?

This is the plan. There are some details in the slack discussion linked from #47603, and the bit about “package drift” there.

i think @RikkiButler20 is doing this in steps (see checklist) — probably need one or two more to move the logic to package install time and env install time.

@aweits I think if you want to help, iterating on reviews would be great so we can get this in for 1.0!

@becker33 becker33 modified the milestones: v1.0.0, v1.0.0 stretch goals Nov 25, 2024
Test does not pass yet
@tgamblin tgamblin modified the milestones: v1.0.0 stretch goals, v1.0.0 Nov 26, 2024
Test still fails, but test works properly now
@RikkiButler20
Copy link
Contributor Author

@spackbot Fix style

Copy link

spackbot-app bot commented Nov 27, 2024

Let me see if I can fix that for you!

Copy link

spackbot-app bot commented Nov 27, 2024

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

spack style --fix
==> Running style checks on spack
  selected: import, isort, black, flake8, mypy
==> Modified files
  lib/spack/spack/cmd/load.py
  lib/spack/spack/test/cmd/load.py
==> Running import checks
import check requires Python 3.9 or later
  import checks were clean
==> Running isort checks
  isort checks were clean
==> Running black checks
reformatted lib/spack/spack/cmd/load.py
reformatted lib/spack/spack/test/cmd/load.py
All done! ✨ 🍰 ✨
2 files reformatted.
  black checks were clean
==> Running flake8 checks
  flake8 checks were clean
==> Running mypy checks
lib/spack/spack/version/version_types.py:145: error: Argument 2 to "StandardVersion" has incompatible type "*Tuple[Tuple[Any, ...], Tuple[Any, ...]]"; expected "Tuple[Tuple[Any, ...], Tuple[Any, ...]]"  [arg-type]
lib/spack/spack/version/version_types.py:452: error: Argument 2 to "StandardVersion" has incompatible type "*Tuple[Tuple[Any, ...], Tuple[Any, ...]]"; expected "Tuple[Tuple[Any, ...], Tuple[Any, ...]]"  [arg-type]
lib/spack/spack/version/version_types.py:481: error: Argument 2 to "StandardVersion" has incompatible type "*Tuple[Tuple[Any, ...], Tuple[Any, ...]]"; expected "Tuple[Tuple[Any, ...], Tuple[Any, ...]]"  [arg-type]
lib/spack/spack/variant.py:131: error: Unsupported right operand type for in ("Union[Collection[Any], Callable[..., Any]]")  [operator]
Found 4 errors in 2 files (checked 624 source files)
  mypy 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 style fixes.

lib/spack/spack/cmd/load.py Outdated Show resolved Hide resolved
@RikkiButler20
Copy link
Contributor Author

@spackbot Fix style

Copy link

spackbot-app bot commented Dec 5, 2024

Let me see if I can fix that for you!

Copy link

spackbot-app bot commented Dec 5, 2024

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

spack style --fix
==> Running style checks on spack
  selected: import, isort, black, flake8, mypy
==> Modified files
  lib/spack/spack/cmd/load.py
  lib/spack/spack/test/cmd/load.py
==> Running import checks
import check requires Python 3.9 or later
  import checks were clean
==> Running isort checks
  isort checks were clean
==> Running black checks
reformatted lib/spack/spack/test/cmd/load.py
All done! ✨ 🍰 ✨
1 file reformatted, 1 file left unchanged.
  black checks were clean
==> Running flake8 checks
  flake8 checks were clean
==> Running mypy checks
lib/spack/spack/version/version_types.py:145: error: Argument 2 to "StandardVersion" has incompatible type "*Tuple[Tuple[Any, ...], Tuple[Any, ...]]"; expected "Tuple[Tuple[Any, ...], Tuple[Any, ...]]"  [arg-type]
lib/spack/spack/version/version_types.py:452: error: Argument 2 to "StandardVersion" has incompatible type "*Tuple[Tuple[Any, ...], Tuple[Any, ...]]"; expected "Tuple[Tuple[Any, ...], Tuple[Any, ...]]"  [arg-type]
lib/spack/spack/version/version_types.py:481: error: Argument 2 to "StandardVersion" has incompatible type "*Tuple[Tuple[Any, ...], Tuple[Any, ...]]"; expected "Tuple[Tuple[Any, ...], Tuple[Any, ...]]"  [arg-type]
lib/spack/spack/variant.py:131: error: Unsupported right operand type for in ("Union[Collection[Any], Callable[..., Any]]")  [operator]
Found 4 errors in 2 files (checked 624 source files)
  mypy 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 style fixes.

Test that the spack uses the new hook and the shell script is cached
Also, add new test to make sure there isn't overlap in shell script
@spackbot-app spackbot-app bot added the documentation Improvements or additions to documentation label Jan 3, 2025
@adamjstewart
Copy link
Member

Going to unsubscribe from this PR while it's still a WIP (too many notifications) but please ping me to review fish support once it is complete.

@RikkiButler20 RikkiButler20 marked this pull request as draft January 16, 2025 20:26
@RikkiButler20
Copy link
Contributor Author

@spackbot Fix style

Copy link

spackbot-app bot commented Jan 16, 2025

Let me see if I can fix that for you!

Copy link

spackbot-app bot commented Jan 16, 2025

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

spack style --fix
==> Running style checks on spack
  selected: import, isort, black, flake8, mypy
==> Modified files
  lib/spack/spack/cmd/load.py
  lib/spack/spack/hooks/__init__.py
  lib/spack/spack/hooks/cache_shell_script.py
  lib/spack/spack/test/environment_modifications.py
  lib/spack/spack/test/hooks/cache_shell_script.py
  lib/spack/spack/test/util/environment.py
  lib/spack/spack/util/environment.py
==> Running import checks
import check requires Python 3.9 or later
  import checks were clean
==> Running isort checks
Fixing /tmp/tmpu87evsa3/spack/lib/spack/spack/test/hooks/cache_shell_script.py
  isort checks were clean
==> Running black checks
reformatted lib/spack/spack/test/hooks/cache_shell_script.py
reformatted lib/spack/spack/util/environment.py
All done! ✨ 🍰 ✨
2 files reformatted, 5 files left unchanged.
  black checks were clean
==> Running flake8 checks
lib/spack/spack/cmd/load.py:12: [F401] 'spack.user_environment as uenv' imported but unused
lib/spack/spack/cmd/load.py:13: [F401] 'spack.util.environment' imported but unused
  flake8 found errors
==> Running mypy checks
lib/spack/spack/util/environment.py:662: error: Item "NameModifier" of "Union[NameModifier, NameValueModifier]" has no attribute "_cache_str"  [union-attr]
lib/spack/spack/util/environment.py:662: error: Item "NameValueModifier" of "Union[NameModifier, NameValueModifier]" has no attribute "_cache_str"  [union-attr]
lib/spack/spack/version/version_types.py:135: error: Incompatible types in assignment (expression has type "Tuple[Any, ...]", variable has type "Tuple[str]")  [assignment]
lib/spack/spack/variant.py:130: error: Unsupported right operand type for in ("Union[Collection[Any], Callable[..., Any]]")  [operator]
lib/spack/spack/build_environment.py:171: error: Overloaded function signatures 1 and 2 overlap with incompatible return types  [misc]
lib/spack/spack/build_environment.py:171: error: Overloaded function signatures 1 and 3 overlap with incompatible return types  [misc]
Found 6 errors in 4 files (checked 636 source files)
  mypy 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 style fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commands core PR affects Spack core functionality documentation Improvements or additions to documentation feature shell-support tests General test capability(ies) utilities
Projects
None yet
6 participants