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

git sync: Get branch heads and fetch from remote #4909

Closed
wants to merge 1 commit into from

Conversation

essiene
Copy link
Contributor

@essiene essiene commented Nov 19, 2024

Summary

  • Get branch pre-fetch heads
  • Build candidates from prefetch heads
  • Fetch from remotes
  • Get branch post-fetch heads
  • Rebase

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.

Issue: #1039

@essiene essiene force-pushed the essiene/snqxmuzvuvpn branch from d28206b to aa853cb Compare November 19, 2024 01:18
@essiene essiene changed the title git sync: Get heads and build candidate commits. git sync: Get branch heads and fetch from remote Nov 19, 2024
@essiene essiene force-pushed the essiene/tknsxroqylmx branch 2 times, most recently from 4e46350 to eba2a6e Compare November 19, 2024 01:46
@essiene essiene force-pushed the essiene/snqxmuzvuvpn branch from aa853cb to 50e1938 Compare November 19, 2024 01:46
@essiene essiene force-pushed the essiene/tknsxroqylmx branch from eba2a6e to 371b264 Compare November 19, 2024 12:53
@essiene essiene force-pushed the essiene/snqxmuzvuvpn branch 3 times, most recently from fa04074 to 2859669 Compare November 19, 2024 14:15
@essiene
Copy link
Contributor Author

essiene commented Nov 19, 2024

I'm not sure if I should add unit tests tests for these or just add integration tests for jj git sync directly; I'm planning to do that in the later PRs.

@essiene essiene marked this pull request as ready for review November 19, 2024 14:18
@essiene essiene requested a review from yuja November 19, 2024 14:18
@essiene essiene force-pushed the essiene/snqxmuzvuvpn branch 2 times, most recently from 327d43d to c3da8e8 Compare November 19, 2024 18:31
Copy link
Contributor

@yuja yuja left a comment

Choose a reason for hiding this comment

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

I'm not sure what we're going to achieve, but my gut feeling is that this and the subsequent PRs duplicate git::import_refs()? I would instead try to extend git:import_refs() to return information needed to rebase commits.

And the core "git sync" function might be implemented in lib/src/git.rs.

## Summary
* [X] Get branch pre-fetch heads
* [ ] Build candidates from prefetch heads
* [X] Fetch from remotes
* [X] Get branch post-fetch heads
* [ ] Rebase

## 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.

Issue: #1039
@essiene essiene force-pushed the essiene/tknsxroqylmx branch from a7661c9 to 6c900a1 Compare November 23, 2024 10:35
@essiene essiene force-pushed the essiene/snqxmuzvuvpn branch from c3da8e8 to 29cd10a Compare November 23, 2024 10:35
@essiene
Copy link
Contributor Author

essiene commented Nov 23, 2024

I'm not sure what we're going to achieve, but my gut feeling is that this and the subsequent PRs duplicate git::import_refs()? I would instead try to extend git:import_refs() to return information needed to rebase commits.

And the core "git sync" function might be implemented in lib/src/git.rs.

Thanks for the suggestion. Based on this and comments from @martinvonz, I've gone to actually read git::import_some_refs() and I understand it better now.

Abandoning this PR now and will send a new one that changes jj-lib to introduce core functionality for sync to be built atop of.

@essiene essiene closed this Nov 23, 2024
@essiene essiene deleted the essiene/snqxmuzvuvpn branch November 23, 2024 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants