update `rustc_index_macros` feature handling #134526

onur-ozkan · 2024-12-19T17:34:16Z

It seems that cargo can't conditionally propagate features if they were enabled by default on the target crate, but disabled with default-features = false in the current/parent crate.

Fixes #118129

It seems that cargo can't conditionally propagate features when `default-features` is set to `false`. Signed-off-by: onur-ozkan <work@onurozkan.dev>

rustbot · 2024-12-19T17:34:27Z

r? @fmease

rustbot has assigned @fmease.
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

jieyouxu · 2024-12-19T19:04:33Z

That is... weird. I'll take a look tmrw.
r? jieyouxu

RalfJung · 2024-12-20T06:31:00Z

It seems that cargo can't conditionally propagate features when default-features is set to false.

What do you mean by this?

onur-ozkan · 2024-12-20T06:47:26Z

This does not work.

RalfJung · 2024-12-20T07:05:23Z

It does seem to work sometimes though? Only particular ./x.py invocations are broken.

onur-ozkan · 2024-12-20T07:14:26Z

I couldn't see anything that could be related in bootstrap.

RalfJung · 2024-12-20T07:17:34Z

Note that if this truly does not work you'll have to also change it here

rust/compiler/rustc_abi/Cargo.toml

Lines 13 to 32 in eca1702

    
           rustc_index = { path = "../rustc_index", default-features = false } 
        
           rustc_macros = { path = "../rustc_macros", optional = true } 
        
           rustc_serialize = { path = "../rustc_serialize", optional = true  } 
        
           rustc_span = { path = "../rustc_span", optional = true } 
        
           tracing = "0.1" 
        
           # tidy-alphabetical-end 
        
           [features] 
        
           # tidy-alphabetical-start 
        
           default = ["nightly", "randomize"] 
        
           # rust-analyzer depends on this crate and we therefore require it to built on a stable toolchain 
        
           # without depending on rustc_data_structures, rustc_macros and rustc_serialize 
        
           nightly = [ 
        
               "dep:rustc_data_structures", 
        
               "dep:rustc_feature", 
        
               "dep:rustc_macros", 
        
               "dep:rustc_serialize", 
        
               "dep:rustc_span", 
        
               "rustc_index/nightly", 
        
           ]

and in the dozen other places where we do the same thing.

I think the first step would be to reproduce this in a minimal example and file a cargo issue, if this is really the problem.

onur-ozkan · 2024-12-20T07:21:27Z

Yeah, I saw those too. This PR was kind of a hotfix for the problem you experienced to improve the developer experience. I think it's worth filing an issue on the cargo side, will do that today.

RalfJung · 2024-12-20T09:08:09Z

Hm yeah this PR does fix that problem... I have no idea why, given that the same pattern is still used in a whole bunch of other places.

jieyouxu · 2024-12-20T09:31:16Z

I think it's worth filing an issue on the cargo side, will do that today.

I think we probably should wait until we hear back from T-cargo before merging this (even as a hotfix)... I don't quite understand this current behavior either.

jieyouxu · 2024-12-20T11:07:17Z

Ok, I did some more testing. This PR's changes notably still passes tests/run-make/rustc-crates-on-stable, i.e. the compiler crates that need to compile on stable still continues to compile on stable (more or less). AFAICT it also preserves the behavior of the non-nightly-ness of rustc_index_macros as a dep for rustc_index. I tried something different like making rustc_index_macros truly optional, and while that also addresses the original report it also regresses tests/run-make/rustc-crates-on-stable (as expected).

onur-ozkan · 2024-12-20T17:22:42Z

Yeah, I saw those too. This PR was kind of a hotfix for the problem you experienced to improve the developer experience. I think it's worth filing an issue on the cargo side, will do that today.

I tried to create a simple reproduction in a new cargo workspace but I couldn't reproduce it. I am not sure what is the special case in compiler tree, but I don't want to spend more time on this since there are alternative x invocations and we already have a fix (which is this PR) for the broken one.

jieyouxu · 2024-12-20T17:24:17Z

Yeah, I also tried to make a smaller repro but failed.

@bors r+ rollup=iffy

bors · 2024-12-20T17:24:20Z

📌 Commit e151148 has been approved by jieyouxu

It is now in the queue for this repository.

…ieyouxu update `rustc_index_macros` feature handling It seems that cargo can't [conditionally propagate features](https://github.com/rust-lang/rust/blob/214587c89d527dd0ccbe1f2150c737d3bdee67b0/compiler/rustc_index/Cargo.toml#L20) if they were enabled by default on the target crate, but disabled with `default-features = false` in the current/parent crate. Fixes rust-lang#118129

