Update replaced registry before search

[knight42]

Close #5550.

It seems that updating the replaced registry before search has not been well considered in cargo and I have to add a function to trait core::source::Source to get the replaced SourceId.

I am not sure whether this is a good design, any advice is welcome.

[rust-highfive]

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

[knight42]

@alexcrichton just a friendly ping to keep this on your radar.

[alexcrichton]

Thanks for the PR! Sorry I was away for a bit but am looking at this now. @Yanhao or @knight42 can y'all elaborate a bit on the use case here? I'm not sure how [source.replace-with] would affect other operations about searching and such?

[knight42]

@alexcrichton I'm glad that you are back now!

At present, cargo search does not respect [source.replace-with] and always updates github.com/crates.io-index before search, because it only checks [registry.index] for altered registry index.

[alexcrichton]

@knight42 the search API though isn't part of the replace-with business and is pretty specific to crates.io itself?

[knight42]

Well yes.. but cargo now needs to update the local crates.io-index repository before search, and as you know people in some countries, like China, may not access GitHub and that's why an altered registry(a mirror) is needed.

In addition, is it necessary to update the local crates.io-index repo before search? Couldn't we just search directly?

[alexcrichton]

Aha an excellent point! Hm though we don't necessarily want to stabilize the API of the registry yet, however. In that sense I wonder if this is a case where something like a proxy would be most useful? Is that possible to set up or is a dedicated registry server running inside of China?

Currently Cargo only knows about the git index, it doesn't actually know about crates.io's URL or server. A config.json in the root of the git index indicates where to find the API server, so Cargo updates it to find that server. This I feel like is a deficiency in Cargo, though. I'd prefer to implement a method where it doesn't involve first updating a git repo first!

[knight42]

Pulling the git repo through a proxy is workable, but that would require extra work on client side. Currently, crates.io has not been blocked in China, so I think we need not to set up another registry server now.

If I understand correctly, @alexcrichton you do agree that we don't need to update a git repo before search, right? If so, I could implement such a method.

[alexcrichton]

@knight42 yeah I think we should probably implement "don't update the registry before search". If you're interested in implementing that I can try to help out!

[knight42]

@alexcrichton Now if I don't update first, I cannot pass the tests, and it sounds reasonable to fetch the repo if it is missing on local machine.

What if I update a repo only if it is missing? To be specific, I would call RegistrySource::update() only if RegistrySource::config() returns an Err.

[alexcrichton]

@knight42 excactly my thinking as well! And yeah I'd only update in the case of a config access error

[knight42]

@alexcrichton Done!

[knight42]

The error seems spurious..

[alexcrichton]

Thanks! Although this looks like it's still got the code to handle replaced registries, is that still needed? Additionally, could a test be added that the index isn't updated when it's already available?

[knight42]

@alexcrichton I believe handling replaced registries is still necessary, because one may want to update the registry he specified in .cargo/config instead of github.com/crates.io-index.

As for the test, I don't know how to create a local registry with available index, does testsuite provide such a utility?

[alexcrichton]

Ah ok I think I understand yeah, I think though in that case we'd want to move this logic to the registry function to affect other commands as well?

And yeah the test suite in places like tests/testsuite/registry.rs should have various utilities for creating dummy registries and indices.

[knight42]

@alexcrichton Sorry for the delay, have been busy these days.

Now corresponding tests have been added, PTAL.

[bors]

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

[knight42]

@alexcrichton rebased

[alexcrichton]

@knight42 hm are the tests here hitting the actual crates.io registry? We try to avoid that when possible, but I think with the new logic it'll see through the redirection and go to crates.io, right?

(sorry for the delay in taking a look!)

[knight42]

@alexcrichton Of course not.

I have added two tests: search::replace_default and search::not_update, and they both replaced crates-io with dummy-registry by calling search::set_cargo_config.

[alexcrichton]

@bors: r+

Ok!

[bors]

📌 Commit 7d279fe has been approved by alexcrichton

[bors]

⌛ Testing commit 7d279fe with merge 5bb8501...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 5bb8501 to master...