[PATCH v4 9/9] checkout & worktree: introduce checkout.defaultRemote

2018-05-31 Thread Ævar Arnfjörð Bjarmason
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?).

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(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 08d3e70989..e0d92217ac 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -350,7 +350,10 @@ advice.*::
remote tracking branch on more than one remote in
situations where an unambiguous argument would have
otherwise caused a remote-tracking branch to be
-   checked out.
+   checked out. See the `checkout.defaultRemote`
+   configuration variable for how to set a given remote
+   to used by default in some situations where this
+   advice would be printed.
amWorkDir::
Advice that shows the location of the patch file when
linkgit:git-am[1] fails to apply it.
@@ -1105,6 +1108,22 @@ browser..path::
browse HTML help (see `-w` option in linkgit:git-help[1]) or a
working repository in gitweb (see linkgit:git-instaweb[1]).
 
+checkout.defaultRemote::
+   When you run 'git checkout ' and only have one
+   remote, it may implicitly fall back on checking out and
+   tracking e.g. 'origin/'. This stops working as soon
+   as you have more than one remote with a ''
+   reference. This setting allows for setting the name of a
+   preferred remote that should always win when it comes to
+   disambiguation. The typical use-case is to set this to
+   `origin`.
++
+Currently this is used by linkgit:git-checkout[1] when 'git checkout
+' will checkout the '' branch on anoth

Re: [PATCH v4 9/9] checkout & worktree: introduce checkout.defaultRemote

2018-05-31 Thread Stefan Beller
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


Re: [PATCH v4 9/9] checkout & worktree: introduce checkout.defaultRemote

2018-05-31 Thread Thomas Gummerer
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

2018-05-31 Thread Junio C Hamano
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

2018-06-01 Thread Eric Sunshine
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

2018-06-01 Thread Ævar Arnfjörð Bjarmason


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