git fetch: add --rebase to rewrite upstream changes in place - #9506
git fetch: add --rebase to rewrite upstream changes in place#9506LoganDark wants to merge 1 commit into
Conversation
| let iter_changed_refs = || itertools::chain(&changed_remote_bookmarks, &changed_remote_tags); | ||
| let index = mut_repo.index(); | ||
| let missing_head_ids: Vec<&CommitId> = iter_changed_refs() | ||
| let newly_imported_ids: HashSet<CommitId> = iter_changed_refs() |
There was a problem hiding this comment.
This means jj git fetch behaves differently from jj git fetch; jj undo; jj git fetch. We'll have to generate rewrite mapping for newly-visible changes, not newly-imported.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| // We collect first so an error on a later commit doesn't leave the repo | ||
| // with a half-applied rewrite mapping. | ||
| let mut rewrites: Vec<(CommitId, CommitId)> = Vec::with_capacity(new_commits.len()); | ||
| for new_commit in new_commits { |
There was a problem hiding this comment.
I'm testing a similar change right now, and I found that we need to build {change_id: [commit_id]} mappings for both old and new candidates to handle newly-divergent and previously-divergent cases.
We'll also need to generate rewrite mappings for ancestors of bookmarked revisions.
| "); | ||
| } | ||
|
|
||
| /// Set up a colocated source repo with a single tracked commit on a bookmark |
There was a problem hiding this comment.
Logic tests should be added to lib/tests/test_git.rs. CLI tests are super slow.
|
I don't think we should support this, #1039 is the appropriate place for such things. |
This fixes a real pain in my workflow, so I would really like to see it somewhere in jj. To me this feels like just an extension of I'm still in the process of investigating based on review comments, so I've marked as draft. |
3f3e215 to
ee04dea
Compare
What will this mean for workflows that are helped / made feasible by this feature? Do you think jj will have something like it soon? I was nearly considering moving all of my projects into a monorepo just to get proper rebases for the ones that depend on shared structure. This is almost the only reason I did not have to. Another huge help to my workflow was #9487, which created huge speedups for my scripts. |
716f5ae to
1d33da0
Compare
When upstream rewrites a change (e.g. amends or rebases) and you fetch the new revision, a normal `jj git fetch` keeps your old revision alongside the new one as a divergent change. With `--rebase`, the incoming revision rewrites your existing one in place: bookmarks, the working copy, and descendants are moved onto the new revision, and the old revision is hidden -- the same outcome jj produces when the rewrite happens locally. If a change already has multiple visible revisions in the repo before the fetch, there is no unambiguous old-to-new mapping, so the import aborts with an error and a hint pointing at `jj abandon` / `jj duplicate`. `abandon_unreachable_commits` now takes an exclusion set so that commits already recorded as rewritten don't have their `Rewritten` entry in `parent_mapping` overwritten with `Abandoned`, which would otherwise reparent descendants onto the old commit's parents instead of onto the rewrite target. git import: rewrite stacks and reject newly-introduced divergence Address two related gaps in --rebase from the previous commit: * Stack rebases: `replace_divergent_changes` only iterated head commits, so when upstream rebased a stack of N changes only the head was paired with its local counterpart. Now we walk every commit that became newly visible in jj (head plus newly-imported ancestors, stopping at commits already reachable from a pre-transaction head) and pair each one's change id. * Fetch-introduced divergence: when one fetch brought in multiple new commits sharing a change id with a single existing local commit, the per-commit loop observed the first new commit as already-visible while processing the second and aborted with `PreExistingDivergentChange`, which is misleading. The rewrite logic now groups by change id and matches old/new candidates by cardinality, and the new `NewlyDivergentChange` error signals the fetch itself was the source. git fetch: shift --rebase coverage into lib tests Move the bulk of --rebase verification out of `cli/tests/test_git_fetch.rs` (where each case needs a colocated source repo and a full CLI roundtrip) into `lib/tests/test_git.rs`, which exercises `git::import_refs` directly against a TestRepo. The CLI-only assertions that lib can't reach — the `Display` for `GitImportError::DivergentChanges` and the exact contents of each problem's commit lists — are pinned at the lib level via `err.to_string()` and sorted commit-id checks. The remote-tracking ref is also asserted in the happy-path lib test so the rewrite covers both `feat` and `feat@origin`. A single CLI snapshot remains for the divergent-changes error renderer (`cli/src/git_util.rs::divergent_changes_error`): it pins the formatted hint header, the indented commit list rendered through `commit_summary_template`, and the trailing hint. The pre-existing case is enough — the newly-introduced path uses the same code with different strings, and those strings are pinned by the lib `err.to_string()` assertions.
1d33da0 to
1c033d1
Compare
|
That looks like a much better solution! |
3 participants
When upstream rewrites a change (e.g. amends or rebases) and you fetch the new revision, a normal
jj git fetchkeeps your old revision alongside the new one as a divergent change. With--rebase, the incoming revision rewrites your existing one in place: bookmarks, the working copy, and descendants are moved onto the new revision, and the old revision is hidden -- the same outcome jj produces when the rewrite happens locally.If a change already has multiple visible revisions in the repo before the fetch, there is no unambiguous old-to-new mapping, so the import aborts with an error and a hint pointing at
jj abandon/jj duplicate.abandon_unreachable_commitsnow takes an exclusion set so that commits already recorded as rewritten don't have theirRewrittenentry inparent_mappingoverwritten withAbandoned, which would otherwise reparent descendants onto the old commit's parents instead of onto the rewrite target.Implementation:
replace_divergent_changes, applied inimport_refs_innerafter refs are updated but beforeabandon_unreachable_commits.newly_imported_ids), so an unchanged ref target that happens to be in the repo isn't treated as a rewrite of something.abandon_unreachable_commitsnow takes an exclusion set so it doesn't overwrite theRewrittenentry inparent_mappingwithAbandoned, which would otherwise reparent descendants onto the old commit's parents instead of onto the rewrite target.Checklist
If applicable:
CHANGELOG.mdREADME.md,docs/,demos/)cli/src/config-schema.json)how it works, how it's organized), including any code drafted by an LLM.
an eye towards deleting anything that is irrelevant, clarifying anything
that is confusing, and adding details that are relevant. This includes,
for example, commit descriptions, PR descriptions, and code comments.
Note: the changes and much of the PR description have definitely been generated by LLM. However, I've tried my best to make sure everything user-facing fits with how the rest of Jujutsu's help is written. As far as I can tell, this implementation is correct (I've looked it over myself to the best of my ability), and I've been using this workflow myself since just a couple hours after this comment.