Handle crate deletion correctly #145
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I am not sure what the previous implementation was trying to do when crates are deleted in the upstream index, but it definitely led to some incorrect behavior.
The actual observed behavior was as follows:
sync_crates_repofetches upstream, which contains crate deletions (random example commit)sync_crates_filesnotices the deletion from the diff and stores the relative path (in this case"ru/st/rust_code_obfuscator") inremoved_crates: Vec<PathBuf>sync_crates_filesremoves the file in the mirrored repo checkout atpanamax/src/crates.rs
Line 301 in 84ca2d7
fast_forwardcallsrepo.checkout_headwhich will somehow add the newly removed file as an addition to the index. Not sure how this happens, I am not very familiar with libgit2rewrite_config_jsonultimately commits the re-added files alongsideconfig.json, which is clearly incorrect since the intention is to only touchconfig.json.This here is my attempt at trying to build a deletion logic that actually makes sense to me, where the crates is removed from the crate mirror instead of the index checkout. I am not familiar with panamax feature set so this could have unintended side effects. Also this is mostly untested. Perhaps a safer fix would be to just remove the deletion logic in
sync_crates_filesaltogether