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

chore: bump WWS to 1.5.0 #139

Merged
merged 2 commits into from
Sep 6, 2023
Merged

chore: bump WWS to 1.5.0 #139

merged 2 commits into from
Sep 6, 2023

Conversation

ereslibre
Copy link
Contributor

@ereslibre ereslibre commented Sep 1, 2023

Bump WWS to 1.5.0

@Mossaka
Copy link
Member

Mossaka commented Sep 2, 2023

Need a rebase

@ereslibre ereslibre force-pushed the bump-wws branch 2 times, most recently from 1b60a94 to 7809bce Compare September 4, 2023 14:26
@ereslibre
Copy link
Contributor Author

Need a rebase

Thanks @Mossaka! The bump looks super clean with all the changes you made! I will promote the PR from Draft after a smoke test :)

@ereslibre ereslibre force-pushed the bump-wws branch 2 times, most recently from 291c052 to 0cfd64d Compare September 4, 2023 14:45
@ereslibre ereslibre marked this pull request as ready for review September 4, 2023 14:46
@ereslibre
Copy link
Contributor Author

Smoke test working fine, this is ready for review :)

Copy link
Member

@Mossaka Mossaka left a 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/$*
Copy link
Member

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.

Copy link
Contributor Author

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 :)

@ereslibre
Copy link
Contributor Author

PR updated with an explanation on the Makefile with a dedicated commit :)

@Mossaka
Copy link
Member

Mossaka commented Sep 5, 2023

@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>
@ereslibre
Copy link
Contributor Author

@ereslibre could you please sign your commits?

I'm sorry again 🙏 I forgot 🤦

I just updated the PR, both commits signed with GPG and signed off too.

@Mossaka
Copy link
Member

Mossaka commented Sep 5, 2023

Thank you! @ereslibre I will merge once the CI passes

@Mossaka Mossaka merged commit d39043e into deislabs:main Sep 6, 2023
11 checks passed
@ereslibre ereslibre deleted the bump-wws branch September 6, 2023 14:36
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.

None yet

2 participants