Add debugging utils and comments to Fuchsia scripts

[tmandry]

This should help when debugging a failure in the Fuchsia build in CI.

I plan to follow up with a PR to the testing section of the dev guide with more details, along with more improvements happening in the Fuchsia repo itself.

try-job: x86_64-gnu-integration

[rustbot]

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[tmandry]

@bors try

[tmandry]

@bors try-

[bors]

⌛ Trying commit d3bdc64 with merge 76d6219...

[tmandry]

welp.. maybe this will replace the current job..

@bors try

[bors]

⌛ Trying commit 168d23d with merge 09ce085...

[tmandry]

r? @lqd

[compiler-errors]

couple random questions

[compiler-errors]

Set it to what? Can I just put 1 there or what?

[lqd]

I think so yes, 1.

edit: yup https://github.com/rust-lang/rustc-dev-guide/pull/1989/files#diff-f33f942078cadb30448b8804b339da655c5ff43396e9e426240903e8e3cc6ba7R46

[lqd]

But mostly, should keeping the checkout be the default? In a CI context, it shouldn't matter, right? (I've described in another comment a case where removing local changes by default caused me an issue, but that happened a couple times)

It could actually be interesting to have some passthrough from the initial shell.

I don't think that the dockerfile is setup for this but e.g. KEEP_CHECKOUT=1 src/ci/docker/run.sh x86_64-gnu-integration --dev could be one of the best ways to get a workable shell (after the initial integration.git checkout, which this very command wouldn't do the first time it's run, IIRC).

[tmandry]

Any nonempty string will work, but adding 1 in the comment so it's less confusing.

[tmandry]

In a CI context you want to force the checkout to a known state before building. If you think it would be nicer, I could make this script accept a --keep-checkout flag to use inside the dev shell (or invert it and have the docker script pass an --overwrite flag or something). But with that you'd have to be careful not to run the full thing with run.sh or it will overwrite your local checkout, which is why I think it might be nicer to use the workflow of modifying this script directly.

[lqd]

I mostly meant that in our CI context the builder should be starting from scratch without an existing checkout to keep IIUC.

It could be an interesting change in the future, but as long as we document this, as you've done in the script and in the docs PR, then it should likely be fine for now.

[lqd]

Cool, thanks for this!

I don't know how much back-and-forth you want to do on this PR, vs iterative improvements on our CI scripts and documentations, and fuchsia's "build from rust CI" setup, but apart from the comments I left here, and what you've added in rust-lang/rustc-dev-guide#1989, here's a list I remember having issues with:

As non-fuchsia engineers, we'll unfortunately need some hand-holding with the build system. We don't know what we can do with fx, if we're to use it or not, how common cargo use-cases translate, etc. Nor do we know the actual fuchsia components to build, or how to discover them, when an issue appears in one of them. Some description about all this would be nice. Otherwise, we're kind of randomly mixing paths and names found in some "build.gn" files, fx expectedly not finding the component, but also not saying which components Levenshtein-match what was requested.

The first thing you need to do when debugging an issue in the builder is to remove --quiet from https://fuchsia.googlesource.com/fuchsia/+/refs/heads/releases/canary/scripts/rust/build_fuchsia_from_rust_ci.sh#118. It's understandable that we don't want to show huge logs for CI (though maybe it's not that costly I don't know, and am not on t-infra), but it would be helpful for people debugging issues to have some output locally. What could work is that only our CI workflows add the --quiet, so that local use-cases with src/ci/docker/run.sh x86_64-gnu-integration would not themselves hide fuchsia's logs.

(Actually, maybe the first thing is to setup one's local rustc checkout to even run the CI docker scripts in general, where IIUC the most common initial hurdle is that some submodules need to be initialized and checked out. CI does this automatically but it's usually not the case for developers).

I also needed a way to change rustc's arguments when building a crate -- otherwise I had to introduce an error in fuchsia code, to extract the command-line used and change it, and stop using the build system afterwards. I don't know if it's possible to with fx (e.g. if I can use RUSTFLAGS, or if there is some equivalent to the cargo check + cargo rustc combination to only override a leaf crate's arguments. I expect this to be very common in practice, and any way to make this easy will be helpful. Maybe some documentation on how to do this today would also be enough, for example.

This is more a QOL thing, but I noticed a few things being built which were surprising, but surely useful for you. In any case, maybe it'd make builds slightly faster to not have:

• 3 targets (x64, fuchsia, wasm, -- w/ fuchsia being the default for the build when maybe the most useful one locally is the host target, so that artifacts can be used from outside the docker container),
• or the cranelift backend being built (I'm almost positive I saw some cg_clif build logs even though only cg_llvm seems to be setup),
• and checking which stage is built and used. Here I expect also some of suboptimal bootstrap + tools stage versioning mismatch to come into play. To overapproximate the situation, and I don't know that it really applies I'd need to check, here's an example: if you build "stage 2 rustdoc", it will build the stage 2 compiler + std, but actually link with stage 1. Fuchsia is using clippy, and maybe it works similarly. If that's the case, then avoiding building an unused stage 2 would be nice. Though avoiding the need to build clippy in the first place could also be good for people working on the compiler I guess, e.g. if locally we could choose to use an fx equivalent of cargo check instead of fx clippy.

(sorry for the long brain dump)

[lqd]

But mostly, should keeping the checkout be the default? In a CI context, it shouldn't matter, right? (I've described in another comment a case where removing local changes by default caused me an issue, but that happened a couple times)

It could actually be interesting to have some passthrough from the initial shell.

I don't think that the dockerfile is setup for this but e.g. KEEP_CHECKOUT=1 src/ci/docker/run.sh x86_64-gnu-integration --dev could be one of the best ways to get a workable shell (after the initial integration.git checkout, which this very command wouldn't do the first time it's run, IIRC).

[tmandry]

This feedback is really helpful, thanks @lqd. I'm incorporating your feedback into this PR and the docs I'm writing in rust-lang/rustc-dev-guide#1989 as much as I can, and also writing down some future improvements we can make in our bug tracker.

It's understandable that we don't want to show huge logs for CI (though maybe it's not that costly I don't know, and am not on t-infra), but it would be helpful for people debugging issues to have some output locally. What could work is that only our CI workflows add the --quiet, so that local use-cases with src/ci/docker/run.sh x86_64-gnu-integration would not themselves hide fuchsia's logs.

I think I'll just remove this flag from the upstream build script.

[lqd]

Great, thank you.

This looks good to me. @bors r+

[bors]

📌 Commit 63ba3f3 has been approved by lqd

It is now in the queue for this repository.