cfg_version
#15533
cfg_version
#15533
Conversation
rustbot
added
A-build-scripts
|
@epage I will of course take care of proper error handling, but I just want to get feedback on the initial direction.
|
| @@ -416,6 +416,7 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul | |||
| let values = cfg_map.entry(k.as_str()).or_default(); | |||
| values.push(v.as_str()); | |||
| } | |||
| Cfg::Version(..) => todo!(), | |||
There was a problem hiding this comment.
No idea what to do here
There was a problem hiding this comment.
I think the following code will be relevant:
cargo/src/cargo/ops/cargo_compile/mod.rs
Lines 495 to 511 in 47c911e
| self.t.next(); | ||
| self.eat(&Token::LeftParen)?; |
There was a problem hiding this comment.
Can you have --cfg version=hello and so need to support #[cfg(version)1.30.0)] and #[cfg(version = "hello")]?
| while let Some(&(end, ch)) = self.s.peek() { | ||
| if ch == ')' { | ||
| let mut iter = self.orig[start..end].split('.'); | ||
| let major = iter.next().unwrap().parse().unwrap(); | ||
| if let Some(minor) = iter.next().map(|s| s.parse().unwrap()) { | ||
| if let Some(patch) = iter.next().map(|s| s.parse().unwrap()) { | ||
| if iter.next().is_some() { |
There was a problem hiding this comment.
This is some fairly deep nesting. Is there an alternative?
I'm not sure what this is referring to as I don't see this in the RFC or the rustc version |
There was a problem hiding this comment.
As a stylistic thing, as you add tests, we prefer tests to be added in a commit before the change, with them passing. Your change then includes updating the tests to make sure they pass. How to apply this is dependent on the PR (and sometimes it just doesn't work). In this case, I'd write the tests without #[cfg(version)] and then add it in the follow ups.
| @@ -416,6 +416,7 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul | |||
| let values = cfg_map.entry(k.as_str()).or_default(); | |||
| values.push(v.as_str()); | |||
| } | |||
| Cfg::Version(..) => todo!(), | |||
There was a problem hiding this comment.
Should we have this behind an unstable gate or insta-stabilize once the stabilization report for the rustc side is done?
What does this PR try to resolve?
#15531