bindeps: Support registry dependencies

[ehuss]

-Z bindeps currently does not fully support dependencies on packages in a registry.

The core issue is that the feature resolver needs to know which target a dependency is for (the target="..." field).
The feature resolver currently uses the dependency data from the dependency resolver, which gets its information from the index. However, the index does not contain the necessary information.

Theoretically the feature resolver could lazily download packages, but we would like to avoid that.

One point of uncertainty I have is whether or not the index should have just the target field, or if it should have all the new fields (target, artifact, lib). I think it only needs to know about target. However, I lean towards keeping all of the fields just for the sake of being complete in case we want the information in the future.

The following is an outline of what I think it will take to resolve this:

Publishing

• Add the fields to NewCrateDependency. This is the structure that is used when publishing.
  • We may also want to add the v field. (See below for discussion about updating the v field to 3.) A concern is that we don't want to force a registry to do a bunch of processing to figure out the correct value of the v field, so cargo could theoretically provide that information to simplify registry implementations.
• Beware that there is already a target field which has a different meaning. The new target field will need a different name.
• Please also bump the version of the crates-io package since this will be a semver-breaking change.
• These fields should only be populated when -Z bindeps is used.
• Publishing without -Z bindeps should be an error.

Reading the index

• Add the fields to RegistryDependency. This is the structure that is used when deserializing the index.
• The new fields should be guarded by a bump in the v field to version 3. This helps ensure that older versions of Cargo won't see these new entries when resolving since they won't know how to handle them correctly (and things may change before it is stabilized).
  • This check will need to depend on whether -Zbindeps is used. If -Zbindeps is used, then it should be > 3, otherwise it should be > INDEX_V_MAX which is 2. That ensures that cargo will only read the entries with the -Z flag passed.

Pre-download

• Cargo tries to download only the minimum of packages it needs. This is done in download_accessible. However, it needs to download before the feature resolver runs. This is primarily so that the feature resolver can know which packages are proc-macros (which is not stored in the index). The target field throws a wrench into this problem, because that can require downloading for extra targets. But we don't want to make download_accessible too complicated. It is intentionally primitive, otherwise it would need to be as complicated as the feature resolver itself. I don't know how to solve this problem. If it can be extended with only a small amount of code, that's great. Otherwise, I'd be reluctant to make it too complicated.

If we can't figure out a simple solution, then an alternative is to do a best effort in download_accessible, and then in the feature resolver download packages on an as-needed basis. I think this can essentially be done by changing this expect to a ? so that any download errors get reported. I think that should work, but it needs some thought and analysis.

Handle RustcTargetData

• The feature resolver needs to query the RustcTargetData structure for information about targets (which are collected from rustc). Unfortunately the point where RustcTargetData is created doesn't know about which target fields are needed for dependencies (it is done far too early). Some possible different solutions:
  • Make RustcTargetData support the ability to lazily add new targets. However that requires access to a Workspace, which I think may make things quite messy.
  • In resolve_ws_with_opts, take a mutable reference to RustcTargetData. After the dependency resolver runs, walk the dependency graph and collect every target that should be added and update RustcTargetData.
  • Change resolve_ws_with_opts so that instead of taking a reference to RustcTargetData, make it responsible for creating RustcTargetData. After running the dependency resolver, it can walk the graph and populate it with all the targets. It can then stuff the result into WorkspaceResolve. (I have no idea if this is workable.)

Update feature resolver?

I actually don't think there is anything to do here, just writing this to note that it should be examined to ensure it works.

Testing

• Add/update any tests related to all of the above.
• Update cargo-test-support/src/registry.rs to accommodate all of the above. It needs to generate artifact entries in the index in the same way as the above new code does. It also needs to set the v key to 3 when using artifact deps.

Update crates.io

This should be done later, closer to when we are gearing up to stabilize bindeps. Adding the fields to the index is a permanent decision, so we need to be sure it has the right design.

• Add fields to EncodableCrateDependency, this is the structure it receives from cargo.
• Add fields to Dependency, this is the structure it uses for serializing to the index.
• The v field here needs to be set to 3 if it contains a dependency with an artifact dependency. (See commentary above about possibly passing the v field in via the JSON API so that a registry does not need to process it.)
• Consider adding some input validation. For example, it may want to put limits on the lengths of strings. I don't think it needs to otherwise be very restrictive, but crates.io people may have further ideas.
• Add the appropriate tests. I think it would go somewhere in src/tests/krate/publish.rs.
• Consider whether or not the artifact information should be displayed on the website. If the crates.io team would like to track this information, then it would also need to be stored in the database (which means also doing a schema update). The model is defined around here. I am no expert in diesel, so I can't provide much more guidance here. Talk with the crates.io team to see if this is something desired.

cc @Byron

[Jasper-Bekkers]

I've been playing around a bit with -Zbindeps, and I'm hitting a panic that I suspect is related to this issue so I'm sharing it here for posterity.

thread 'main' panicked at 'activated_features for invalid package: features did not find PackageId { name: "jni", version: "0.19.0", source: "registry `crates-io`" } NormalOrDevOrArtifactTarget(Some(CompileTarget { name: "aarch64-linux-android" }))', src\tools\cargo\src/cargo\core\resolver\features.rs:318:14

In case it's not relevant, or in case anybody wants access to the zip'd up cargo workspace (just a hello world example) give me a poke and I'll post it to the relevant place.

[ehuss]

@Jasper-Bekkers I'm not sure if that is related, that's not an error I would expect. Can you open a new issue and include the zip file? Thanks!

[Jasper-Bekkers]

@ehuss Filed here #10431

[npmccallum]

cc @Byron

[fee1-dead]

I am working on this.