-
Notifications
You must be signed in to change notification settings - Fork 557
Add missing documentation for running tests with GCC backend #2587
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
base: master
Are you sure you want to change the base?
Add missing documentation for running tests with GCC backend #2587
Conversation
Then when running tests, add the `--test-codegen-backend gcc` option. For example: | ||
|
||
```bash | ||
./x.py test tests/ui --test-codegen-backend gcc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this flag, it is not necessary to add gcc
to codegen-backends
above, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is still needed. The user could specify an external rustc sysroot that we don't build, and still ask for GCC to be used in tests.
--test-codegen-backend
=> which backend to use in tests
codegen-backends
=> which backends should be built and included in rustc's sysroot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
codegen-backends
=> which backends should be built and included in rustc's sysroot
Does that mean that the sysroot will be built with LLVM with the config above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If by sysroot you mean the standard library, then yes. To build the standard library with GCC, you'd need to set codegen-backends = ["gcc"]
.
cc @RalfJung |
setting in `bootstrap.toml`: | ||
|
||
```toml | ||
rust.codegen-backends = ["llvm", "gcc"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this can hit any contributor in any PR, I think it should be possible to run these tests in a one-shot way without permanently altering one's configuration.
I don't know what exactly altering the config in this way does, but if it means during my next PR it will build this backend, then that's not great -- it's unlikely to be relevant again then.
Please put yourself into the shoes of a rustc contributor that has nothing to do with the GCC backend, and is unlikely to encounter it for most of their PRs -- and tell that contributor what they should do to fix the PR where this weird failure they didn't expect shows up. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be done with ./x test ... --test-codegen-backend gcc --set 'rust.codegen-backends=["llvm", "gcc"]'
. A bit of a mouthful, but it will likely be copy-pasted from the docs anyway, or stored in some local helper file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somewhere on this page, in a super obvious place that the reader will find immediately when going here the first time (i.e., probably at the top), should be a command to copy-paste to run the tests. That's still not the case, isn't it? I have to copy multiple things together to get the actual command I need to use.
I don't know how much more clear I can be about optimizing this for an inexperienced contributor who doesn't want to do much work on the GCC backend, they just want to fix their PR. Or even an experienced contributor who doesn't normally touch GCC but has their tests fail in GCC every 4 months (i.e., too rarely to remember the command). Please, when writing this, put yourself in the shoes of such a contributor, not in the shoes of someone who's been working on rustc and its GCC backend for years.
I think that command is
x.py test --set 'rust.codegen-backends=["llvm", "gcc"]' tests/ui --test-codegen-backend gcc
but I am not actually sure.
1f9eb93
to
f764662
Compare
I added more examples and also cases where you don't need to update |
If you don't want to build `gcc` yourself, you also need to set: | ||
|
||
```toml | ||
gcc.download-ci-gcc = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I also have to add this to --set
to avoid waiting hours until GCC is built?
Why is true
not the default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do I also have to add this to
--set
to avoid waiting hours until GCC is built?
Yes.
Why is
true
not the default?
Good question. But not something to be discussed in here I think. 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it should really be the default for any non-dist profile. No one wants to build GCC except for the few people working on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree: rust-lang/rust#146435.
Hmm, the documentation was kind of all over the place, and it seemed to me that it was a bit hard to understand which command does what. I tried to restructure it, let me know what you think. FWIW, I think that it would make more sense to have a dedicated page for each backend, rather than a "testing with XYZ backend", but for now I think this is good enough. |
…r=GuillaumeGomez Change the default value of `gcc.download-ci-gcc` to `true` It makes sense for the vast majority of uses (rust-lang/rustc-dev-guide#2587 (comment)). r? `@GuillaumeGomez`
…r=GuillaumeGomez Change the default value of `gcc.download-ci-gcc` to `true` It makes sense for the vast majority of uses (rust-lang/rustc-dev-guide#2587 (comment)). r? ``@GuillaumeGomez``
…r=GuillaumeGomez Change the default value of `gcc.download-ci-gcc` to `true` It makes sense for the vast majority of uses (rust-lang/rustc-dev-guide#2587 (comment)). r? ```@GuillaumeGomez```
Rollup merge of #146435 - Kobzol:gcc-download-default-true, r=GuillaumeGomez Change the default value of `gcc.download-ci-gcc` to `true` It makes sense for the vast majority of uses (rust-lang/rustc-dev-guide#2587 (comment)). r? ```@GuillaumeGomez```
…meGomez Change the default value of `gcc.download-ci-gcc` to `true` It makes sense for the vast majority of uses (rust-lang/rustc-dev-guide#2587 (comment)). r? ```@GuillaumeGomez```
Since we're now going to make GCC backend mandatory for CI (rust-lang/rust#146414), we need to make it as easy as possible for contributors to be able to test their code with this backend as well.
r? @Kobzol