Rollup of 6 pull requests Successful merges: - rust-lang#131072 (Win: Use POSIX rename semantics for `std::fs::rename` if available) - rust-lang#134325 (Correctly document CTFE behavior of is_null and methods that call is_null.) - rust-lang#134526 (update `rustc_index_macros` feature handling) - rust-lang#134581 (Bump Fuchsia toolchain for testing) - rust-lang#134607 (on pair → on par) - rust-lang#134608 (Move test rust-lang#93775 to crashes) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#131072 (Win: Use POSIX rename semantics for `std::fs::rename` if available) - rust-lang#134325 (Correctly document CTFE behavior of is_null and methods that call is_null.) - rust-lang#134526 (update `rustc_index_macros` feature handling) - rust-lang#134581 (Bump Fuchsia toolchain for testing) - rust-lang#134607 (on pair → on par) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#134526 - onur-ozkan:nightly-feat-rustc, r=jieyouxu update `rustc_index_macros` feature handling It seems that cargo can't [conditionally propagate features](https://github.com/rust-lang/rust/blob/214587c89d527dd0ccbe1f2150c737d3bdee67b0/compiler/rustc_index/Cargo.toml#L20) if they were enabled by default on the target crate, but disabled with `default-features = false` in the current/parent crate. Fixes rust-lang#118129

update rustc_index_macros feature handling

e151148

It seems that cargo can't conditionally propagate features when `default-features` is set to `false`. Signed-off-by: onur-ozkan <work@onurozkan.dev>

rustbot assigned fmease Dec 19, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 19, 2024

rustbot assigned jieyouxu and unassigned fmease Dec 19, 2024

onur-ozkan mentioned this pull request Dec 20, 2024

Bootstrap sometimes fails to build miri because of "nightly" feature confusion #118129

Closed

This comment was marked as off-topic.

Sign in to view

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 20, 2024

jieyouxu mentioned this pull request Dec 20, 2024

Rollup of 6 pull requests #134571

Closed

DianQK mentioned this pull request Dec 21, 2024

Rollup of 6 pull requests #134613

Closed

matthiaskrgr mentioned this pull request Dec 21, 2024

Rollup of 5 pull requests #134631

Merged

bors merged commit e461a3f into rust-lang:master Dec 22, 2024
6 checks passed

rustbot added this to the 1.85.0 milestone Dec 22, 2024

onur-ozkan deleted the nightly-feat-rustc branch December 22, 2024 05:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update `rustc_index_macros` feature handling #134526

update `rustc_index_macros` feature handling #134526

onur-ozkan commented Dec 19, 2024 •

edited

Loading

rustbot commented Dec 19, 2024

jieyouxu commented Dec 19, 2024

RalfJung commented Dec 20, 2024

onur-ozkan commented Dec 20, 2024

RalfJung commented Dec 20, 2024

onur-ozkan commented Dec 20, 2024

RalfJung commented Dec 20, 2024

onur-ozkan commented Dec 20, 2024

RalfJung commented Dec 20, 2024

jieyouxu commented Dec 20, 2024

This comment was marked as off-topic.

jieyouxu commented Dec 20, 2024 •

edited

Loading

onur-ozkan commented Dec 20, 2024

jieyouxu commented Dec 20, 2024

bors commented Dec 20, 2024

update rustc_index_macros feature handling #134526

update rustc_index_macros feature handling #134526

Conversation

onur-ozkan commented Dec 19, 2024 • edited Loading

rustbot commented Dec 19, 2024

jieyouxu commented Dec 19, 2024

RalfJung commented Dec 20, 2024

onur-ozkan commented Dec 20, 2024

RalfJung commented Dec 20, 2024

onur-ozkan commented Dec 20, 2024

RalfJung commented Dec 20, 2024

onur-ozkan commented Dec 20, 2024

RalfJung commented Dec 20, 2024

jieyouxu commented Dec 20, 2024

This comment was marked as off-topic.

jieyouxu commented Dec 20, 2024 • edited Loading

onur-ozkan commented Dec 20, 2024

jieyouxu commented Dec 20, 2024

bors commented Dec 20, 2024

update `rustc_index_macros` feature handling #134526

update `rustc_index_macros` feature handling #134526

onur-ozkan commented Dec 19, 2024 •

edited

Loading

jieyouxu commented Dec 20, 2024 •

edited

Loading