Re: [PATCH v4 9/9] checkout & worktree: introduce checkout.defaultRemote
On Thu, May 31 2018, Stefan Beller wrote: > Hi Ævar, > > Sorry for chiming in late. I have a couple of thoughts: > >> ( >> cd /tmp && >> rm -rf tbdiff && >> git clone g...@github.com:trast/tbdiff.git && >> cd tbdiff && >> git branch -m topic && >> git checkout master >> ) >> >> That will output: >> >> Branch 'master' set up to track remote branch 'master' from 'origin'. >> Switched to a new branch 'master' > > I thought master is already there after the clone operation and > you'd merely switch back to the local branch that was created at > clone time? > > $ git clone g...@github.com:trast/tbdiff.git && cd tbdiff > $ git branch > * master > $ cat .git/config > ... > [branch "master"] > remote = origin > merge = refs/heads/master > > But the observation is right, we get that message. When do we > do the setup for the master branch specifically? What you're missing is this part: git branch -m topic I.e. we clone the repo, and have a "master" branch, we then rename "master" to "topic", now there's no local master branch. Then we checkout master either with only one remote or two. >> >> But as soon as a new remote is added (e.g. just to inspect something >> from someone else) the DWIMery goes away: >> >> ( >> cd /tmp && >> rm -rf tbdiff && >> git clone g...@github.com:trast/tbdiff.git && >> cd tbdiff && >> git branch -m topic && >> git remote add avar g...@github.com:avar/tbdiff.git && >> git fetch avar && >> git checkout master >> ) >> >> Will output (without the advice output added earlier in this series): >> >> error: pathspec 'master' did not match any file(s) known to git. >> >> The new checkout.defaultRemote config allows me to say that whenever >> that ambiguity comes up I'd like to prefer "origin", and it'll still >> work as though the only remote I had was "origin". >> >> Also adjust the advice.checkoutAmbiguousRemoteBranchName message to >> mention this new config setting to the user, the full output on my >> git.git is now (the last paragraph is new): >> >> $ ./git --exec-path=$PWD checkout master >> error: pathspec 'master' did not match any file(s) known to git. >> hint: The argument 'master' matched more than one remote tracking branch. >> hint: We found 26 remotes with a reference that matched. So we fell back >> hint: on trying to resolve the argument as a path, but failed there too! >> hint: >> hint: Perhaps you meant fully qualify the branch name? E.g. origin/ > > s/meant fully/meant to fully/ > s/? E.g./?\nFor example/ Thanks, will fix. >> hint: instead of ? > > In builtin/submodule--helper.c there is get_default_remote() which also > hardcodes "origin". I think that is a safe thing to do. > >> hint: >> hint: If you'd like to always have checkouts of 'master' prefer one >> remote, >> hint: e.g. the 'origin' remote, consider setting >> checkout.defaultRemote=origin >> hint: in your config. See the 'git-config' manual page for details. > > his new setting elevates one remote over all others, which may > be enough for most setups and not confusing, too. > Consider the following: > > git clone https://kernel.googlesource.com/pub/scm/git/git && cd git > git remote add gitster https://github.com/gitster/git > git remote add interesting-patches https://github.com/avar/git > git remote add my-github https://github.com/stefanbeller/git > > git checkout master > > This probably means I want to have origin/master (from kernel.org) > > git checkout ab/checkout-implicit-remote > > This probably wants to have it from gitster/ (as it is not found on > kernel.org); > I am not sure if it would want to look at interesting-patches/ that mirrors > github, but probably if it were not to be found at gitster. > > So maybe we rather want a setup to give a defined priority for > the search order: > > git config dwim.remoteSearchOrder origin gitster avar > > Stepping back a bit, there is already an order in the config file > for the remotes, and that order is used for example for 'fetch --all'. > > I wonder if we want to take that order? (Or are the days of hand > editing the config over and this is too arcane? We would need a > config command to re order remotes). Then we could just have a > boolean switch to use the config order on ambiguity. > Although you might want to have a different order for fetching > and looking for the right checkout. I thought about this use-case, and if we want this in the future I think the most straightforward way is not to invent some new search order variable, but just make use of git config allowing multi-values, i.e.: [checkout] defaultRemote = origin defaultRemote = gitster Although I'm not interested in implementing that now, and unlike just having one special remote I don't think it's of interest to the vas
Re: [PATCH v4 9/9] checkout & worktree: introduce checkout.defaultRemote
On Thu, May 31, 2018 at 5:49 PM, Stefan Beller wrote: >> I considered splitting this into checkout.defaultRemote and >> worktree.defaultRemote, but it's probably less confusing to break our >> own rules that anything shared between config should live in core.* >> than have two config settings, and I couldn't come up with a short >> name under core.* that made sense (core.defaultRemoteForCheckout?). > > core.dwimRemote ? It's a bit cryptic, though. This option started out as 'core.dwimRemote' in the very first version of this series[1], but someone argued against it for several reasons and suggested 'defaultRemote' instead[2]. [1]: https://public-inbox.org/git/20180502105452.17583-1-ava...@gmail.com/ [2]: https://public-inbox.org/git/capig+ctzyyc-1_txl2prfof6haktuxxm+g5excbys5fcdmd...@mail.gmail.com/
Re: [PATCH v4 9/9] checkout & worktree: introduce checkout.defaultRemote
Thomas Gummerer writes: >> I considered splitting this into checkout.defaultRemote and >> worktree.defaultRemote, but it's probably less confusing to break our >> own rules that anything shared between config should live in core.* >> than have two config settings, and I couldn't come up with a short >> name under core.* that made sense (core.defaultRemoteForCheckout?). I do think "checkout" in name is grately helpful. I do not see why it is a bad idea for the worktree codepath to pay attention to the checkout.defaultRemote configuration variable, especially when those who are discussing this thread agree "checkout" and "worktree add" are quite similar in end-users' minds.
Re: [PATCH v4 9/9] checkout & worktree: introduce checkout.defaultRemote
On 05/31, Ævar Arnfjörð Bjarmason wrote: > Introduce a checkout.defaultRemote setting which can be used to > designate a remote to prefer (via checkout.defaultRemote=origin) when > running e.g. "git checkout master" to mean origin/master, even though > there's other remotes that have the "master" branch. > > I want this because it's very handy to use this workflow to checkout a > repository and create a topic branch, then get back to a "master" as > retrieved from upstream: > > ( > cd /tmp && > rm -rf tbdiff && > git clone g...@github.com:trast/tbdiff.git && > cd tbdiff && > git branch -m topic && > git checkout master > ) > > That will output: > > Branch 'master' set up to track remote branch 'master' from 'origin'. > Switched to a new branch 'master' > > But as soon as a new remote is added (e.g. just to inspect something > from someone else) the DWIMery goes away: > > ( > cd /tmp && > rm -rf tbdiff && > git clone g...@github.com:trast/tbdiff.git && > cd tbdiff && > git branch -m topic && > git remote add avar g...@github.com:avar/tbdiff.git && > git fetch avar && > git checkout master > ) > > Will output (without the advice output added earlier in this series): > > error: pathspec 'master' did not match any file(s) known to git. > > The new checkout.defaultRemote config allows me to say that whenever > that ambiguity comes up I'd like to prefer "origin", and it'll still > work as though the only remote I had was "origin". > > Also adjust the advice.checkoutAmbiguousRemoteBranchName message to > mention this new config setting to the user, the full output on my > git.git is now (the last paragraph is new): > > $ ./git --exec-path=$PWD checkout master > error: pathspec 'master' did not match any file(s) known to git. > hint: The argument 'master' matched more than one remote tracking branch. > hint: We found 26 remotes with a reference that matched. So we fell back > hint: on trying to resolve the argument as a path, but failed there too! > hint: > hint: Perhaps you meant fully qualify the branch name? E.g. origin/ > hint: instead of ? > hint: > hint: If you'd like to always have checkouts of 'master' prefer one > remote, > hint: e.g. the 'origin' remote, consider setting > checkout.defaultRemote=origin > hint: in your config. See the 'git-config' manual page for details. > > I considered splitting this into checkout.defaultRemote and > worktree.defaultRemote, but it's probably less confusing to break our > own rules that anything shared between config should live in core.* > than have two config settings, and I couldn't come up with a short > name under core.* that made sense (core.defaultRemoteForCheckout?). I agree that splitting this into two variables would be needlessly confusing. 'checkout' and 'worktree add' are similar enough in spirit, that users only setting one of the configuration variables would end up confused at some point. Because the commands are so similar, I also feel like it would be okay to break our own rules here, and use the 'core.defaultRemote' name you suggested (I also can't come up with anything better in core.* right now). > See also 70c9ac2f19 ("DWIM "git checkout frotz" to "git checkout -b > frotz origin/frotz"", 2009-10-18) which introduced this DWIM feature > to begin with, and 4e85333197 ("worktree: make add > dwim", 2017-11-26) which added it to git-worktree. > > Signed-off-by: Ævar Arnfjörð Bjarmason > --- > Documentation/config.txt | 21 - > Documentation/git-checkout.txt | 9 + > Documentation/git-worktree.txt | 9 + > builtin/checkout.c | 15 +++ > checkout.c | 21 - > checkout.h | 5 - > t/t2024-checkout-dwim.sh | 12 > t/t2025-worktree-add.sh| 21 + > 8 files changed, 106 insertions(+), 7 deletions(-) > > [snip] > > diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt > index ca5fc9c798..8cb77bddeb 100644 > --- a/Documentation/git-checkout.txt > +++ b/Documentation/git-checkout.txt > @@ -38,6 +38,15 @@ equivalent to > $ git checkout -b --track / > > + > +If the branch exists in multiple remotes and one of them is named by > +the `checkout.defaultRemote` configuration variable, we'll use that > +one for the purposes of disambiguation, even if the `` isn't > +unique across all remotes. Set it to > +e.g. `checkout.defaultRemote=origin` to always checkout remote > +branches from there if ` is ambiguous but exists on the s/`/&`/ > +'origin' remote. See also `checkout.defaultRemote` in > +linkgit:git-config[1]. > ++ > You could omit , in which case the command degenerates to > "check out the current branch", which is a glorified no-op with >
Re: [PATCH v4 9/9] checkout & worktree: introduce checkout.defaultRemote
Hi Ævar, Sorry for chiming in late. I have a couple of thoughts: > ( > cd /tmp && > rm -rf tbdiff && > git clone g...@github.com:trast/tbdiff.git && > cd tbdiff && > git branch -m topic && > git checkout master > ) > > That will output: > > Branch 'master' set up to track remote branch 'master' from 'origin'. > Switched to a new branch 'master' I thought master is already there after the clone operation and you'd merely switch back to the local branch that was created at clone time? $ git clone g...@github.com:trast/tbdiff.git && cd tbdiff $ git branch * master $ cat .git/config ... [branch "master"] remote = origin merge = refs/heads/master But the observation is right, we get that message. When do we do the setup for the master branch specifically? > > But as soon as a new remote is added (e.g. just to inspect something > from someone else) the DWIMery goes away: > > ( > cd /tmp && > rm -rf tbdiff && > git clone g...@github.com:trast/tbdiff.git && > cd tbdiff && > git branch -m topic && > git remote add avar g...@github.com:avar/tbdiff.git && > git fetch avar && > git checkout master > ) > > Will output (without the advice output added earlier in this series): > > error: pathspec 'master' did not match any file(s) known to git. > > The new checkout.defaultRemote config allows me to say that whenever > that ambiguity comes up I'd like to prefer "origin", and it'll still > work as though the only remote I had was "origin". > > Also adjust the advice.checkoutAmbiguousRemoteBranchName message to > mention this new config setting to the user, the full output on my > git.git is now (the last paragraph is new): > > $ ./git --exec-path=$PWD checkout master > error: pathspec 'master' did not match any file(s) known to git. > hint: The argument 'master' matched more than one remote tracking branch. > hint: We found 26 remotes with a reference that matched. So we fell back > hint: on trying to resolve the argument as a path, but failed there too! > hint: > hint: Perhaps you meant fully qualify the branch name? E.g. origin/ s/meant fully/meant to fully/ s/? E.g./?\nFor example/ > hint: instead of ? In builtin/submodule--helper.c there is get_default_remote() which also hardcodes "origin". I think that is a safe thing to do. > hint: > hint: If you'd like to always have checkouts of 'master' prefer one > remote, > hint: e.g. the 'origin' remote, consider setting > checkout.defaultRemote=origin > hint: in your config. See the 'git-config' manual page for details. his new setting elevates one remote over all others, which may be enough for most setups and not confusing, too. Consider the following: git clone https://kernel.googlesource.com/pub/scm/git/git && cd git git remote add gitster https://github.com/gitster/git git remote add interesting-patches https://github.com/avar/git git remote add my-github https://github.com/stefanbeller/git git checkout master This probably means I want to have origin/master (from kernel.org) git checkout ab/checkout-implicit-remote This probably wants to have it from gitster/ (as it is not found on kernel.org); I am not sure if it would want to look at interesting-patches/ that mirrors github, but probably if it were not to be found at gitster. So maybe we rather want a setup to give a defined priority for the search order: git config dwim.remoteSearchOrder origin gitster avar Stepping back a bit, there is already an order in the config file for the remotes, and that order is used for example for 'fetch --all'. I wonder if we want to take that order? (Or are the days of hand editing the config over and this is too arcane? We would need a config command to re order remotes). Then we could just have a boolean switch to use the config order on ambiguity. Although you might want to have a different order for fetching and looking for the right checkout. > I considered splitting this into checkout.defaultRemote and > worktree.defaultRemote, but it's probably less confusing to break our > own rules that anything shared between config should live in core.* > than have two config settings, and I couldn't come up with a short > name under core.* that made sense (core.defaultRemoteForCheckout?). core.dwimRemote ? It's a bit cryptic, though. > See also 70c9ac2f19 ("DWIM "git checkout frotz" to "git checkout -b > frotz origin/frotz"", 2009-10-18) which introduced this DWIM feature > to begin with, and 4e85333197 ("worktree: make add > dwim", 2017-11-26) which added it to git-worktree. Stefan