Skip to content

binary_distribution.py: list parent dirs in binary tarball#41773

Merged
haampie merged 8 commits intospack:developfrom
haampie:fix/subdirs-in-tarball
Jan 10, 2024
Merged

binary_distribution.py: list parent dirs in binary tarball#41773
haampie merged 8 commits intospack:developfrom
haampie:fix/subdirs-in-tarball

Conversation

@haampie
Copy link
Copy Markdown
Member

@haampie haampie commented Dec 19, 2023 •
edited
Loading

Previously the directory structure on the spack create tarballs were as follows:

path/to/prefix
path/to/prefix/bin
path/to/prefix/bin/app
...
path/to/prefix/.spack/binary_distribution

With this PR, they look like this:

path
path/to
path/to/prefix
path/to/prefix/bin
path/to/prefix/bin/app
...
path/to/prefix/.spack/binary_distribution

The reason for this change is that AWS lambda functions complain about missing
parent directories when giving them a Spack package as an OCI image layer.

Other container runtimes seem to have no issues with this.

Compatibility

Tarballs are backwards compatible, but not forwards compatible, because Spack 0.21
determines the package prefix as that prefix common to all tarfile entries -- so, that'd
be path in the example above instead of path/to/prefix.

Instead of (or additionally to) bumping CURRENT_BUILD_CACHE_LAYOUT_VERSION I'm
also happy to backport the search mechanism introduced in this PR, which is to compute
the root relative to the lowest-depth .spack/binary_distribution file.

@spackbot-app spackbot-app bot added binary-packages core PR affects Spack core functionality tests General test capability(ies) labels Dec 19, 2023
@haampie haampie force-pushed the fix/subdirs-in-tarball branch from 389b940 to bf1fa1b Compare December 19, 2023 12:12
@Jayapreethi
Copy link
Copy Markdown

Jayapreethi commented Dec 20, 2023

We rebuilt the image with the updated spack and changed the padded length configuration. The container deployment is successful and awslambda got invoked. Thank you :)

@haampie haampie added this to the v0.21.1 milestone Dec 20, 2023
@cameronrutherford
Copy link
Copy Markdown
Contributor

cameronrutherford commented Dec 21, 2023 •
edited
Loading

+1, this fixed our deployment!

I have a follow up question about the padded length: why would you ever not use padded_length: False? For AWS Lambda functions there is a 16KB limit from this so post with a similar issue, and so having a length of 128 resulted in us being past this limit (26.17KB). Setting it to false moved us under that limit, so we were able to invoke our lambda per @Jayapreethi

@haampie
Copy link
Copy Markdown
Member Author

haampie commented Dec 21, 2023 •
edited
Loading

See the section about relocation for why to use a longer padded_length. If you're just using container images to distribute the binaries, you can certainly disable it -- just make sure you're not changing the install location to something longer later.

If the environment variables are still too big, you can disable some that aren't necessary. See the section about prefix_inspections. I'm reasonably sure you can disable all of them, cause PATH is set regardless, and probably that's the only one you care about:

spack:
  specs: [x, y, z]
  modules:
    # do not set default env variables (double colon is not a typo, it overrides the value)
    prefix_inspections:: {}

If you still end up with too many bytes of config, one thing I've been thinking about is to add one last layer to the container image with an environment view, so that all executables end up in <view>/bin/ and only one directory has to go into PATH.

alalazo
alalazo reviewed Jan 5, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@alalazo alalazo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only minor comments, but basically LGTM. Do we have a test ensuring that new Spack can still use layout version v1 ?

