cargo: Fix sparse registry error on update

[jakecoffman]

We're seeing some errors like this:

error: failed to get `arrow` as a dependency of package `geoengine-datatypes v0.7.0 (/home/dependabot/dependabot-updater/dependabot_tmp_dir/datatypes)`
 
Caused by: 
 failed to load source for dependency `arrow`
 
Caused by:
 Unable to update registry `crates-io`
 
Caused by:
  usage of sparse registries requires `-Z sparse-registry`

This is due to the sparse registry we enabled in #6995. It seems this setting is required for the update to work as well as the environment variable.

[jakecoffman]

Interesting, the smoke test outputs error: the `-Z` flag is only accepted on the nightly channel of Cargo, but this is the `stable` channel so that makes me think this might actually be a bug that it is requiring the -Z flag with the environment variable enabled.

[jakecoffman]

I filed an issue with Cargo: rust-lang/cargo#11965

[jakecoffman]

The problem is I overlooked that Rust users can pin versions of the toolchain. That means we need to handle older versions that aren't on the stable release. So I'll try to reformulate this PR to add the unstable -Z flag when it's needed based on the toolchain file.

Hmm, we also need to check for older stable releases and versions that came before this feature existed at all. I wonder if there's a way to ask cargo if it supports the feature?

[jakecoffman]

This isn't a complete solution, we also need to consider a range of unstable channel releases. But this should improve our success rate with Cargo updates, so we can ship incrementally.

The -Z flags only apply to unstable channels so we can ignore older stable toolchains. They will continue to use the non-sparse protocol.

[jeffwidman]

How frequently is this problem occurring?

Given it's only cropping up on nightly/unstable, and anyone using nightly/unstable generally is quick to update their toolchains, I suspect most folks affected by this will be updating soon if they haven't already...

I'm not objecting to merging since this is already written, but any code we add is also code we have to maintain later... so I'd be inclined to simultaneously open a draft PR to remove this workaround, and consider it blocked for another 6-12 months and then pull it out.

This also gets into the territory that we've discussed for later this quarter of setting clear policy guidelines around what versions we support within different ecosystems (cc @abdulapopoola for visibility as we discuss that initiative). Eg, for Rust Cargo, in general we probably don't want to be trying to support workarounds required for nightly/unstable for very long once a feature has been merged into stable.

Two other ideas:

1. don't try to use the sparse protocol at all for nightly/unstable that are older than the stable version where the sparse protocol landed... this has a perf hit for us internally, but again I suspect the frequency is pretty darn low so it'd be okay.
2. throw an error message to the user of "you are on an older nightly/unstable, please upgrade to a stable >= "

Any reason we can't go with 1️⃣ ??

Also, my apologies if I misunderstand completely how channels work in Cargo/Rust, I've read the docs but haven't worked directly with them so I may be misunderstanding something here that limits the implementation options.

[jakecoffman]

The errors are hidden behind the HelperSubprocessFailed exception, so we can't tell exactly how many updates this is affecting.

I created a PR to gather toolchain channel data that should be helpful going forward: #7080

Consider that this feature only stabilized in January, so anyone who pinned to a nightly last year are now broken. I don't want to disable the sparse protocol and risk an incident unless we know more about who is using what channels. So, do we want to wait on this and proceed with #7080?

[jakecoffman]

Initial numbers indicate <7% of our jobs involve a toolchain, and even less of that are toolchains before 1.68, maybe ~1%?

Based on that I think we can go with recategorizing the error as a resolution error and recommend upgrading toolchains to >= 1.68.

[ilya-bobyr]

I have little understanding in how Dependabot is structured, but we are also affected by this change. And we are not on nightly.

Is it the case that Dependabot is run in a docker environment specified here: https://github.com/dependabot/dependabot-core/blob/main/cargo/Dockerfile ?
It has this line:

ENV RUSTUP_HOME=/opt/rust \
  CARGO_HOME=/opt/rust \
  CARGO_REGISTRIES_CRATES_IO_PROTOCOL=sparse \
  PATH="${PATH}:/opt/rust/bin"

CARGO_REGISTRIES_CRATES_IO_PROTOCOL=sparse forces sparse registry protocol.

Sparse registry protocol was stabilized only in Rust 1.68, and before that required the -Z sparse-registry flag.

In the same Dockerfile, the default toolchain is set to 1.68.2:

RUN curl https://sh.rustup.rs -sSf | sh -s -- -y --default-toolchain 1.68.2 --profile minimal

The cargo update command invoked by dependabot runs in the registry repo without an explicit toolchain specification (which probably makes sense):

dependabot-core/cargo/lib/dependabot/cargo/update_checker/version_resolver.rb

Lines 135 to 140 in 009b8f2

	def run_cargo_update_command
	run_cargo_command(
	"cargo update -p #{dependency_spec} --verbose",
	fingerprint: "cargo update -p <dependency_spec> --verbose"
	)
	end

But if the repository contains a rust-toolchain.toml that would use a toolchain prior to 1.68, this command would require an explicit -Z sparse-registry flag, or would fail with this error.

I think this is affecting our repo.

I guess, it does not change the stats you are seeing. But I just wanted to point out that not only people pinned to nightly are affected. But also people pinned to any stable before 1.68.

[jeffwidman]

@ilya-bobyr Yep, see #7122.

Is there a reason you can't update your toolchain to >= 1.68?

[ilya-bobyr]

We should. We combined upgrade to 1.68 with an upgrade in our nightly channel that we are also checking.
And nightly had issues in the dynamically loaded code.
We are actually working on this upgade now.
Dependabot would be another reason to rush it a bit :)

[jakecoffman]

Sorry for the trouble! Dependabot was putting too much pressure on GitHub without the sparse registry so it forced us to update. Usually we're not this aggressive about forcing recent versions.

[ilya-bobyr]

Sorry for the trouble! Dependabot was putting too much pressure on GitHub without the sparse registry so it forced us to update. Usually we're not this aggressive about forcing recent versions.

No worries, and thank you for the explanation!
After you merge the warning, it would be even easier for the users to understand that it is time to update :))