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

Fix #864 (fetching deleted/moved branches in colocated repos) #1600

Merged
merged 7 commits into from
May 18, 2023

Conversation

ilyagr
Copy link
Contributor

@ilyagr ilyagr commented May 11, 2023

Fixes #864 ("In a git colocated repo, commits are not rebased off of a deleted/moved branch")

Update: Some technical details from the commit description:

This bug concerns the way import_refs that gets called by fetch computes
the heads that should be visible after the import.

Previously, the list of such heads was computed before local branches were
updated based on changes to the remote branches. So, commits that should have
been abandoned based on this update of the local branches weren't properly
abandoned.

Now, import_refs tracks the heads that need to be visible because of some ref
in a mapping keyed by the ref. If the ref moves or is deleted, the
corresponding heads are updated.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • (n/a) I have updated the config schema (src/config-schema.json)
  • I have added tests to cover my changes

@ilyagr ilyagr force-pushed the fetchdel branch 10 times, most recently from 5ce7dca to 0caa03b Compare May 13, 2023 23:44
@ilyagr ilyagr marked this pull request as ready for review May 13, 2023 23:47
@ilyagr
Copy link
Contributor Author

ilyagr commented May 14, 2023

I added some no-op commits that mildly polish import-refs/export-refs, to preserve some minor improvements I made in #1487.

I'm happy to move those (the last three commits) to a separate PR. There will likely be a separate PR in any case to add some tests I added in #1487.

@ilyagr ilyagr force-pushed the fetchdel branch 7 times, most recently from d444bcc to 522f553 Compare May 14, 2023 05:11
@ilyagr
Copy link
Contributor Author

ilyagr commented May 14, 2023

I decided to change the "root cause" test to make it more clear why this crazy bug only affects colocated repos. I'm also reordering the commits a bit. (Update: this is all done)

@ilyagr
Copy link
Contributor Author

ilyagr commented May 16, 2023

I'll update this to address #864 (comment) as well.

@ilyagr ilyagr marked this pull request as draft May 16, 2023 04:51
ilyagr added 2 commits May 16, 2023 17:41
@ilyagr ilyagr changed the title Fix #864 Fix #864 (fetching deleted/moved branches in colocated repos) May 17, 2023
@ilyagr ilyagr marked this pull request as ready for review May 17, 2023 01:16
@ilyagr
Copy link
Contributor Author

ilyagr commented May 17, 2023

I believe it's all fixed. 🎉

@ilyagr ilyagr force-pushed the fetchdel branch 3 times, most recently from 2842632 to b297489 Compare May 17, 2023 02:44
Copy link
Member

@martinvonz martinvonz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

lib/tests/test_git.rs Show resolved Hide resolved
lib/tests/test_git.rs Outdated Show resolved Hide resolved
lib/tests/test_git.rs Outdated Show resolved Hide resolved
lib/src/git.rs Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
lib/src/git.rs Show resolved Hide resolved
@ilyagr ilyagr force-pushed the fetchdel branch 2 times, most recently from ea27bd6 to 8e14835 Compare May 17, 2023 22:32
ilyagr added 5 commits May 17, 2023 17:37
jj-vcs#864)

This bug concerns the way `import_refs` that gets called by `fetch` computes
the heads that should be visible after the import.

Previously, the list of such heads was computed *before* local branches were
updated based on changes to the remote branches. So, commits that should have
been abandoned based on this update of the local branches weren't properly
abandoned.

Now, `import_refs` tracks the heads that need to be visible because of some ref
in a mapping keyed by the ref. If the ref moves or is deleted, the
corresponding heads are updated.

Fixes jj-vcs#864
…s (no-op)

This is supposed to make `import_refs` and `export_refs` a little less prone to typos
This is supposed to make `export_refs` a little more readable.
@ilyagr ilyagr enabled auto-merge (rebase) May 18, 2023 00:50
@ilyagr ilyagr merged commit db8fcf9 into jj-vcs:main May 18, 2023
@ilyagr ilyagr mentioned this pull request May 18, 2023
2 tasks
@ilyagr ilyagr deleted the fetchdel branch May 18, 2023 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In a git colocated repo, commits are not rebased off of a deleted/moved branch
2 participants