cli: git clone: add --branch option #7338
Conversation
owenbrooks
force-pushed
the
push-zmmvnpytokvt
branch
from
48c542c to
49ed4eb
Compare
cli/src/commands/git/clone.rs
Outdated
| /// If not present, all branches are fetched and the repository's default | ||
| /// branch is the parent of the working-copy change | ||
| #[arg(long)] | ||
| branch: Option<String>, |
There was a problem hiding this comment.
Maybe we can support multiple branches, i.e. branch: Vec<StringPattern>?
If #7275 gets merged, these patterns can be set as the default refspecs so future jj git fetch wouldn't fetch all branches.
I'm not sure which branch should be checked out if there are multiple matching bookmarks, but maybe we can try the default first, then fall back to one of the matching bookmark?
There was a problem hiding this comment.
Multiple branches makes sense.
I'm thinking checking out the first matching pattern could be nice. It gives the caller control over the branch which I feel is the purpose of this option.
So jj git clone <repo> --branch feature_branch --branch main --branch other would result in feature_branch being checked out, even if main is normally the default. What do you think?
There was a problem hiding this comment.
That sounds also good, but well need to pick one arbitrarily for --branch glob:pattern. I don't mind if the first matching branch (in e.g. lexicographical order) were chosen if that's easier to implement.
There was a problem hiding this comment.
Added multiple branch support, where the first match in lexicographical order, of the first --branch pattern by position, becomes the working copy parent.
owenbrooks
force-pushed
the
push-zmmvnpytokvt
branch
2 times, most recently
from
6af27ac to
4aca08e
Compare
| target_branches | ||
| .unwrap_or(&[StringPattern::everything()]) | ||
| .to_vec(), | ||
| )?, |
There was a problem hiding this comment.
Since #7275 gets merged, we can configure the remote to fetch only the specified branches/bookmarks. That seems better since future jj git fetch won't fetch other bookmarks. Then, we wouldn't have to specify the target branches here.
EDIT: we'll need to pass the same refspecs to fetch() function.
There was a problem hiding this comment.
Just clarifying, there is no change needed here, but we now will have the expected behaviour for future calls to jj git fetch, right?
There was a problem hiding this comment.
We'll probably need to pass the fetch refspecs to git::add_remote() to make them persist. Can you add test for clone --branch && fetch?
We can address this later, but I think it's better to include it in the same release.
cli/src/commands/git/clone.rs
Outdated
| if first_match.is_none() { | ||
| return Err(user_error(format!( | ||
| "No such branch matching pattern: {first_pattern}", | ||
| ))); | ||
| } |
There was a problem hiding this comment.
If we make it error out, I think we should check if there are no bookmarks matching any of the patterns, not the first pattern. Another option is to check if each pattern matches at least one bookmark.
There was a problem hiding this comment.
Added a check that every pattern must match at least one bookmark.
There was a problem hiding this comment.
Do we need the special logic for the first match then? They seem functionally equivalent to me, so that extra logic should probably be removed
owenbrooks
force-pushed
the
push-zmmvnpytokvt
branch
3 times, most recently
from
c85eb9e to
f6896f0
Compare
When cloning with the branch option: - Only the specified branch will be fetched - The trunk alias is only set if the specified branch happens to be the default branch - The clone fails if the branch does not exist in the remote
owenbrooks
force-pushed
the
push-zmmvnpytokvt
branch
from
f6896f0 to
e746ff2
Compare
Fixes #7083
Checklist
If applicable:
CHANGELOG.md