@alalazo alalazo self-assigned this Jan 5, 2024
@alalazo alalazo mentioned this pull request Jan 8, 2024
Merged
36 tasks
@haampie haampie merged commit 8c1226e into spack:develop Jan 10, 2024
@haampie haampie deleted the fix/subdirs-in-tarball branch January 10, 2024 12:21
haampie added a commit that referenced this pull request Jan 10, 2024
@haampie
binary_distribution.py: support build cache layout 2 (#41773)
e3436d3 
Add forward compatibility for tarballs created by Spack 0.22, which
use build cache layout version 2.

Spack 0.21 continues to produce build cache layout version 1 tarballs.

Build cache layout version 2 also lists parent directories of the
package prefix in the tarball, which is required for certain container
runtimes.
haampie added a commit that referenced this pull request Jan 10, 2024
@haampie
binary_distribution.py: support build cache layout 2 (#41773)
e42f167 
Add forward compatibility for tarballs created by Spack 0.22, which
use build cache layout version 2.

Spack 0.21 continues to produce build cache layout version 1 tarballs.

Build cache layout version 2 also lists parent directories of the
package prefix in the tarball, which is required for certain container
runtimes.
haampie added a commit that referenced this pull request Jan 10, 2024
@haampie
binary_distribution.py: support build cache layout 2 (#41773)
48936c8 
Add forward compatibility for tarballs created by Spack 0.22, which
use build cache layout version 2.

Spack 0.21 continues to produce build cache layout version 1 tarballs.

Build cache layout version 2 also lists parent directories of the
package prefix in the tarball, which is required for certain container
runtimes.
haampie added a commit that referenced this pull request Jan 10, 2024
@haampie
binary_distribution.py: support build cache layout 2 (#41773)
72cdfbd 
Add forward compatibility for tarballs created by Spack 0.22, which
use build cache layout version 2.

Spack 0.21 continues to produce build cache layout version 1 tarballs.

Build cache layout version 2 also lists parent directories of the
package prefix in the tarball, which is required for certain container
runtimes.
alalazo pushed a commit that referenced this pull request Jan 10, 2024
@haampie @alalazo
binary_distribution.py: support build cache layout 2 (#41773)
a17e23f 
Add forward compatibility for tarballs created by Spack 0.22, which
use build cache layout version 2.

Spack 0.21 continues to produce build cache layout version 1 tarballs.

Build cache layout version 2 also lists parent directories of the
package prefix in the tarball, which is required for certain container
runtimes.
haampie added a commit that referenced this pull request Jan 11, 2024
@haampie
binary_distribution.py: support build cache layout 2 (#41773)
19e8608 
Add forward compatibility for tarballs created by Spack 0.22, which
use build cache layout version 2.

Spack 0.21 continues to produce build cache layout version 1 tarballs.

Build cache layout version 2 also lists parent directories of the
package prefix in the tarball, which is required for certain container
runtimes.
@haampie haampie mentioned this pull request Jan 15, 2024
Closed
@cameronrutherford
Copy link
Copy Markdown
Contributor

cameronrutherford commented Jan 15, 2024

I went on vacation and only just read through this now. Thank you for the thorough explanation, and the changes make sense to me. We were able to get unstuck thanks to these fixes, so again I really appreciate your help!

RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Jan 25, 2024
@haampie @RikkiButler20
binary_distribution.py: list parent dirs in binary tarball (spack#41773)
f72381a 
* Bump the build cache layout version from 1 to 2
* Version to lists parent directories of the prefix in the tarball too, which is required from some container runtimes
RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Jan 31, 2024
@haampie @RikkiButler20
binary_distribution.py: list parent dirs in binary tarball (spack#41773)
50fbab6 
* Bump the build cache layout version from 1 to 2
* Version to lists parent directories of the prefix in the tarball too, which is required from some container runtimes
greenc-FNAL pushed a commit to FNALssi/spack that referenced this pull request Mar 21, 2024
@haampie @greenc-FNAL
binary_distribution.py: support build cache layout 2 (spack#41773)
88899ca 
Add forward compatibility for tarballs created by Spack 0.22, which
use build cache layout version 2.

Spack 0.21 continues to produce build cache layout version 1 tarballs.

Build cache layout version 2 also lists parent directories of the
package prefix in the tarball, which is required for certain container
runtimes.
kwryankrattiger pushed a commit to kwryankrattiger/spack that referenced this pull request Jul 9, 2024
@haampie @kwryankrattiger
binary_distribution.py: list parent dirs in binary tarball (spack#41773)
e4be0e7 
* Bump the build cache layout version from 1 to 2
* Version to lists parent directories of the prefix in the tarball too, which is required from some container runtimes
vjranagit pushed a commit to vjranagit/spack that referenced this pull request Jan 18, 2026
@haampie
binary_distribution.py: support build cache layout 2 (spack#41773)
2d7014d 
Add forward compatibility for tarballs created by Spack 0.22, which
use build cache layout version 2.

Spack 0.21 continues to produce build cache layout version 1 tarballs.

Build cache layout version 2 also lists parent directories of the
package prefix in the tarball, which is required for certain container
runtimes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alalazo alalazo alalazo approved these changes

Assignees

@alalazo alalazo

Labels

binary-packages core PR affects Spack core functionality tests General test capability(ies)

Projects

None yet

Milestone

v0.21.1

Development

Successfully merging this pull request may close these issues.

4 participants

@haampie @Jayapreethi @cameronrutherford @alalazo