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.

Sign up for GitHub

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

Jump to bottom

git sync: Setup scaffolding #4824

Open
wants to merge 2 commits into
base: essiene/pmwupzxmwsqz
Choose a base branch
from
Open

git sync: Setup scaffolding #4824

wants to merge 2 commits into from
+1,839 −63

Conversation

essiene
Copy link
Collaborator

@essiene essiene commented Nov 10, 2024 •
edited
Loading

The chain of commits is long, so I'm splitting them up for ease of review, starting
with this simple scaffolding PR.

I'm not sure if there's a better way to handle the splits though, I'm happy to adjust in that case.

  • First commit that just sets up scaffolding.
  • jj git sync is hidden as it's not yet ready.
  • Set up some tracing guards which will help debugging when run with --debug.

Issue: #1039

@essiene essiene force-pushed the essiene/tknsxroqylmx branch 2 times, most recently from 9720683 to a28c7e1 Compare November 11, 2024 21:27
@essiene essiene force-pushed the essiene/tknsxroqylmx branch 4 times, most recently from 344f0ae to 1225a22 Compare November 12, 2024 20:36
@essiene essiene force-pushed the essiene/tknsxroqylmx branch 3 times, most recently from e4de5bc to a31e01d Compare November 13, 2024 22:53
@essiene essiene force-pushed the essiene/pmwupzxmwsqz branch 2 times, most recently from 5f24508 to 9b0981d Compare November 17, 2024 22:08
@essiene essiene force-pushed the essiene/tknsxroqylmx branch 3 times, most recently from 240a7e0 to bebcc8b Compare November 19, 2024 00:13
@essiene essiene changed the title PoC: Implement jj git sync - v1 git sync: Setup scaffolding Nov 19, 2024
@essiene essiene force-pushed the essiene/tknsxroqylmx branch 3 times, most recently from eba2a6e to 371b264 Compare November 19, 2024 12:53
@essiene essiene marked this pull request as ready for review November 19, 2024 14:08
@essiene essiene requested a review from yuja November 19, 2024 14:08
@martinvonz
Copy link
Owner

I'm not sure if there's a better way to handle the splits though, I'm happy to adjust in that case.

I prefer to see it split up so the jj-lib changes come first. Speaking of that, I hope it's not too late to say that ideally most of the changes and tests should go in jj-lib. The CLI crate should just do the UI-level stuff like handling arguments and presenting the result. Tests are much faster in jj-lib than in jj-cli, so we prefer to test what's possible there.

@essiene
Copy link
Collaborator Author

essiene commented Nov 19, 2024

I'm not sure if there's a better way to handle the splits though, I'm happy to adjust in that case.

I prefer to see it split up so the jj-lib changes come first. Speaking of that, I hope it's not too late to say that ideally most of the changes and tests should go in jj-lib. The CLI crate should just do the UI-level stuff like handling arguments and presenting the result. Tests are much faster in jj-lib than in jj-cli, so we prefer to test what's possible there.

This makes sense. Let me see how best to rework it to end up with most changes in jj-lib

@essiene essiene force-pushed the essiene/pmwupzxmwsqz branch 2 times, most recently from 60c695f to d1248e3 Compare November 23, 2024 11:57
@essiene
cli: Refactor out get_fetch_remotes into git_util.rs
eceb664 
Similarly to `git_fetch`, this will be used in `jj git sync`, so
moving this out to maintain the same behaviour across `fetch` command and `sync`.

* Refactor out `get_fetch_remotes` to `git_util.rs`.
* inline the different `get_*_remote*` functions into `get_fetch_remotes`;
  The resulting function is still small enough to be easily readable.
* Move `get_single_remote` into `git_util.rs` and update call sites.
* Use common FetchArgs to pass arguments both to `get_fetch_remotes` and `git_fetch`
* Update use references

All tests still pass.

Issue: #1039
@essiene
git sync: PoC
174600a 
## Summary
* [X] Get branch pre-fetch heads
* [X] Build candidates from prefetch heads
* [X] Fetch from remotes
* [X] Get branch post-fetch heads
* [X] Rebase
  * [X] Build old -> new map
  * [X] transform descendants

# git sync: Setup scaffolding
## Details
* First commit that just sets up scaffolding.
* `jj git sync` is hidden as it's not yet ready.
* Set up some tracing guards which will help debugging when run with --debug.


# git sync: Get branch heads and fetch from remote.
## Details
* For branch heads, we grab local branches matching the pattern
  specified in args.
* Pre-fetch heads query tx.base_repo() while post-fetch heads
  query tx.repo(); tx.repo() is the in memory Mutable repo which
  is updated after a fetch in a transaction.
* We fetch from all branches from all configured remotes.
  --all_remotes affects if we use `git.fetch` or all repo remotes.

# git sync: Build candidate commits.
## Details
* Candidate commits (parent_id, child_id) are:
    * branch commits
    * and their descendants
    * which are not found in untracked remote branches

# git sync: Build update record
## Details
Build a record of old branch heads mapped to the new branch heads.
This record will be used to derive the candidates that need
rebasing.


# git sync: unhide `jj git sync`
## Details
* Add tests
* Unhide the command, now that it's complete.
* Update CHANGELOG


# git sync: Rebase commits onto new branch heads.
## Details
* implement CommitRewritter::repo() to return an immutable reference.
* transform_descendants() roots are candidates with parents in update_record.
* simplify parent merge
* drop newly emptied commits
* update new parents from the updated heads set if the old parents are ancestors.

Issue: #1039
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuja yuja Awaiting requested review from yuja

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@essiene @martinvonz