Improve fetch from multiple remotes #4923
Conversation
mateon1
force-pushed
the
multifetch
branch
2 times, most recently
from
e81be24
to
bbb44a7
Compare
|
Here's how fetching looks like in the situation given in #4920 with this PR: For comparison, here are the situations with separate fetch operations (behavior not affected by this PR) b, then cc, then b |
|
Perhaps, the easiest workaround is to do diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs
index 74ebe2d2fa..8949218232 100644
--- a/cli/src/git_util.rs
+++ b/cli/src/git_util.rs
@@ -496,6 +496,11 @@
_ => user_error(err),
})?;
print_git_import_stats(ui, tx.repo(), &stats.import_stats, true)?;
+ let settings = tx.settings().clone(); // XXX silly clone
+ let num_rebased = tx.repo_mut().rebase_descendants(&settings)?;
+ if num_rebased > 0 {
+ writeln!(ui.status(), "Rebased {num_rebased} descendant commits")?;
+ }
}
warn_if_branches_not_found(
ui,It might be cleaner to do import at the end, but it's weird that |
|
Yuja's patch does work, with this result: So it's a direct fix to the current bug while keeping existing behavior. Re: default_branch, that's also a bit weird. It's only used from jj git clone, which only supports a single remote. Even if it did support multiple, how could you possibly know which remote is 'most authoritative' for choosing the default branch? What if you clone two forks of the same repo? What if you clone two completely independent repos? |
You can turn it off by
Perhaps, the API can be refactored to something like this: let mut peer = GitFetch::new(..);
let default_branch = peer.fetch(remote_name, branch_names, ..);
...
let stats = peer.import_refs();(and @essiene might be interested. |
This seems like a good approach. We could call peer.fetch() in a loop for all remotes and at the end, call peer.import_refs just once.
We could also have a thin wrapper for If there are no objections, I go ahead and send out a few PRs that do this.
Indeed. Thanks for looping me in. |
Sounds good, thanks. BTW, I used |
Yeah, makes sense. |
* This allows more control over the stages of the fetch.
* Provide thin and directed wrappers for clone and fetch
which allow slightly different overall actions.
* Allow `git::fetch` to handle multiple remotes.
* One important side-effect to note (as demonstrated in the changed
cli test) is that when there are multiple remotes, all fetches
must pass before `import_refs` starts; no branch imports if a
fetch from any remote fails.
* Update call sites to use new `git::clone` and `git::fetch`
Fixes: #4923
* This allows more control over the stages of the fetch.
* Provide thin and directed wrappers for clone and fetch
which allow slightly different overall actions.
* Allow `git::fetch` to handle multiple remotes.
* One important side-effect to note (as demonstrated in the changed
cli test) is that when there are multiple remotes, all fetches
must pass before `import_refs` starts; no branch imports if a
fetch from any remote fails.
* Update call sites to use new `git::clone` and `git::fetch`
Fixes: #4923
* Reimplement `git::fetch` in terms of the new lower-level `GitFetch` api, which allows more control over the stages of the fetch. * Make `git::fetch` to accept multiple remotes, fetch from them all, before calling `import_refs`. * Set `default_branch` to None in the `GitFetchStats` return value from `git::fetch`. * Update call sites to use new `git::clone` and `git::fetch` * Update tests: One important side-effect to note (as demonstrated in the changed cli test) is that when there are multiple remotes, all fetches must pass before `import_refs` starts; no branch imports if a fetch from any remote fails. This is WAI. Fixes: #4923
* This allows more control over the stages of the fetch. * Implement `git::clone` in terms of the new api. * Update call sites to use new `git::clone`. * Update tests to explicitly use `git::clone` where relevant. Issue: #4923
* Reimplement `git::fetch` in terms of the new lower-level `GitFetch` api, which allows more control over the stages of the fetch. * Make `git::fetch` to accept multiple remotes, fetch from them all, before calling `import_refs`. * Set `default_branch` to None in the `GitFetchStats` return value from `git::fetch`. * Update call sites to use new `git::clone` and `git::fetch` * Update tests: One important side-effect to note (as demonstrated in the changed cli test) is that when there are multiple remotes, all fetches must pass before `import_refs` starts; no branch imports if a fetch from any remote fails. This is WAI. Fixes: #4923
* Reimplement `git::fetch` in terms of the new lower-level `GitFetch` api, which allows more control over the stages of the fetch. * Make `git::fetch` to accept multiple remotes, fetch from them all, before calling `import_refs`. * Set `default_branch` to None in the `GitFetchStats` return value from `git::fetch`. * Update call sites to use new `git::clone` and `git::fetch` * Update tests: One important side-effect to note (as demonstrated in the changed cli test) is that when there are multiple remotes, all fetches must pass before `import_refs` starts; no branch imports if a fetch from any remote fails. This is WAI. Fixes: #4923
|
See #4920 for rationale.
Checklist
If applicable:
CHANGELOG.md