Allow features prefixed with "crate:" for namespaced-features

[djc]

See description of the unstable cargo feature here:

https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#namespaced-features

I would like some mentoring on how/where to add tests for this. Ideally in my mind we'd have some kind of end-to-end test that we can publish a crate that makes use of the new explicit dependency features. I did search for tests around the features aspect, but didn't find any.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @smarnach (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[djc]

(The nightly test failure here is spurious, from rust-lang/rust#70041.)

[jtgeibel]

Hey @djc, I've added this to the agenda for our weekly meeting. Feel free to attend if you're able to make it. We don't really have a solid process in place yet for how we handle unstable features, and hopefully the team can iron some of that out soon.

Regarding tests, ignored_categories is a good template:

crates.io/src/tests/krate.rs

Lines 1816 to 1819 in 3d8c86e

	let (_, _, _, token) = TestApp::full().with_token();
	let crate_to_publish = PublishBuilder::new("foo_ignored_cat").category("bar");
	let json = token.enqueue_publish(crate_to_publish).good();

It doesn't look like PublishBuilder has a function for features yet, but it should be easy to add to the builder pattern to customize this:

crates.io/src/tests/builders.rs

Line 472 in 3d8c86e

features: HashMap::new(),

Once the crates are published, then tests usually query API endpoints (like search or crate#show) to verify the public data. In the next example, the first line also captures anon: MockAnonymousUser and calling show_crate on that returns the deserialized JSON result.

crates.io/src/tests/krate.rs

Lines 1595 to 1603 in 3d8c86e

	let (_, anon, _, token) = TestApp::full().with_token();
	// Upload a new crate
	let crate_to_publish = PublishBuilder::new("fyk_max");
	token.enqueue_publish(crate_to_publish).good();
	// double check the max version
	let json = anon.show_crate("fyk_max");
	assert_eq!(json.krate.max_version, "1.0.0");

[djc]

Hi @jtgeibel, don't think I can attend the meeting, but let me know what you come up with. In the meantime, I'll see if I can figure out a test for this functionality.

[ehuss]

Do we know what impact this will have on older cargo's?

[smarnach]

@djc I'm trying to figure out the impact of this change, so I'd like to ask for some context.

• Will implicitly generated feature names for optional dependencies be explicitly written to the index?
• Will feature names of the form crate:name be listed in the index?
• If I depend on another crate that has namespsace enabled, can I enable a feature in the from crate:name? If so, do I need to enable namespaces for my crate as well?

I'll probably have more questions until I fully get this, but let's go with these for starters. :)

[djc]

@smarnach thanks for taking a look!

• On making implicit features explicit in the index: I haven't thought about it. I'm guessing not? It seems cleaner to have a close mapping between what's in the TOML vs what's in the index.
• I would say they're not features, so no? They're feature values referring to the dependency.

The way I see it the flag namespaced-features separates the features and dependencies namespaces for the crate it's enabled in. From the outside, this should be invisible: there are features, but a depending crate should not be able to discern how they are defined.

[djc]

Do we know what impact this will have on older cargo's?

I don't! cargo has known about namespaced-features for a long time, but of course it's still behind the unstable flag. Not sure how that flag interacts with what it gets from the index though.

[smarnach]

@djc Thanks for the clarifications! Thinking about your answer for the first bullet point, I realize that implicitly defined features aren't new – we already have them. I'll double check how they are currently handled.

We probably need further changes in the crates.io codebase to make sure the names in the form crate:name never end up in the index, and are not returned by the API as feature names.

We also need clarity about backwards compatibility before we can merge this, so there is still a bit of work left. :)

[djc]

Sure, I'm happy to do the necessary work -- but am very happy to get some mentoring on what exactly the work is that needs to be done.

[smarnach]

@djc It looks like there isn't full clarity yet how exactly the feature should work. We should first clarify what exactly we want before implementing something that's hard to revert in crates.io. I'll comment on the tracking issue as well.

[pksunkara]

I am also happy to help with anything regarding this PR.

[bors]

☔ The latest upstream changes (presumably #2510) made this pull request unmergeable. Please resolve the merge conflicts.

[Turbo87]

@djc @ehuss is this PR still relevant? given that rust-lang/rfcs#3143 has proposed to stabilize the cargo feature without our API explicitly supporting it, I'm wondering if this change is really needed on our side. 🤔

[ehuss]

I think this PR can be closed. I am preparing a new one that accommodates the new syntax. I have it mostly finished, I've just been working on thoroughly testing it.

[Turbo87]

awesome, thank you! :)