Allow spec.json files to be clearsigned, use transparent compression for *.json by haampie · Pull Request #34490 · spack/spack

haampie · 2022-12-13T10:26:55Z

This PR does two things:

Reduce the number of requests to a mirror for metadata
Make Spack accept gzipped metadata (i.e. transmit fewer bytes)

It does not make any changes to how we create binary caches.
The idea is to allow this PR to be backported.

Currently we have separate spec.json and spec.json.sig files. The
former is a pure spec, the latter is clearsigned spec. This is unfortunate,
cause metadata lookups now requires at most two requests per remote.

Further, update-index logic fetches all spec.json files, which can take
quite some bandwidth. We can drop ~85% of the size by compressing
those files. To avoid yet another lookup (spec.json.sig.gz etc) we can
use transparent decompression. So a file is decompressed when the
magic bytes signal it's gzip, otherwise it's treated as text.

This PR allows spec.json (and index.json) files to be any combination
of the cartesian product [gzip compressed, uncompressed] x
[clearsigned, unsigned].

The idea is to backport this to 0.19.1, since for 0.19.1 it is not a
behavioral change, but rather a bugfix for improved Spack 0.21 forward
compatibility.

Next steps after this PR:

Backport to 0.19.1
Start uploading spec.json.sig files both as spec.json.sig and
spec.json. This ensure they work with any Spack version.
Compress before pushing (if the filesize is reduced)
Deprecate spec.json.sig in 0.20.
After Spack 0.20 branches off, stop uploading *.sig, and stop checking
the remote for *.sig files.
Potentially we can also add a config option to disable checking for
*.sig files on develop earlier.

See spack#34491. Should go after spack#34490.

scottwittenburg

Looks great, thanks @haampie 👍

I just had one question, but feel free to merge regardless.

lib/spack/spack/binary_distribution.py

haampie · 2023-01-05T21:30:20Z

lib/spack/spack/binary_distribution.py

-                local_specfile_stage.destroy()
+            # If it is a clearsign file, we must verify it (unless disabled)
+            should_verify = not unsigned and is_clearsigned_file(specfile_path)
+            if should_verify and not try_verify(specfile_path):


Hm, another issue is that this is not yet working, cause it's first clearsigned and then gzipped, so verification requires decompression. Should the order be reverted? Sign the compressed json?

Went with compress(sign(json)) so that we can swap the compression scheme without the need to re-sign the files.

Currently we have separate `spec.json` and `spec.json.sig` files. The former is a pure spec, the latter is clearsigned spec. This is a bit unfortunate, cause a lookup for a spec file now requires two remote requests (with or without cache). This PR allows clearsigned `spec.json` files too, and changes otherwise nothing. The idea is to backport this to 0.19.1, since for 0.19.1 it is not a behavioral change, but rather a bugfix for improved 0.20 / 0.21 forward compatibility. Next steps after this PR: - Start uploading spec.json.sig files both as spec.json.sig and spec.json. This ensure they work with any Spack version. - Deprecate spec.json.sig in 0.20. - After Spack 0.20 branches off, stop uploading *.sig, and stop checking the remote for *.sig files. - Potentially we can also add a config option to disable checking for *.sig files on develop earlier.

The files spec.json / spec.json.sig / index.json can now be stored gzip compressed (without .gz extension)

scottwittenburg

Looks good to my eyeballs 😆 thanks @haampie! I notice the past several gitlab pipelines haven't built anything (as expected), but since this isn't actually changing what we push (still pushing .spec.json.sig, not compressing them yet), this isn't likely to break anything when run in the wild. But it's good preparation for the next steps, thanks again 👍

