Add a config setting to disable the 'test' cfg in specified crates #9227
Conversation
|
Actually marking this as a draft because per feedback from @flodiebold, it should probably be a config setting we can set per workspace. |
|
Curious, what breaks if we comment out that cfg(not(test)) in rust-lang/rust? Due to this line, this cfg shouldn't have any effect I would think? Can we use this info from cargo.toml to not apply |
I think we should be able to, there's a |
|
This would also be fixed if we correctly created all dependencies without |
The thing I'm concerned about with this is that it would have unintended side effects: currently you can work in a workspace with r-a and work on tests of arbitrary crates within it. I think (?) if we were to do this, it would disable tests in some/all of those crates, breaking this use case. The compiler's behaviour is not necessarily the most user friendly in an IDE context. Setting cfg(test) in workspace crates only wouldn't work either, because sometimes you are working in a workspace with a cfg(test) bomb. I concur with @flodiebold that probably the only way to do this properly is to analyze multiple times or to add a config option that overrides cfgs per crate to allow us to fix the ones that are broken by the current strategy. |
lf-
changed the title
|
Made it a config option, which conveniently also fixes some other bugs on the tracker about other crates that were doing this. |
|
Cannot reproduce the CI failure, guessing it's spurious. :/ |
|
Uh, that looks bad. bors try |
|
bors try |
If you are opening libcore from rust-lang/rust as opposed to e.g. goto definition from some other crate which would use the sysroot instance of libcore, a `#![cfg(not(test))]` would previously have made all the code excluded from the module tree, breaking the editor experience. This puts in a slight hack that checks for the crate name "core" and turns off `#[cfg(test)]`.
Fixes crates which vanish when the 'test' cfg atom is set. Fix rust-lang#7243. Fix rust-lang#9203. Fix rust-lang#7225.
|
Rebased for the 1.53 fix. Good to go. |
There was a problem hiding this comment.
bors r+
Observation: workspace.rs seem to acquire more and more special cases, and the code there is pretty horrible. What's concerning is that this functionality is essentially untested. I fear that one day we'll have to split it into IOing and data wrangling parts, and write unit-tests for data wrangling...
Ideally we'd want to do this on the VFS anyway, right? That should make it more testable as well... |
|
Yeah, VFS is in a similar situation, although it's more stable in practice! |
This took some time, but now we have 1 (one) tests for this: |
If you are opening libcore from rust-lang/rust as opposed to e.g.
goto definition from some other crate which would use the sysroot
instance of libcore, a
#![cfg(not(test))]would previously have madeall the code excluded from the module tree, breaking the editor
experience.
Core does not need to ever be edited with
#[cfg(test)]enabled,as the tests are in another crate.
This PR puts in a slight hack that checks for the crate name "core" and
turns off
#[cfg(test)]for that crate.Fixes #9203
Fixes #9226