I'm approaching the end of my ideas of how to make this build; the Compiling cargo v0.44.1 ran even into a 30' timeout. Maybe that's really too short as I had to put the travis_wait command around all of the docker build, trying once with a 45' timeout and gonna stop wasting build time if even that doesn't work out.

This really is a timeout issue; I now have a machine where I can run this without timeouts, and it does terminate (albeit with an error I'm just tracking further).

On the more general side, are we OK with increasing the build time by as much as this (don't have numbers, but this might double the build times), with hacks to extend the timeout? (Single Cargo crate compilation is simply nothing too verbose, and the travis_wait doesn't quite cut it; a du-based custom watcher (like "have a background process that prints directory sizes every five minutes; as long as it's still going that will increase") could do, but will we stay on travis anyway? I don't know much about that tool as I'm using GitLab for my projects, but the builds show a deprecation / migration warning.)

Hm, seems I had the wrong travis log open; the current version builds locally, and the latest build before this push (https://travis-ci.org/github/RIOT-OS/riotdocker/builds/765744396) indicates the timeout extension does work.

So if things go green here, the main question to remain is whether we want to have the increased build time and image size; I think the benefits of running Rust on RIOT in CI are worth it, but I'm obviously biased ;-)

… and rebased with the time-saving do-not-merge commit removed because now the builds run through the Docker image creation, and all steps in there are required to make the later hello-world build tests run through.

So if things go green here, the main question to remain is whether we want to have the increased build time and image size; I think the benefits of running Rust on RIOT in CI are worth it, but I'm obviously biased ;-)

I might be similarly biased, but having Rust is definitely worth a lot. I think we can cover up the increased image size ("giant" will still be "giant" afterwards), but, can we mitigate the build time by building c2rust externally?

c2rust should end up being a single binary, with some runtime dependencies (libllvm, ...). If we carve out c2rusts building into it's own Docker container, we can rebuild that only if c2rust changes, and then re-use the artifact for the build of riotdocker.
Maybe move that into another repository?

I've had immunant/c2rust#326 open for some time on that topic. Options seem to be:

• Have a Docker build from the same image (we're on bionic) that can be merged in
• Have generally usable Linux binaries produced by c2rust that can just be wgot and installed by placing them into the container.

I have a personal preference for the latter, and am exploring that road; I'd welcome anyone familiar with Docker workings and customs to explore the other road.

Hm, they do already provide a docker image; that route may actually be easier even without good familiartiy with Docker. Playing around...

Next iteration: Turns out the image provided by immunant only contains everything required for building and testing c2rust and not c2rust itself. However, that makes it rather straightforward to create one where it's built too, and (until my two pending PRs there are merged) we need a customized build anyway.

The current setup uses one intermediate image (also built in here by Travis) based on the immunant image that just builds "our" c2rust; that image is added as a dependency of the riotbuild image and the single c2rust binary is copied over. It ran through locally, so if anything runs afoul I may need to introduce travis_waits again…

With the latest fixes, this can now be now tested comprehensively by running, inside the produced riotdocker (at least with what I'm producing locally):

# git clone https://github.com/chrysn-pull-requests/RIOT.git -b rust-application
# cd RIOT/examples/rust-hello-world/
# make all term

(I assume it doesn't make much sense to include a test in the code at this stage where RIOT-OS/RIOT#16274 is not merged yet; testing the fundamentals manually, then testing that PR based on Rust support in riotdocker, and at some point later maybe adding the rust-hello-world example to the make tests.)

With the Travis Docker images, I'm still in trouble: There's a travis run for building the c2rust-built docker image now, and riotdocker depends on that, but somehow fails to pull it in unlike the static-test-tools one built just the same way a line above. I tried with 49c3326 but that wasn't it -- any clues?

With the Travis Docker images, I'm still in trouble: There's a travis run for building the c2rust-built docker image now, and riotdocker depends on that, but somehow fails to pull it in unlike the static-test-tools one built just the same way a line above. I tried with 49c3326 but that wasn't it -- any clues?

could it be that because of the --pull in docker build --pull -t riotdocker riotbuildthe riotbuild run always tries to pull from docker hub, and this just works for the other images because there's a version there (pushed earlier)? This would mean the current workflow is broken. @aabadie do you have an idea?

I think the --pull should not be used for riotbuild, but the base images.

That --pull may even have entered the system inadvertedly when some Rotzbua committed unrelated changes in ea042a7, trying without...

[edit: The --pull was discussed just not mentioned in the commit message; https://github.com//pull/81 sounds like it made sense at the time; possibly the right course of action is really to just remove the --pull for the PR evaluation, then upload an initial riot/c2rust image to dockerhub and then drop the --pull-removing commit before merging].

All running fine through the changed parts up to the point when the global timeout reaps the job. What can we do about that? Build our c2rust in a separate repo? Or just build and upload one (it's a 2-liner Dockerfile) out of CI and publish that?

bors try

bors ping

pong

try

Build failed:

• continuous-integration/travis-ci/push

Hm, bors runs into the same timeout. It's about 20' for the C2Rust compilation step, and in total this exceeds the 50' limit.

Splitting it up in different repos would be the obvious option, but that'll mean that changes in C2Rust build repo won't be CI-tested with the rest until they're merged and published. Then again, the risk is from breaking changes down at C2Rust anyway, and they don't CI with us (how should they with every downstream), so meh. I'm preparing such a step by publishing what's now in c2rust-built as chrysn/c2rust-built in preparation of the same image being tagged riot/c2rust-built.

Generally speaking, how do we publish riotdocker images? Is there an automatism from it being merged, or does someone locally build and publish them at regular intervals?

Runs through now. May I rebase?

I've taken the liberty to rebase this, given that #148 made this unmergable on its own. Some of the travis workarounds could vanish with that.

I'm a tad confused as to where the information about what to build actually went, hoping to get a bit of clarity from looking at the CI output.

[edit: found it in .github/workflows/build.yml, will adapt...]

I don't know much about the github workflows involved here, and have taken the recipes from the other docker containers in something between cargo-culting and guessing-what's-needed -- relying on reviewers to judge whether what's in there makes sense.

Having the container rebuilt on every CI run seems extraneous, can we somehow make it so that "Build c2rust" is only ever executed when c2rust/Dockerfile changes? (As that clones from a git branch, if it helps, I can make that clone a named revision, to ensure that the built container is as much a function of the Dockerfile as possible, and thus use Docker's memoization).

As-good-as-it-gets for now: The cyclic dependency which riotbuild complained about is done (turned out I don't need a FROM to COPY --from), but an actual circular dependency came up when it turned out buildx (how many layers deep has all that docker stuff become?) can't parse arguments in --from.

Quoting from the comment:

+# The local prefix should be ${DOCKER_REGISTRY}, but variables are not supported
+# in this position by buildx, leading to errors like this:
+#
+# Error: buildx failed with: error: failed to solve: failed to solve with
+# frontend dockerfile.v0: failed to create LLB definition: failed to parse
+# stage name "${DOCKER_REGISTRY}/c2rust-built": invalid reference format:
+# repository name must be lowercase
+#
+# Until github supports this, the hardcoded "local" works well enough for the
+# tests, and needs to be replaced manually with "riot" when going live. Once
+# replaced and published, the ill-effect downstream users will experience is
+# that if they change their c2rust build, they'll have to adjust this line.
+# (For others, c2rust will be built but not used here).
+COPY --from=local/c2rust-built /usr/bin/c2rust

I think that's OK for now, can't fix the whole docker world (and especially the nonfree github infrastructure, not sinking time there).

This builds fine now. For it to build in deployment, I'll have to switch over from local to riot -- can do that right before going live. Once that is changed and has run with publishing once, it can stay like this even for local builds: These will then build c2rust but use the published one. Shouldn't be too bad unless one is changing something in c2rust and relying on the change at the same time.

Please review.

To recap, open question:

• (How) can we skip the c2rust build to only happen if actually triggered, eg. by a change to the respective Dockerfile?
• Any better ideas on the --from hardcoded "riot/c2rust-built" string? (If we only build c2rust sporadically, which is probably a good idea, then it'll need to stay hardcoded anyway, b/c when it's not built it can't be fetched from local).

bors try

(to see whether I can wield this magic too)

try

Build failed:

• Build and Test

OK, that failed in the expected mode; we'll need to publish a riot/c2rust-built once either manually, or just change that before merging to s/local/c2rust/, and then bors can do the final tests and merge after they pass.

To recap, open question:

Can't help with those. I agree that we hopefully won't rebuild c2rust too often.

Overall this PR looks good. One thing that bugs me though: can we pin the nightly version somehow?
I know riotdocker is not really reproducible, but installing rust/nightly will result in different rust versions daily.

@chrysn how about adding a distinct workflow for c2rust, and the use on workflow_run combined with action-workflow-run-wait@v1 so that the riotbuild action test is triggered after that build as well. And you can add:

    paths:
      - 'c2rust-built//**'

I played around with it a bit here https://github.com/fjmolinas/riotdocker/blob/master/.github/workflows/flashers_build.yml.

Converting this to a separate workflow, there's one thing I don't see how to do: How would I get the information about whether the c2rust workflow was run into the riotbuild/Dockerfile?

If the c2rust workflow was run, I'd need to set up the copy from as ${DOCKER_REGISTRY}/c2rust-built:latest (do I need to get any files around for that, or are they all in the runner's local environment?) -- but if the c2rust workflow was not run, it'd need to copy from the latest riot/c2rust-built.

Frustrated with the github worker setup, let's take this a step at a time -- with c2rust-built from a third party source (me). Let's focus on #152 first -- that's basically this but stripped down to treating c2rust-built as a third party image.

Once we're done with that (I'd guess it's pretty ready), we can resume work here to explore how to build a riot/c2rust-built on this infrastructure.

Given that the split solution of #152 Kind Of Works, I'm closing this for now; next steps are updating the c2rust build process anyway (#153) which simplifies how that (manually built / managed by me but still having its sources here) process works.

What remains (remained?) valuable from this PR is the machinery of building c2rust in here too; I don't see that happen right now (also for lack of pressure). As the branch in this PR is hard to read (needs diffing to #152 as that was squashed), I'm force-pushing a latest state here based on #152 that loses history but in return is understandable in terms of what is missing compared to there.