-
Notifications
You must be signed in to change notification settings - Fork 47
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
chore: bump WWS to 1.5.0 #139
Conversation
Need a rebase |
1b60a94
to
7809bce
Compare
Thanks @Mossaka! The bump looks super clean with all the changes you made! I will promote the PR from Draft after a smoke test :) |
291c052
to
0cfd64d
Compare
Smoke test working fine, this is ready for review :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just left one comment. Mostly LGTM!
@@ -93,7 +93,7 @@ update-deps: | |||
|
|||
test/out_%/img.tar: images/%/Dockerfile | |||
mkdir -p $(@D) | |||
docker buildx build --platform=wasi/wasm --load -t $(TEST_IMG_NAME_$*) ./images/$* | |||
docker buildx build --provenance=false --platform=wasi/wasm --load -t $(TEST_IMG_NAME_$*) ./images/$* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please explain why setting provenance to false here as I am not so familiar with provenance attestations. Even better if you could write a comment explaining above this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When building with buildx on my current ubuntu-based VM, I'm getting: "ERROR: failed to solve: no match for platform in manifest...".
Complete buildx output
docker buildx build --platform=wasi/wasm --load -t wasmtest_wws:latest ./images/wws
[+] Building 0.4s (5/5) FINISHED docker:default
=> [internal] load build definition from Dockerfile 0.0s
=> => transferring dockerfile: 107B 0.0s
=> [internal] load .dockerignore 0.0s
=> => transferring context: 2B 0.0s
=> [internal] load build context 0.0s
=> => transferring context: 30B 0.0s
=> CACHED [1/1] COPY --chmod=0755 ./hello.js . 0.0s
=> ERROR exporting to image 0.2s
=> => exporting layers 0.0s
=> => exporting manifest sha256:37fecfda4b76efe65283b3e7ad60251262c6648cc4379aeed0ba8f524d7af782 0.0s
=> => exporting config sha256:2a94bf471b8da7a262c690effbecaaddea5f5697d1986ab564f5b95989b0315d 0.0s
=> => exporting attestation manifest sha256:2afe3934e1218e1ff3e4892d2699a5b8ac9b75e2cded3e088b7e6b9672dd0e69 0.1s
=> => exporting manifest list sha256:363816af6aeab000e88f0a8458449eff31ec63715e1149bfcc8f66c1f8d9fb40 0.0s
=> => naming to docker.io/library/wasmtest_wws:latest 0.0s
=> => unpacking to docker.io/library/wasmtest_wws:latest 0.0s
------
> exporting to image:
------
ERROR: failed to solve: no match for platform in manifest sha256:363816af6aeab000e88f0a8458449eff31ec63715e1149bfcc8f66c1f8d9fb40: not found
make: *** [Makefile:96: test/out_wws/img.tar] Error 1
This seems related to moby/buildkit#3891. This was workarounded in moby/buildkit#3983, released with v0.12.0-rc2 onwards. However, the version of Docker I have installed is 24.0.5, which seems to use buildkit 0.11.7.
I'm going to update the PR with a specific commit for this change on the Makefile (while adding a comment as you suggested).
If you think we should avoid adding this completely I'm fine as well, I can keep this change on my stash of local patches :)
PR updated with an explanation on the Makefile with a dedicated commit :) |
@ereslibre could you please sign your commits? |
Signed-off-by: Rafael Fernández López <rfernandezl@vmware.com>
This will get fixed with a more recent version of Docker that bumps buildkit at least to 0.12.0, so the `--provenance=false` flag can be removed with a future release of Docker. Signed-off-by: Rafael Fernández López <rfernandezl@vmware.com>
I'm sorry again 🙏 I forgot 🤦 I just updated the PR, both commits signed with GPG and signed off too. |
Thank you! @ereslibre I will merge once the CI passes |
Bump WWS to 1.5.0