…ression for *.json (#34490)" This reverts commit 8a1b817.

…ression for *.json (#34490)" (#34856) This reverts commit 8a1b817.

…for *.json (spack#34490) This commit allows (remote) spec.json files to be clearsigned and gzipped. The idea is to reduce the number of requests and number of bytes transferred

…ression for *.json (spack#34490)" (spack#34856) This reverts commit 8a1b817.

…for *.json (spack#34490) This commit allows (remote) spec.json files to be clearsigned and gzipped. The idea is to reduce the number of requests and number of bytes transferred

…ression for *.json (spack#34490)" (spack#34856) This reverts commit 8a1b817.

…for *.json (spack#34490) This commit allows (remote) spec.json files to be clearsigned and gzipped. The idea is to reduce the number of requests and number of bytes transferred

…ression for *.json (spack#34490)" (spack#34856) This reverts commit 8a1b817.

…for *.json (spack#34490) This commit allows (remote) spec.json files to be clearsigned and gzipped. The idea is to reduce the number of requests and number of bytes transferred

…ression for *.json (spack#34490)" (spack#34856) This reverts commit 8a1b817.

spackbot-app bot added binary-packages commands core PR affects Spack core functionality tests General test capability(ies) labels Dec 13, 2022

haampie requested a review from scottwittenburg December 13, 2022 10:28

haampie mentioned this pull request Dec 13, 2022

Phasing out spec.json.sig #34491

Open

haampie added a commit to haampie/spack that referenced this pull request Dec 13, 2022

Upload spec.json.sig as spec.json{.sig,}

e9cd77c

See spack#34491. Should go after spack#34490.

haampie mentioned this pull request Dec 13, 2022

Upload spec.json.sig as spec.json{.sig,} #34492

Closed

haampie added a commit to haampie/spack that referenced this pull request Dec 13, 2022

Upload spec.json.sig as spec.json{.sig,}

6624cee

See spack#34491. Should go after spack#34490.

haampie mentioned this pull request Dec 14, 2022

binary cache: do not create separate spec.json.sig files #34350

Closed

haampie changed the title ~~Allow spec.json files to be clearsigned~~ Allow spec.json files to be clearsigned and compressed Jan 5, 2023

haampie force-pushed the binary_cache/allow-clearsigned-spec.json-files branch 6 times, most recently from 92bf8c8 to f48b6ca Compare January 5, 2023 14:17

haampie changed the title ~~Allow spec.json files to be clearsigned and compressed~~ Allow spec.json files to be clearsigned, use transparent compression of *.json Jan 5, 2023

haampie changed the title ~~Allow spec.json files to be clearsigned, use transparent compression of *.json~~ Allow spec.json files to be clearsigned, use transparent compression for *.json Jan 5, 2023

scottwittenburg previously approved these changes Jan 5, 2023

View reviewed changes

lib/spack/spack/binary_distribution.py Outdated Show resolved Hide resolved

lib/spack/spack/binary_distribution.py Show resolved Hide resolved

haampie commented Jan 5, 2023

View reviewed changes

haampie dismissed scottwittenburg’s stale review via 018ba60 January 5, 2023 22:29

haampie force-pushed the binary_cache/allow-clearsigned-spec.json-files branch from f48b6ca to 018ba60 Compare January 5, 2023 22:29

haampie and others added 2 commits January 5, 2023 23:36

Allow transparent decompression

6c20ec1

The files spec.json / spec.json.sig / index.json can now be stored gzip compressed (without .gz extension)

haampie force-pushed the binary_cache/allow-clearsigned-spec.json-files branch from 018ba60 to 6c20ec1 Compare January 5, 2023 22:36

haampie requested a review from scottwittenburg January 6, 2023 13:43

scottwittenburg approved these changes Jan 6, 2023

View reviewed changes

haampie merged commit 8a1b817 into spack:develop Jan 7, 2023

haampie deleted the binary_cache/allow-clearsigned-spec.json-files branch January 7, 2023 11:22

haampie added a commit that referenced this pull request Jan 9, 2023

Revert "Allow spec.json files to be clearsigned, use transparent comp…

f1128e1

…ression for *.json (#34490)" This reverts commit 8a1b817.

haampie mentioned this pull request Jan 9, 2023

Revert "Allow spec.json files to be clearsigned, use transparent compression for *.json" #34856

Merged

alalazo pushed a commit that referenced this pull request Jan 9, 2023

Revert "Allow spec.json files to be clearsigned, use transparent comp…

47011f5

…ression for *.json (#34490)" (#34856) This reverts commit 8a1b817.

RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Jan 24, 2023

Revert "Allow spec.json files to be clearsigned, use transparent comp…

c7b24f2

…ression for *.json (spack#34490)" (spack#34856) This reverts commit 8a1b817.

amd-toolchain-support pushed a commit to amd-toolchain-support/spack that referenced this pull request Feb 16, 2023

Revert "Allow spec.json files to be clearsigned, use transparent comp…

7a31200

…ression for *.json (spack#34490)" (spack#34856) This reverts commit 8a1b817.

techxdave pushed a commit to Tech-XCorp/spack that referenced this pull request Feb 17, 2023

Revert "Allow spec.json files to be clearsigned, use transparent comp…

efb7df8

…ression for *.json (spack#34490)" (spack#34856) This reverts commit 8a1b817.

jmcarcell pushed a commit to key4hep/spack that referenced this pull request Apr 13, 2023

Revert "Allow spec.json files to be clearsigned, use transparent comp…

2a2ce42

…ression for *.json (spack#34490)" (spack#34856) This reverts commit 8a1b817.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow spec.json files to be clearsigned, use transparent compression for *.json#34490

Allow spec.json files to be clearsigned, use transparent compression for *.json#34490
haampie merged 2 commits intospack:developfrom
haampie:binary_cache/allow-clearsigned-spec.json-files

haampie commented Dec 13, 2022 •

edited

Loading

Uh oh!

scottwittenburg left a comment

Uh oh!

Uh oh!

Uh oh!

haampie Jan 5, 2023

Uh oh!

haampie Jan 5, 2023

Uh oh!

scottwittenburg left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

haampie commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

scottwittenburg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

haampie Jan 5, 2023

Choose a reason for hiding this comment

Uh oh!

haampie Jan 5, 2023

Choose a reason for hiding this comment

Uh oh!

scottwittenburg left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

haampie commented Dec 13, 2022 •

edited

Loading