Skip to content

More CI Fixes#119

Merged
pchickey merged 8 commits into
masterfrom
pch/docker_tweaks
Apr 8, 2020
Merged

More CI Fixes#119
pchickey merged 8 commits into
masterfrom
pch/docker_tweaks

Conversation

@pchickey

@pchickey pchickey commented Apr 7, 2020

Copy link
Copy Markdown
Collaborator

The Dockerfile bitrotted because it isn't checked in CI.

Sam fixed the cmake version issue in #120. This PR adds a job to run docker_build.sh (in addition to the regular native ubuntu-latest build) in CI.

It also re-enables Windows.

If successful, we'll merge this PR and tag the result as version wasi-sdk-10. We'll leave a note on wasi-sdk-9 that it has been superseded by this new release. And we'll use the artifacts from the CI for the create: tag trigger as the release binaries.

Comment thread Dockerfile Outdated
# Need a more recent cmake than ships with stretch
RUN curl -sSfL \
https://github.com/Kitware/CMake/releases/download/v3.17.0/cmake-3.17.0-Linux-x86_64.tar.gz \
| tar xzf -

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it the use of -B build/llvm the in the Makefile that doesn't work. Maybe we can find a fix for that instead of this extra download?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like that solution is to use the -S and -B together maybe:

cmake [<options>] -S <path-to-source> -B <path-to-build>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually that didn't fix it for me..

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah it looks like the -B flag was added in 3.13: https://cmake.org/cmake/help/v3.13/release/3.13.html

Strange the older version silently ignore it :)

Anyway I think it easy enough to avoid the -B flag rather then downloading custom cmake? What do you think?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'll convert the makefile to cd to the build directory rather than use -B. Its disappointing that cmake chose to deploy a new flag that old versions ignore, though :(

@pchickey

pchickey commented Apr 7, 2020

Copy link
Copy Markdown
Collaborator Author

With this fix I have a build of wasi-sdk, unfortunately it is version 9.2ga2aa6e93c5b7
instead of 9.0 :/. We could 1. patch version.sh to lie and call this 9.0 anyway, or 2. merge this branch and tag 10.0. I don't have a strong feeling one way or the other.

This problem is on me for not checking if the docker build had bitrotted before tagging the release.

@sbc100

sbc100 commented Apr 7, 2020

Copy link
Copy Markdown
Member

How about tagging 9.1 and abandon 9?

@sbc100

sbc100 commented Apr 7, 2020

Copy link
Copy Markdown
Member

10 also sounds fine if your prefer.

Could we also then use the build artifact from github as the linux binary that gets uploaded?

@pchickey

pchickey commented Apr 7, 2020

Copy link
Copy Markdown
Collaborator Author

To use an artifact from github we have to tag the branch, and then we can't do the squash-and-merge or rebase-and-merge. So, we can change the repo rules for just releases maybe?

@pchickey pchickey force-pushed the pch/docker_tweaks branch from e46656f to 5246c2c Compare April 7, 2020 20:32
@pchickey pchickey changed the title Docker build tweaks More CI Fixes Apr 7, 2020
@pchickey pchickey requested a review from sbc100 April 8, 2020 01:44

@sbc100 sbc100 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

still lgtm

@pchickey

pchickey commented Apr 8, 2020

Copy link
Copy Markdown
Collaborator Author

unfortunately I still had a typo in the deb script, hopefully this is the last tweak...

@pchickey

pchickey commented Apr 8, 2020

Copy link
Copy Markdown
Collaborator Author

Finally got the deb uploading, and I've QA'd it by running the lucet test suite against it. Gonna merge and tag wasi-sdk-10!

@pchickey pchickey merged commit 6b453df into master Apr 8, 2020
@pchickey pchickey deleted the pch/docker_tweaks branch April 8, 2020 22:10
kildom pushed a commit to kildom/clang-wasi-port that referenced this pull request Jul 14, 2021
This works around [this bug], which manifests as

```
error: could not remove 'setup' file: 'C:\Users\VssAdministrator\.cargo\bin/rustup-init.exe'
info: caused by: Access is denied. (os error 5)
```

as suggested by [this comment].

[this bug]: https://github.com/microsoft/azure-pipelines-image-generation/issues/1224
[this comment]: WebAssembly/wasi-libc#118 (comment)
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