Support multiple registries (#213)

[woutersl]

Hi,
This is an attempt to support multiple registries. This change adds a new configuration for the specification of the additional registries, so that it is backward compatible with existing configurations.
The implementation is simply to attempt to get the data from all registries and merge them by keeping the first successful response. It is a bit brutal but it works and is a minimal change.
I tested this on my end, works like a charm with a private registry in addition to crates.io.

[serayuzgur]

Thanks for the effort I will test and publish it if everything goes well

[serayuzgur]

@BarbossHack do you have any time to test these, can you help ?

[marcus-griep-simplisafe]

Definitely interested in seeing this get merged in and completed. I too use a private registry for internal crates, and so having this tool be able to support that would be great. I'll note that, when I tried this out locally, it didn't seem to be working for me. An alternate implementation in the forked sparse-crates extension did seem to support this out of the box, though, but it doesn't have quite the same niceties that this extension has.

[marcus-griep-simplisafe]

Using sparseIndexServerURL here only serves to check crates.io twice if it fails to get the configuration value. A fallback to an empty array is more appropriate with the splat below.

Suggested change

	const otherIndexServerURLs = config.get<string[]>("crates.otherIndexServerURLs") ?? sparseIndexServerURL;
	const otherIndexServerURLs = config.get<string[]>("crates.otherIndexServerURLs") ?? [];

[woutersl]

Thanks for the feedback, indeed this was redundant, I fixed this in the new version.

[marcus-griep-simplisafe]

This doesn't really specify it, but like the logic here is, if this is the first dependency, add it to the accumulator. For all remaining items in the input, skip them, but if the only item we have is an error, then replace it with that one.

If this is the intended logic, then the entire reduce may be more simply handled by a single for loop over responses, selecting the first element that is not an error.

Suggested change

	return [responses.reduce((accumulatedDependencies, current) => {
	if (accumulatedDependencies.length === 0) {
	return current;
	}
	for (let i = 0; i !== accumulatedDependencies.length; i++) {
	if (accumulatedDependencies[i].error) {
	// this dependency failed so far, replaced by current
	accumulatedDependencies[i] = current[i];
	}
	}
	return accumulatedDependencies;
	}, []), responsesMap];
	let deps = [];
	for (let i = 0; i < responses.length; i++) {
	if (!responses[i].error) {
	deps.push(responses[i]);
	break;
	}
	}
	// If all responses were errors, at least include the first response so the error gets reported.
	if (deps.length === 0) {
	deps.push(responses[0]);
	}
	return [deps, responsesMap];

If the intended behavior is different, because we want to add multiple successful dependencies, then you current iteration is still a bit more complicated than necessary, and could be achieved by removing the break above.

[woutersl]

Thanks for the feedback! I think the reduce was not really clear as to what it does. The logic is that, at this point we have all the responses from all the servers in the responses variables. The type of responses is Dependency[][]. In the new proposed version, we first take the dependencies from the first server, that is usually crates.io. And then for the responses for other servers we accumulate by replacing the dependencies that are still errors.

[lapointexavier-pulse]

Would it break backward compatibility if indexServerURL was now accepting a comma separated list? An existing value would remain a list of 1 url and , separated would be turned into an array of urls.

Or is the issue the name (Singular indexServerURL vs. plural for otherIndexServerURLs)?

[fawadasaurus]

@woutersl

How does this work with private registries that require authentication? I have my PAT in my ~/.cargo/permissions.toml file but don't see anything in here that would use that.

[BarbossHack]

imho this MR is far from being able to be merged, unfortunately...

It's not a common "rust behavior" to have multiple "transparent registries" like that. You can only have one "main registry" (usually index.crates.io), and mutiple alternate registries. The main difference here is that all the "alternate registries" must be declared in .cargo/config.toml with a name; and when used in a Cargo.toml it MUST specify the alternate registry name if any :

[dependencies]
other-crate = { version = "1.0", registry = "my-registry" }

Instead, in this MR you check all alternate registries when it does not succeed with the main registry, but you don't care either it should be check only with the main registry or with an alternate registry (and with which one). It could lead to a lot of unwanted behaviors... (checking the version on the wrong registry, etc...)

Also, like @fawadasaurus said, it should support alternate registry authentification.

Here is how it should be implemented (imho) :

• use the following vscode config (or directly read the .cargo/config.toml) :

"crates.alternateRegistries": [
    {
        "name": "my-registry",
        "index": "https://my-intranet:8080/git/index",
        "token": "854DvwSlUwEHtIo3kWy6x7UCPKHfzCmy"
    }
],

• do NOT use the main registry (indexServerURL) to check version of dependencies that have registry = "xxx"
• check for dependencies that have registry = "my-registry" and use the matching alternate registry my-registry

What do you think ?