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

Fix content not found with push=true,rewrite-timestamp=true #5008

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

AkihiroSuda
Copy link
Member

Fix #4793

if opts.RewriteTimestamp {
annotations := map[digest.Digest]map[string]string{}
addAnnotations(annotations, *desc)
// e.pushImage cannot be used because src ref does not point to the rewritten image
Copy link
Member

Choose a reason for hiding this comment

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

What's the context behind removing this condition

Copy link
Member Author

Choose a reason for hiding this comment

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

The comment seems outdated and the CI has no complaint on removing the code

dockerfile: `FROM amd64/debian:bullseye-20230109 AS dangling
FROM amd64/debian:bullseye-20230109-slim
`,
expectedDigest: "sha256:eeba8ef81dec46359d099c5d674009da54e088fa8f29945d4d7fb3a7a88c450e",
Copy link
Member

Choose a reason for hiding this comment

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

Could we get these digests computed with digest.FromBytes() . Atm. it is hard/impossible to understand what is the expected result or where this digest is coming from.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can't know the manifest digest until building it

Copy link
Member

Choose a reason for hiding this comment

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

Well, you need to know the contents of the manifest if you know its digest here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it is not specific to this PR and can be revisited in a separate issue/PR?

frontend/dockerfile/dockerfile_test.go Outdated Show resolved Hide resolved
@tonistiigi tonistiigi merged commit 5642608 into moby:master Jun 13, 2024
75 checks passed
@tonistiigi
Copy link
Member

@AkihiroSuda is this for v0.14.1?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[v0.13] fail to export image when using rewrite-timestamp=true
2 participants