Add proc-macro to index, and new feature resolver. #8003
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
NOTE: This modifies |
There was a problem hiding this comment.
Looks good to me, thanks @ehuss! I'm tempted to go ahead and merge this since crates.io will ignore other fields we send it. In that sense it'll take awhile for this to fully come to fruition, but do you see a reason to not merge this now?
|
I think it should be OK. I'm not super confident this is the right decision, but I just can't get around the potential difficulty, risk and complexity of trying to rebuild features while packages are downloaded. The main drawback here is the slow rollout since proc-macros will need to be published with a new version to get the new behavior (and that may not be obvious to the proc-macro author, since it is the proc-macro users who care about this). I just realized I forgot to update the registry documentation, so I just pushed a fix for that. |
|
@bors: r+ Ok I think in that case let's go ahead and try to get this in the works, and we can adapt the roll-out as time progresses. |
|
📌 Commit 1c28d89 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-azure |
|
@alexcrichton I wanted to discuss a concern I had about this. If a proc-macro is published with an older Cargo where the Do you think that is enough of a concern that this is maybe a bad idea to move forward with? I was initially thinking of just "update all proc-macros in the index" to avoid that, but then I was thinking, after doing that, old Cargo's would keep putting old data into the index. So maybe that isn't such a good solution. |
|
Hm no that's actually a really good point and I would indeed be worried about that. I do think though that a "rewrite the index" strategy would still work. The main gotcha there though would be that we'd have to update crates.io to work whenever Cargo doesn't send a proc-macro field. crates.io itself would have to parse I agree though that if we can't fix this we should probably back this out to prevent the action-at-a-distance from happening. |
|
Yea, I thought about the option of having crates.io parse I'll ponder this a bit. I think the only other option is to have Cargo do feature resolution after downloading, and that would require some significant changes to how the code is structured that I would prefer to avoid. I could maybe hack together some prototypes to see how difficult it would be. Another thought I had was to have a simple pre-pass that would determine the minimum set of crates to download, and then build the unit graph. If that could be done in a relatively small amount of code, that might be another option. |
|
I do agree that the ideal would be doing feature resolution after all the downloads so we have full manifest information, but I thought that you tried that earlier and said it would be an extremely large undertaking. That being said I think it's fine to download extra crates even if we don't end up using them in a few corner cases. |
|
Another option to avoid the action-at-a-distance concern is to have the proc-macro authors opt into the new behavior - maybe by setting |
Re-implement proc-macro feature decoupling. This is essentially a rewrite of #8003. Instead of adding proc-macro to the index, it uses a strategy of downloading all packages before doing feature resolution. Then the package can be inspected for the proc-macro field. This is a fairly major change. A brief overview: - `PackageSet` now has a `download_accessible` method which tries to download a minimal set of every accessible package. This isn't very smart right now, and errs on downloading too much. In most cases it should be the same (or nearly the same) as before. It downloads extra in the following cases: - The check for `[target]` dependencies checks both host and target for every dependency. I could tighten that up a little so build dependencies only check for the host, but it would add some complexity and I wanted to get feedback first. - Optional dependencies disabled by the new feature resolver will get downloaded. - Removed the loop in computing unit dependencies where downloading used to reside. - When downloading starts, it should now show a more accurate count of how many crates are to be downloaded. Previously the count would fluctuate while the graph is being built.
Update cargo. 8 commits in 7019b3ed3d539db7429d10a343b69be8c426b576..8a0d4d9c9abc74fd670353094387d62028b40ae9 2020-03-17 21:02:00 +0000 to 2020-03-24 17:57:04 +0000 - Re-implement proc-macro feature decoupling. (rust-lang/cargo#8028) - Remove unused transitive dependencies: miniz_oxide, adler32 (rust-lang/cargo#8023) - Fix bug with -Zfeatures=dev_dep and `check --profile=test`. (rust-lang/cargo#8027) - Remove Config from CompileOptions. (rust-lang/cargo#8021) - Add `rustless.org` to documented blocklist. (rust-lang/cargo#7922) - Print colored warnings when build script panics (rust-lang/cargo#8017) - Do not supply --crate-version flag to rustdoc if present in RUSTDOCFLAGS (rust-lang/cargo#8014) - Add proc-macro to index, and new feature resolver. (rust-lang/cargo#8003)
Update cargo. 8 commits in 7019b3ed3d539db7429d10a343b69be8c426b576..8a0d4d9c9abc74fd670353094387d62028b40ae9 2020-03-17 21:02:00 +0000 to 2020-03-24 17:57:04 +0000 - Re-implement proc-macro feature decoupling. (rust-lang/cargo#8028) - Remove unused transitive dependencies: miniz_oxide, adler32 (rust-lang/cargo#8023) - Fix bug with -Zfeatures=dev_dep and `check --profile=test`. (rust-lang/cargo#8027) - Remove Config from CompileOptions. (rust-lang/cargo#8021) - Add `rustless.org` to documented blocklist. (rust-lang/cargo#7922) - Print colored warnings when build script panics (rust-lang/cargo#8017) - Do not supply --crate-version flag to rustdoc if present in RUSTDOCFLAGS (rust-lang/cargo#8014) - Add proc-macro to index, and new feature resolver. (rust-lang/cargo#8003)
This adds the "pm" field to the index so that Cargo can detect which packages contain a proc-macro without downloading the package.
The second commit builds on that to support proc-macros in the new "de-unification" of the new feature resolver. This prevents dependencies shared between proc-macros and other dependency kinds from having features unified.
cc #7915