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

Manifest list support nits #1268

Merged
merged 14 commits into from
Jun 22, 2021
Merged

Manifest list support nits #1268

merged 14 commits into from
Jun 22, 2021

Conversation

mtrmac
Copy link
Collaborator

@mtrmac mtrmac commented Jun 19, 2021

An assorted set of small cleanups and some test extensions, mostly related to #400 .

See individual commit messages for details.

@mtrmac mtrmac force-pushed the manifest-list-nits branch from 4dbce95 to 8037df1 Compare June 21, 2021 17:21
Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM

mtrmac added 14 commits June 22, 2021 21:45
... and acknowledge that various tests are strictly speaking
invalid, to reinforce that real callers must not pass nil.

Signed-off-by: Miloslav Trmač <[email protected]>
nil is a valid value for *types.SystemContext.

Signed-off-by: Miloslav Trmač <[email protected]>
It's used to access an ImageStreamImage resource.

Signed-off-by: Miloslav Trmač <[email protected]>
- Use a different manifest and manifest list
- Write per-arch manifest before the top-level one
- Same for signatures
- Don't write the top-level manifest twice

Signed-off-by: Miloslav Trmač <[email protected]>
Signed-off-by: Miloslav Trmač <[email protected]>
Missing s.manifest was already checked at the top of the
function.

Signed-off-by: Miloslav Trmač <[email protected]>
We are definitely going to need it at least in Commit,
so compute it already in PutManifest; that fails early
and avoids computing it in PutSignatures.

Signed-off-by: Miloslav Trmač <[email protected]>
We save s.manifest anyway, so avoid the overhead of doing it twice
(probably primarily taking the required c/storage lock).

Signed-off-by: Miloslav Trmač <[email protected]>
The extra hash table it maintains is a bit more expensive,
but it allows us to directly express our preferences, and
it is more similar to determineManifestConversion
(if we ever wanted to add a prioritization for
manifest list formats).

Also use the two-argument for...range loops to avoid
repetition of the long array names.

Signed-off-by: Miloslav Trmač <[email protected]>
... instead of something else + shutting up lint.

Signed-off-by: Miloslav Trmač <[email protected]>
@mtrmac mtrmac force-pushed the manifest-list-nits branch from 8037df1 to c1d27bb Compare June 22, 2021 19:48
@mtrmac mtrmac merged commit 20b148f into containers:main Jun 22, 2021
@mtrmac mtrmac deleted the manifest-list-nits branch June 22, 2021 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants