Get current target config from --print=cfg
#111472
Conversation
|
(rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
A-testsuite
This comment has been minimized.
This comment has been minimized.
|
This is useful for cg_clif too. It currently has to explicitly filter out |
djkoloski
force-pushed
the
compiletest_cfg_current_target
branch
from
989e821 to
6aea01b
Compare
This comment has been minimized.
This comment has been minimized.
Compiletest was switched to querying all targets using `--print=all-target-specs-json` and `--print=target-spec-json` in rust-lang#108905. This unintentionally prevented codegen flags like `-Cpanic` from being reflected in the current target configuration. This change gets the current compiletest target config using `--print=cfg` like it was previously while still using the faster prints for getting information on all other targets. Fixes rust-lang#110850.
djkoloski
force-pushed
the
compiletest_cfg_current_target
branch
from
6aea01b to
9dffb52
Compare
|
Doesn't this defeat the point of |
|
|
|
Architecturally, I think it makes sense to have separate operations to get all default configurations for supported targets, and to get the very specific configuration for a specific target given some additional flags. For example, if we passed some target RUSTCFLAGS while getting all configurations, should we apply those flags to every target? What if we wanted to only apply to some targets? My opinion is that it seems more straightforward to keep the operations separate. |
|
Makes sense. @bors r+ |
bors
added
S-waiting-on-bors
…target, r=compiler-errors Get current target config from` --print=cfg` Compiletest was switched to querying all targets using `--print=all-target-specs-json` and `--print=target-spec-json` in rust-lang#108905. This unintentionally prevented codegen flags like `-Cpanic` from being reflected in the current target configuration. This change gets the current compiletest target config using `--print=cfg` like it was previously while still using the faster prints for getting information on all other targets. Fixes rust-lang#110850. `@jyn514` might be interested in reviewing since they commented on the issue. cc `@tmandry` since this issue is affecting Fuchsia.
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (72b2716): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 641.794s -> 641.487s (-0.05%) |
…inking, r=davidtwco Make compiletest aware of targets without dynamic linking Some parts of the compiletest internals and some tests require dynamic linking to work, which is not supported by all targets. Before this PR, this was handled by if branches matching on the target name. This PR loads whether a target supports dynamic linking or not from the target spec, and adds a `// needs-dynamic-linking` attribute for tests that require it. Note that I was not able to replace all the old conditions based on the target name, as some targets have `dynamic_linking: true` in their spec but pretend they don't have it in compiletest. Also, to get this to work I had to *partially* revert rust-lang#111472 (cc `@djkoloski` `@tmandry` `@bjorn3).` On one hand, only the target spec contains whether a target supports dynamic linking, but on the other hand a subset of the fields can be overridden through `-C` flags (as far as I'm aware only `-C panic=$strategy`). The solution I came up with is to take the target spec as the base, and then override the panic strategy based on `--print=cfg`. Hopefully that should not break y'all again.
Compiletest was switched to querying all targets using
--print=all-target-specs-jsonand--print=target-spec-jsonin #108905. This unintentionally prevented codegen flags like-Cpanicfrom being reflected in the current target configuration. This change gets the current compiletest target config using--print=cfglike it was previously while still using the faster prints for getting information on all other targets.Fixes #110850.
@jyn514 might be interested in reviewing since they commented on the issue.
cc @tmandry since this issue is affecting Fuchsia.