Re: [RFD/PATCH 3/5] checkout: Use remote refspecs when DWIMming tracking branches
On Fri, Apr 19, 2013 at 9:44 PM, Junio C Hamano gits...@pobox.com wrote: I am _guessing_ that you mean a case like this: [remote origin] fetch = refs/heads/*:refs/remotes/origin/* [remote xyzzy] fetch = refs/heads/*:refs/remotes/xyzzy/nitfol/* [remote frotz] fetch = refs/heads/*:refs/remotes/frotz/nitfol/* and refs/remotes/origin/foo or refs/remotes/frotz/nitfol/foo do not exist but refs/remotes/xyzzy/nitfol/foo does. And the user says git checkout foo. Instead of finding a remote ref that matches refs/remotes/*/foo pattern (and assuming the part that matched * is the name of the remote), you can iterate the RHS of the refspecs to see if there is a unique match. Then the new branch can unambiguously find that its upstream is xyzzy's foo. Correct. I will try to phrase the problem better in the next iteration. I think it makes sense to update the semantics like that. Wouldn't the traditional case (i.e. without nitfol/ in the xyzzy/frotz remotes above) be a mere special case for this new logic? Yes. You mentioned there is a regression caught by existing tests if you go this route, but I do not see how that happens. It's technically a regression since it breaks existing tests, but as you observe in your reply to patch #5, it is really the test that relies on current implementation details. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFD/PATCH 3/5] checkout: Use remote refspecs when DWIMming tracking branches
The DWIM mode of checkout allows you to run git checkout foo when there is
no existing local ref or path called foo, and there is exactly one remote
with a remote-tracking branch called foo. Git will then automatically
create a new local branch called foo using the remote-tracking foo as
its starting point and configured upstream.
However, the current code hardcodes the assumption that all remote-tracking
branches are located at refs/remotes/$remote/*, and that git checkout foo
must find exactly one ref matching refs/remotes/*/foo to succeed.
This approach fails if a user has customized the refspec of a given remote to
place remote-tracking branches elsewhere.
The better way to find a tracking branch is to use the fetch refspecs for the
configured remotes to deduce the available candidate remote-tracking branches
corresponding to a remote branch of the requested name, and if exactly one of
these exists (and points to a valid SHA1), then that is the remote-tracking
branch we will use.
For example, in the case of git checkout foo, we map refs/heads/foo
through each remote's refspec to find the available candidate remote-tracking
branches, and if exactly one of these candidates exist in our local repo, then
we have found the upstream for the new local branch foo.
This fixes most of the failing tests introduced in the previous patch.
Signed-off-by: Johan Herland jo...@herland.net
---
Documentation/git-checkout.txt | 6 +++---
builtin/checkout.c | 42 ++
t/t2024-checkout-dwim.sh | 6 +++---
3 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 8edcdca..bf0c99c 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -131,9 +131,9 @@ entries; instead, unmerged entries are ignored.
--track in linkgit:git-branch[1] for details.
+
If no '-b' option is given, the name of the new branch will be
-derived from the remote-tracking branch. If remotes/ or refs/remotes/
-is prefixed it is stripped away, and then the part up to the
-next slash (which would be the nickname of the remote) is removed.
+derived from the remote-tracking branch, by looking at the local part of
+the refspec configured for the corresponding remote, and then stripping
+the initial part up to the *.
This would tell us to use hack as the local branch when branching
off of origin/hack (or remotes/origin/hack, or even
refs/remotes/origin/hack). If the given name has no slash, or the above
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f8033f4..d6f9c01 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -822,38 +822,40 @@ static int git_checkout_config(const char *var, const
char *value, void *cb)
}
struct tracking_name_data {
- const char *name;
- char *remote;
+ /* const */ char *src_ref;
+ char *dst_ref;
+ unsigned char *dst_sha1;
int unique;
};
-static int check_tracking_name(const char *refname, const unsigned char *sha1,
- int flags, void *cb_data)
+static int check_tracking_name(struct remote *remote, void *cb_data)
{
struct tracking_name_data *cb = cb_data;
- const char *slash;
-
- if (prefixcmp(refname, refs/remotes/))
- return 0;
- slash = strchr(refname + 13, '/');
- if (!slash || strcmp(slash + 1, cb-name))
+ struct refspec query;
+ memset(query, 0, sizeof(struct refspec));
+ query.src = cb-src_ref;
+ if (remote_find_tracking(remote, query) ||
+ get_sha1(query.dst, cb-dst_sha1))
return 0;
- if (cb-remote) {
+ if (cb-dst_ref) {
cb-unique = 0;
return 0;
}
- cb-remote = xstrdup(refname);
+ cb-dst_ref = xstrdup(query.dst);
return 0;
}
-static const char *unique_tracking_name(const char *name)
+static const char *unique_tracking_name(const char *name, unsigned char *sha1)
{
- struct tracking_name_data cb_data = { NULL, NULL, 1 };
- cb_data.name = name;
- for_each_ref(check_tracking_name, cb_data);
+ struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+ char src_ref[PATH_MAX];
+ snprintf(src_ref, PATH_MAX, refs/heads/%s, name);
+ cb_data.src_ref = src_ref;
+ cb_data.dst_sha1 = sha1;
+ for_each_remote(check_tracking_name, cb_data);
if (cb_data.unique)
- return cb_data.remote;
- free(cb_data.remote);
+ return cb_data.dst_ref;
+ free(cb_data.dst_ref);
return NULL;
}
@@ -916,8 +918,8 @@ static int parse_branchname_arg(int argc, const char **argv,
if (dwim_new_local_branch_ok
!check_filename(NULL, arg)
argc == 1) {
- const char *remote = unique_tracking_name(arg);
- if (!remote ||
Re: [RFD/PATCH 3/5] checkout: Use remote refspecs when DWIMming tracking branches
Johan Herland jo...@herland.net writes: The DWIM mode of checkout allows you to run git checkout foo when there is no existing local ref or path called foo, and there is exactly one remote with a remote-tracking branch called foo. Git will then automatically create a new local branch called foo using the remote-tracking foo as its starting point and configured upstream. However, the current code hardcodes the assumption that all remote-tracking branches are located at refs/remotes/$remote/*, and that git checkout foo must find exactly one ref matching refs/remotes/*/foo to succeed. This approach fails if a user has customized the refspec of a given remote to place remote-tracking branches elsewhere. The better way to find a tracking branch is to use the fetch refspecs for the configured remotes to deduce the available candidate remote-tracking branches corresponding to a remote branch of the requested name, and if exactly one of these exists (and points to a valid SHA1), then that is the remote-tracking branch we will use. For example, in the case of git checkout foo, we map refs/heads/foo through each remote's refspec to find the available candidate remote-tracking branches, and if exactly one of these candidates exist in our local repo, then we have found the upstream for the new local branch foo. Once you introduce a concrete foo as a name in the example, it would be far easier to understand if you spelled all the other assumptions in the example out. I am _guessing_ that you mean a case like this: [remote origin] fetch = refs/heads/*:refs/remotes/origin/* [remote xyzzy] fetch = refs/heads/*:refs/remotes/xyzzy/nitfol/* [remote frotz] fetch = refs/heads/*:refs/remotes/frotz/nitfol/* and refs/remotes/origin/foo or refs/remotes/frotz/nitfol/foo do not exist but refs/remotes/xyzzy/nitfol/foo does. And the user says git checkout foo. Instead of finding a remote ref that matches refs/remotes/*/foo pattern (and assuming the part that matched * is the name of the remote), you can iterate the RHS of the refspecs to see if there is a unique match. Then the new branch can unambiguously find that its upstream is xyzzy's foo. I think it makes sense to update the semantics like that. Wouldn't the traditional case (i.e. without nitfol/ in the xyzzy/frotz remotes above) be a mere special case for this new logic? You mentioned there is a regression caught by existing tests if you go this route, but I do not see how that happens. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
