Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support checking for offline updates against a local registry #277

Merged
merged 2 commits into from Nov 10, 2021

Conversation

anp
Copy link
Contributor

@anp anp commented Aug 19, 2021

It seems that TempProject::find_updates calls SourceId::load to get a registry for queries, but that function doesn't pay attention to registry overrides in the config. SourceConfigMap::load does take them into account, and everything else seems to work fine once the override is correctly configured.

I tested this by adding 127.0.0.1 localhost to /etc/hosts to prevent cargo from accessing the servers. I intend to add a more thorough test for CI here but wanted to make sure this seems like the right fix to maintainers beforehand. It looks like CI isn't running much in the way of tests so I'm hesitant to build out a whole harness just for this, let me know what your preference is.

EDIT: For context, the motivation here is to allow isolated CI testing of a crate update workflow tool without taking a dependency on crates.io itself.

@anp
Copy link
Contributor Author

anp commented Aug 20, 2021

I think it's possible that this would address #275 (cc @theduke) and I think may be relevant for #247 (cc @jonhoo), although I'm not certain those reports were caused by the same failure to consume config overrides for registries.

@theduke
Copy link

theduke commented Aug 21, 2021

Out of town this weekend, I can try the changes on Monday.

@theduke
Copy link

theduke commented Aug 24, 2021

This doesn't seem to change the behaviour for me.

Still getting the same error.

@tmandry
Copy link

tmandry commented Sep 28, 2021

This unblocks using cargo-outdated in CI for us, any chance we can get this merged?

@deg4uss3r
Copy link
Collaborator

@anp Thanks for this! I'd like to make sure the lint and format checks pass could you try to push a commit to trigger the checks (might have to rebase ontop of the latest upstream changes). I'm not sure why they didn't trigger here.

@tmandry
Copy link

tmandry commented Oct 11, 2021

Branch was force pushed, but needs approval for running workflows:

https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks

Useful for testing against a local registry and ensuring crates.io
isn't accessed.
Loading them directly from SourceId::load fails to account for
registry override configuration, causing a panic when querying for
updates for crates that aren't on crates.io.

Change-Id: I5e47f8f69b78a70db689b2f755a889cf942d3268
@kbknapp
Copy link
Owner

kbknapp commented Nov 10, 2021

Rebased on the latest master.

@kbknapp
Copy link
Owner

kbknapp commented Nov 10, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 10, 2021

Build succeeded:

@bors bors bot merged commit de28fd4 into kbknapp:master Nov 10, 2021
@anp anp deleted the local-registry branch January 10, 2022 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants