search

panamax-rs / panamax

Mirror rustup and crates.io repositories, for offline Rust and cargo usage.
Apache License 2.0
443 stars 46 forks source link

Missing crates not downloaded when syncing mirror #64

Closed ricked-twice closed 2 years ago

ricked-twice commented 2 years ago

Hi there :wave: ! First of all, well done for your work on panamax !

So, the issue here has already been highlighted in others issues (same author):

I opened a pull request in January to add a feature to panamax allowing users to check for missing crates in the synced mirror PR#57. The check is not fetching the crates.io-index, but just looking at the the local index state.

I closed it as I had no feedback from you. But I decided to still open an issue about it.

I couldn't find a way to trigger the problem using panamax, but I decided to manually delete some crates from my local mirror. Here are the steps I did (from my panamax's fork):

  1. Removed /path/to/my/mirror/crates/fa/st/fastrand/ and /path/to/my/mirror/crates/ri/ng/ring/0.16.20/
  2. Ran cargo run -- verify /path/to/my/mirror. The manually deleted crate are spotted by the check.
  3. Ran cargo run -- sync /path/to/my/mirror. Everything went find according to the execution's output.
  4. Ran cargo run -- verify /path/to/my/mirror. The manually deleted crate are still spotted by the check.
  5. Ran ls /path/to/my//mirror/crates/fa/st/fastrand /path/to/my//mirror/crates/ri/ng/ring/0.16.20 just to be sure. And get No such file or directory error for both directory.

There are two things highlighted by this issue:

  1. If, for some reason, some crates are deleted for the mirror, panamax is not able to download them again because of the fact that the reference to download crates is a diff between the local index (master branch) and the distant one (origin/master branch). This makes sense that the missing crates are not downloaded.
  2. That behavior does not explain why the mirror can sometimes be in an inconsistent state without any crates removal.

By exchanging about it around me, I've been put on the track that it could come from the fact that our local commit have been squashed on origin which result of local commit id to be absent from the tree. But I've looked into panamax's code and you seem to handle this case. So, I can't figure a reason of this behavior.

Anyway, I hope this issue will lead to some discussion, and if you need help, I'd be glad to re-open a PR.

Cheers :smile:

k3d3 commented 2 years ago

I closed it as I had no feedback from you. But I decided to still open an issue about it.

Oh shoot. Apparently I don't know how to use Github PRs - I had left feedback in a review, but I think because I hadn't finalized the review, the comments didn't show up.

JamesMConroy commented 2 years ago

I'm facing the same issue, specifically with cargo-make. There was a new release for that project, but panamax is unable to grab it.

ricked-twice commented 2 years ago

Sorry, just saw your answer, didn't get any notification weirdly ! Do you want me to submit a PR again or should I try to add another feature to download missing crates ?

k3d3 commented 2 years ago

No worries at all! Yes, please do go ahead and reopen the PR, and I'll make a review on it.

Thank you!

JamesMConroy commented 2 years ago

Can you just reopen #57 instead of creating a whole new pull request?

k3d3 commented 2 years ago

Sure thing!

ricked-twice commented 2 years ago

I modified the current state of the old PR regarding your comments.

Regarding download of missing crates, I think it would be better to first add a feature to download one specific crates, and then an option could be added to verify to download the missing crates after asking user validation and possibly two other options: assuming yes to user validation for downloading missing crates and maybe a dry-run option too. For example:

# Would ask for user validation before download if any missing crates
panamax verify --fix /path/to/mirror
# Not asking anything here
panamax verify --fix --assume-yes /path/to/mirror
# Just checking mirror state
panamax verify --dry-run /path/to/mirror

What do you think about it?

JamesMConroy commented 2 years ago

Looks like #57 is merged. Does everyone think that resolves this issue?

k3d3 commented 2 years ago

Yep, that should give the ability for people to make sure any missed crates get downloaded, so hopefully that solves the issue for most people. I'll close the issue - feel free to bump/reopen this issue if it still persists.

ricked-twice commented 2 years ago

Just to be clear I did not added the feature yet, I was waiting for any comment about my last comment which was a suggestion and a proposition to do it, if it was interesting you.

Anyway, I'll try to do it in next few days.