Merged
Conversation
Closed
63b9260 to
9db54d9
Compare
true lines inserted for simple local reverting to multi-stage builds that are preferable for local development and testing
9db54d9 to
f81b001
Compare
This ensures that the LLVM versions used around are aligned
Member
Author
|
With #104 done this is now ready for review. The large changes have already been pushed to dockerhub and are thus available as chrysn/c2rust-built:focal; the only "real" change in this PR is that that is now actually used, which helps getting everyone agree on the same standard library files. (The other changes are also "real" but don't effect the builds directly, but indirectly because I uploaded :focal based on them already). On a side note, c2rust-built is more and more becoming a Debian source package (but with no chance of upstreaming due to C2Rust's idiosyncrasies, I'm not going all the way of converting it, also because we have infrastructure for Docker builds but not for Debian builds). |
Member
Author
|
Thanks! |
Contributor
|
Build succeeded: |
chrysn
added a commit
to chrysn-pull-requests/riotdocker
that referenced
this pull request
Nov 17, 2021
This drops a workaround that was necessary during evaluation of RIOT-OS#153 in favor of a more long-term usable distinction between experimentation and stable delivery, which is necessary until RIOT-OS#141 is done properly and c2rust is built in one go with the rest of this.
bors bot
added a commit
that referenced
this pull request
Nov 17, 2021
162: riotbuild: Drop :focal tag in favor of a more stable one r=kaspar030 a=chrysn This drops a workaround that was necessary during evaluation of #153 in favor of a more long-term usable distinction between experimentation and stable delivery, which is necessary until #141 is done properly and c2rust is built in one go with the rest of this. --- Cleanup change with no expected actual impact. (Tags diverge because `latest` = `for-riot` was rebuilt after #153 was merged, but any differences should be just because building is not byte-by-byte reproducible.) Co-authored-by: chrysn <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This issue tracks changes needed in the Rust installation (OK: C2Rust -- the rest should just work) when #104 is rebased and gets merged.
By building c2rust on the same Ubuntu release (focal), the friction between different installed LLVM versions will be reduced.
This
The latter is done to get us rid of the manual
apt-get install llvm-7-dev(and pray that it matches what c2rust needs) in favor of copying the .deb out of the c2rust image and getting the right build-deps automatically. (A part of building a Debian package is the tools inspecting any binaries for used shared libraries, looking up the packages they're in, and looking at that library's exported symbol versionings to get a correctly version for the dependency).Put on hold / WIP / draft until #104 is done, given that these don't need lockstepping yet.