worktree: add per-worktree config files
diff mbox series

Message ID 20180923170438.23610-1-pclouds@gmail.com
State New
Headers show
Series
  • worktree: add per-worktree config files
Related show

Commit Message

Duy Nguyen Sept. 23, 2018, 5:04 p.m. UTC
A new repo extension is added, worktreeConfig. When it is present:

 - Repository config reading by default includes $GIT_DIR/config _and_
   $GIT_DIR/config.worktree. "config" file remains shared in multiple
   worktree setup.

 - The special treatment for core.bare and core.worktree, to stay
   effective only in main worktree, is gone. These config files are
   supposed to be in config.worktree.

This extension is most useful in multiple worktree setup because you
now have an option to store per-worktree config (which is either
.git/config.worktree for main worktree, or
.git/worktrees/xx/config.worktree for linked ones).

This extension can be used in single worktree mode, even though it's
pretty much useless (but this can happen after you remove all linked
worktrees and move back to single worktree).

"git config" reads from both "config" and "config.worktree" by default
(i.e. without either --user, --file...) when this extension is
present. Default writes still go to "config", not "config.worktree". A
new option --worktree is added for that (*).

Since a new repo extension is introduced, existing git binaries should
refuse to access to the repo (both from main and linked worktrees). So
they will not misread the config file (i.e. skip the config.worktree
part). They may still accidentally write to the config file anyway if
they use with "git config --file <path>".

This design places a bet on the assumption that the majority of config
variables are shared so it is the default mode. A safer move would be
default writes go to per-worktree file, so that accidental changes are
isolated.

(*) "git config --worktree" points back to "config" file when this
    extension is not present so that it works in any setup.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/config.txt               | 11 +++-
 Documentation/git-config.txt           | 26 +++++---
 Documentation/git-worktree.txt         | 37 ++++++++++++
 Documentation/gitrepository-layout.txt |  8 +++
 builtin/config.c                       | 16 ++++-
 cache.h                                |  2 +
 config.c                               |  7 +++
 environment.c                          |  1 +
 setup.c                                |  5 +-
 t/t2029-worktree-config.sh             | 82 ++++++++++++++++++++++++++
 worktree.c                             | 41 +++++++++++++
 worktree.h                             |  6 ++
 12 files changed, 231 insertions(+), 11 deletions(-)
 create mode 100755 t/t2029-worktree-config.sh

Comments

Eric Sunshine Sept. 23, 2018, 8:47 p.m. UTC | #1
On Sun, Sep 23, 2018 at 1:04 PM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> A new repo extension is added, worktreeConfig. When it is present:
>
>  - Repository config reading by default includes $GIT_DIR/config _and_
>    $GIT_DIR/config.worktree. "config" file remains shared in multiple
>    worktree setup.
>
>  - The special treatment for core.bare and core.worktree, to stay
>    effective only in main worktree, is gone. These config files are
>    supposed to be in config.worktree.

"These config _settings_ are supposed to be..."

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> @@ -2,8 +2,9 @@ CONFIGURATION FILE
>  The Git configuration file contains a number of variables that affect
> -the Git commands' behavior. The `.git/config` file in each repository
> -is used to store the configuration for that repository, and
> +the Git commands' behavior. The files `.git/config` and optionally
> +`config.worktree` (see `extensions.worktreeConfig` below) are each

s/are/in/

> +repository is used to store the configuration for that repository, and
>  `$HOME/.gitconfig` is used to store a per-user configuration as
> diff --git a/t/t2029-worktree-config.sh b/t/t2029-worktree-config.sh
> @@ -0,0 +1,82 @@
> +cmp_config() {
> +       if [ "$1" = "-C" ]; then

Style:

    if test "$1" = "-C"
    then
        ...

> +}
> +
> +test_expect_success 'setup' '
> +       test_commit start &&
> +       git config --worktree per.worktree is-ok &&

Did you want:

    cmp_config false extensions.worktreeConfig

at this point in the test or something? It's not clear what the
intention is here.

> +       git worktree add wt1 &&
> +       git worktree add wt2 &&
> +       git config --worktree per.worktree is-ok &&
> +       cmp_config true extensions.worktreeConfig
> +'
> +test_expect_success 'core.bare no longer for main only' '
> +       git config core.bare true &&
> +       cmp_config true core.bare &&
> +       cmp_config -C wt1 true core.bare &&
> +       cmp_config -C wt2 true core.bare &&
> +       git config --unset core.bare

Should this be?

    test_when_finished "git config --unset core.bare" &&

or perhaps test_config()?

> +'
Taylor Blau Sept. 24, 2018, 2:21 p.m. UTC | #2
On Sun, Sep 23, 2018 at 07:04:38PM +0200, Nguyễn Thái Ngọc Duy wrote:
> A new repo extension is added, worktreeConfig. When it is present:

I was initially scratching my head at why this should be a repository
extension, but...

>  - The special treatment for core.bare and core.worktree, to stay
>    effective only in main worktree, is gone. These config files are
>    supposed to be in config.worktree.

This seems to be sufficient justification for it. A destructive action
(such as migrating configuration from one location to another, as you
have implemented in the patch below) should be something that the user
opts into, rather than having happen automatically.

> This extension is most useful in multiple worktree setup because you
> now have an option to store per-worktree config (which is either
> .git/config.worktree for main worktree, or
> .git/worktrees/xx/config.worktree for linked ones).

This sounds quite useful for these situations.

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 8d85d1a324..c24abf5871 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2,8 +2,9 @@ CONFIGURATION FILE
>  ------------------
>
>  The Git configuration file contains a number of variables that affect
> -the Git commands' behavior. The `.git/config` file in each repository
> -is used to store the configuration for that repository, and
> +the Git commands' behavior. The files `.git/config` and optionally
> +`config.worktree` (see `extensions.worktreeConfig` below) are each
> +repository is used to store the configuration for that repository, and

Typo: 'are each'.

>  ENVIRONMENT
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index e2ee9fc21b..3f9112db56 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -204,6 +204,43 @@ working trees, it can be used to identify worktrees. For example if
>  you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
>  then "ghi" or "def/ghi" is enough to point to the former working tree.
>
> +CONFIGURATION FILE
> +------------------
> +By default, the repository "config" file is shared across all working
> +directories. If the config variables `core.bare` or `core.worktree`
> +are already present in the config file, they will be applied to the
> +main working directory only.
> +
> +In order to have configuration specific to working directories, you
> +can turn on "worktreeConfig" extension, e.g.:
> +
> +------------
> +$ git config extensions.worktreeConfig true
> +------------

Good, this matches my expectation from above.

> @@ -24,6 +25,7 @@ static char key_delim = ' ';
>  static char term = '\n';
>
>  static int use_global_config, use_system_config, use_local_config;
> +static int use_worktree_config;
>  static struct git_config_source given_config_source;
>  static int actions, type;
>  static char *default_value;
> @@ -123,6 +125,7 @@ static struct option builtin_config_options[] = {
>  	OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
>  	OPT_BOOL(0, "system", &use_system_config, N_("use system config file")),
>  	OPT_BOOL(0, "local", &use_local_config, N_("use repository config file")),
> +	OPT_BOOL(0, "worktree", &use_worktree_config, N_("use per-worktree config file")),
>  	OPT_STRING('f', "file", &given_config_source.file, N_("file"), N_("use given config file")),
>  	OPT_STRING(0, "blob", &given_config_source.blob, N_("blob-id"), N_("read config from given blob object")),
>  	OPT_GROUP(N_("Action")),
> @@ -602,6 +605,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
>  			     PARSE_OPT_STOP_AT_NON_OPTION);
>
>  	if (use_global_config + use_system_config + use_local_config +
> +	    use_worktree_config +
>  	    !!given_config_source.file + !!given_config_source.blob > 1) {

I feel like we're getting into "let's extract a function" territory,
here, since this line is growing in width. Perhaps:

  static int config_files_count()
  {
    return use_global_config + use_system_config + use_local_config +
  		use_worktree_config +
  		!!given_config_source.file +
  		!!given_config_source.blob;
  }

Simplifying the call to:

> diff --git a/t/t2029-worktree-config.sh b/t/t2029-worktree-config.sh
> new file mode 100755
> index 0000000000..4ebdf13cf9
> --- /dev/null
> +++ b/t/t2029-worktree-config.sh
> @@ -0,0 +1,82 @@
> +#!/bin/sh
> +
> +test_description="config file in multi worktree"
> +
> +. ./test-lib.sh
> +
> +cmp_config() {
> +	if [ "$1" = "-C" ]; then
> +		shift &&
> +		GD="-C $1" &&
> +		shift
> +	else
> +		GD=
> +	fi &&
> +	echo "$1" >expected &&
> +	shift &&
> +	git $GD config "$@" >actual &&
> +	test_cmp expected actual
> +}

This cmp_config seems generally useful, perhaps beyond t2029. What do
you think about putting it in t/test-lib-functions.sh instead?

Thanks,
Taylor
Duy Nguyen Sept. 25, 2018, 3:57 p.m. UTC | #3
On Mon, Sep 24, 2018 at 4:21 PM Taylor Blau <me@ttaylorr.com> wrote:
> > +cmp_config() {
> > +     if [ "$1" = "-C" ]; then
> > +             shift &&
> > +             GD="-C $1" &&
> > +             shift
> > +     else
> > +             GD=
> > +     fi &&
> > +     echo "$1" >expected &&
> > +     shift &&
> > +     git $GD config "$@" >actual &&
> > +     test_cmp expected actual
> > +}
>
> This cmp_config seems generally useful, perhaps beyond t2029. What do
> you think about putting it in t/test-lib-functions.sh instead?

Good point. t1300 (and I think some t13xx) does the same. Other tests also do

    test "$(git config...)" = blah

which can also use this function to have better error reporting when
things go wrong.
Junio C Hamano Sept. 25, 2018, 9:26 p.m. UTC | #4
Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> This extension is most useful in multiple worktree setup because you
> now have an option to store per-worktree config (which is either
> .git/config.worktree for main worktree, or
> .git/worktrees/xx/config.worktree for linked ones).

Heh.  "This is useful if you have multiple" is true without saying,
because this is totally useless if you have only a single worktree.
I'd suggest writing the above without "most useful".

> This design places a bet on the assumption that the majority of config
> variables are shared so it is the default mode. A safer move would be
> default writes go to per-worktree file, so that accidental changes are
> isolated.

Warning: devil's advocate mode on.

Done in either way, this will confuse the users.  What is the reason
why people are led to think it is a good idea to use multiple
worktrees, even when they need different settings?  What do they
want out of "multiple worktrees linked to a single repository" as
opposed to just a simple "as many clones as necessary"?  Reduced
disk footprint?  Is there a better way to achieve that without the
downside of multiple worktrees (e.g. configuration need to be
uniform)?

> (*) "git config --worktree" points back to "config" file when this
>     extension is not present so that it works in any setup.

Shouldn't it barf and error out instead?  A user who hasn't enabled
the extension uses --worktree option and misled to believe that the
setting affects only a single worktree, even though the change is
made globally---that does not sound like a great end-user experience.
Duy Nguyen Sept. 26, 2018, 3:48 p.m. UTC | #5
On Tue, Sep 25, 2018 at 11:26 PM Junio C Hamano <gitster@pobox.com> wrote:
> Warning: devil's advocate mode on.

Oh good. I need to a good dose of sanity anyway.

> Done in either way, this will confuse the users.  What is the reason
> why people are led to think it is a good idea to use multiple
> worktrees, even when they need different settings?  What do they
> want out of "multiple worktrees linked to a single repository" as
> opposed to just a simple "as many clones as necessary"?  Reduced
> disk footprint?

I believe the main selling point of multiple worktrees is sharing
refs. You could easily avoid expensive clones with --local, but
synchronizing between different clones is not very convenient. Other
than that, different worktrees tend to behave like separate clones.

This leaves a gray area where other things should be shared or not. I
think the preference (or default mode) is still _not_ shared (*).
Sharing more things (besides refs and object database) is just a new
opportunity popping up when we implement multiple worktrees. Since
multiple worktrees (or clones before its time) are grouped together,
sometimes you would like to share common configuration. We could sort
of achieve this already with includeIf but again not as convenient.

(*) real life is not that simple. Since refs are shared, including
_remotes_ refs, so configuration related to remotes should also be
shared, or it will be a maintenance nightmare. Which is why
$GIT_DIR/config so far has been shared besides the two exceptions that
are core.bare and core.worktree. And I really like to get rid of these
exceptions.

> Is there a better way to achieve that without the
> downside of multiple worktrees (e.g. configuration need to be
> uniform)?

Is there a better way to achieve sharing refs between clones? I gave
it a minute but couldn't come up with a good answer :(

> > (*) "git config --worktree" points back to "config" file when this
> >     extension is not present so that it works in any setup.
>
> Shouldn't it barf and error out instead?

The intention is a uniform interface/api that works with both single
and multiple worktrees configurations. Otherwise in your scripts you
would need to write "if single worktree, do this, else do that". If
"git config --worktree" works with both, the existing scripts can be
audited and updated just a bit, adding "--worktree" where the config
should not be shared, and we're done.

> A user who hasn't enabled
> the extension uses --worktree option and misled to believe that the
> setting affects only a single worktree, even though the change is
> made globally---that does not sound like a great end-user experience.

I was talking about a single worktree. But I think here you meant the
user has multiple worktrees, but the extension is not enabled. I'm
probably not clear in the commit message, but "git config" can detect
that the extension has not been enabled, automatically enable it (and
move core.bare and core.worktree (if present) to the main worktree's
private config), so "git config --worktree" will never share the
change.

But perhaps the user should be made aware of this situation and asked
to explicitly enable the extension instead? It's certainly a more
conservative approach.
Junio C Hamano Sept. 26, 2018, 5:40 p.m. UTC | #6
Duy Nguyen <pclouds@gmail.com> writes:

> I believe the main selling point of multiple worktrees is sharing
> refs. You could easily avoid expensive clones with --local, but
> synchronizing between different clones is not very convenient. Other
> than that, different worktrees tend to behave like separate clones.

OK.  Even with the enforced limitation that no single branch can be
checked out in multiple worktrees at the same time, it is more
convenient as you can "merge" other branch and trust that the result
on the checked-out branch in your worktree is immediately visible to
other worktrees.

> user has multiple worktrees, but the extension is not enabled. I'm

Exactly.  "config --worktree" in your script will silently break
other worktrees; it wanted to affect only the current worktree, but
it changed settings to all the others.

> probably not clear in the commit message, but "git config" can detect
> that the extension has not been enabled, automatically enable it (and
> move core.bare and core.worktree (if present) to the main worktree's
> private config), so "git config --worktree" will never share the
> change.

I wonder if that is good enough.  The user in one worktree did
"config --worktree" and all the worktrees now start diverging in
their config---but that is true _only_ when they use "--worktree"
option to say "I want this to be set differently from others".  All
the other calls to "git config" will stil be shared.

So from a cursory thinking, it sounds OK to me, but somebody else
may think of bad ramifications after a good night sleep.
Ævar Arnfjörð Bjarmason Sept. 26, 2018, 6:25 p.m. UTC | #7
On Sun, Sep 23 2018, Nguyễn Thái Ngọc Duy wrote:

> +extensions.worktreeConfig::
> +	If set, by default "git config" reads from both "config" and
> +	"config.worktree" file in that order.

How does this interact with config that's now only used if it's in
.git/config? E.g. you can't set remote.<remote>.<url> in ~/.gitconfig,
will that be inherited across the two of these?

> In multiple working
> +	directory mode, "config" file is shared while
> +	"config.worktree" is per-working directory.

"But then how will it work with more than one?" I found myself thinking
before reading some more and remembering .git/worktree. Shouldn't we
consistently say:

    [...]"config" and "worktrees/<worktree name>/config"[...]

Or something like that?
Marc Branchaud Sept. 27, 2018, 3:24 p.m. UTC | #8
On 2018-09-26 11:48 AM, Duy Nguyen wrote:
> 
> I believe the main selling point of multiple worktrees is sharing
> refs. You could easily avoid expensive clones with --local, but
> synchronizing between different clones is not very convenient. Other
> than that, different worktrees tend to behave like separate clones.

Sharing hooks is also useful, but yes mainly the refs.

I love being able to work in more than one branch at a time.  I often 
have a couple of ongoing big, messy topics, and being able to easily 
jump onto some release branch for a quick bugfix, without having to 
first stash things or finish an interactive rebase or fix a conflicting 
merge, is a godsend.

And the reason I use worktrees for this, instead of clones, is for the 
shared refs.  It makes sense to me that I'm working with different 
checkouts from a single repo, with all my local branches and local tags. 
  "git fetch" updates the remote refs regardless of which worktree I'm 
in when I run it.  The setup is lightweight and efficient; it's just how 
I want to work.

Having used git-new-workdir for a long time, it's main deficiency for me 
is submodules (the shared bisection state didn't bother me much).  It 
would be nice if all my worktrees' submodules also shared refs.  That's 
"nice", but not "essential".  Mainly it would be convenient if a 
recursive-submodule fetch performed in one worktree updated the 
submodule refs in my other worktrees.  Similarly, if I create a local 
branch in a submodule in one worktree, it would be nice to see that 
branch in the submodule in other worktrees.  Again, "nice", but probably 
just because I've lived with git-new-workdir's limitations for so long 
that I'm used to them.

That said, I really appreciate Duy's work here -- thanks!  Git deserves 
to have a cool feature like worktrees be part of its standard toolkit.

		M.


> This leaves a gray area where other things should be shared or not. I
> think the preference (or default mode) is still _not_ shared (*).
> Sharing more things (besides refs and object database) is just a new
> opportunity popping up when we implement multiple worktrees. Since
> multiple worktrees (or clones before its time) are grouped together,
> sometimes you would like to share common configuration. We could sort
> of achieve this already with includeIf but again not as convenient.
> 
> (*) real life is not that simple. Since refs are shared, including
> _remotes_ refs, so configuration related to remotes should also be
> shared, or it will be a maintenance nightmare. Which is why
> $GIT_DIR/config so far has been shared besides the two exceptions that
> are core.bare and core.worktree. And I really like to get rid of these
> exceptions.
> 
>> Is there a better way to achieve that without the
>> downside of multiple worktrees (e.g. configuration need to be
>> uniform)?
> 
> Is there a better way to achieve sharing refs between clones? I gave
> it a minute but couldn't come up with a good answer :(
> 
>>> (*) "git config --worktree" points back to "config" file when this
>>>      extension is not present so that it works in any setup.
>>
>> Shouldn't it barf and error out instead?
> 
> The intention is a uniform interface/api that works with both single
> and multiple worktrees configurations. Otherwise in your scripts you
> would need to write "if single worktree, do this, else do that". If
> "git config --worktree" works with both, the existing scripts can be
> audited and updated just a bit, adding "--worktree" where the config
> should not be shared, and we're done.
> 
>> A user who hasn't enabled
>> the extension uses --worktree option and misled to believe that the
>> setting affects only a single worktree, even though the change is
>> made globally---that does not sound like a great end-user experience.
> 
> I was talking about a single worktree. But I think here you meant the
> user has multiple worktrees, but the extension is not enabled. I'm
> probably not clear in the commit message, but "git config" can detect
> that the extension has not been enabled, automatically enable it (and
> move core.bare and core.worktree (if present) to the main worktree's
> private config), so "git config --worktree" will never share the
> change.
> 
> But perhaps the user should be made aware of this situation and asked
> to explicitly enable the extension instead? It's certainly a more
> conservative approach.
>
Duy Nguyen Sept. 27, 2018, 4:36 p.m. UTC | #9
On Thu, Sep 27, 2018 at 5:24 PM Marc Branchaud <marcnarc@xiplink.com> wrote:
>
> On 2018-09-26 11:48 AM, Duy Nguyen wrote:
> >
> > I believe the main selling point of multiple worktrees is sharing
> > refs. You could easily avoid expensive clones with --local, but
> > synchronizing between different clones is not very convenient. Other
> > than that, different worktrees tend to behave like separate clones.
>
> Sharing hooks is also useful

Well yes, but for hooks I think a better way is moving hook management
back to config files. I think we have a rough idea what to do, and
AEvar highlighted the mand roadblocks elsewhere. Hopefully it will
materialize someday.

> Having used git-new-workdir for a long time, it's main deficiency for me
> is submodules (the shared bisection state didn't bother me much).  It
> would be nice if all my worktrees' submodules also shared refs.  That's
> "nice", but not "essential".

Heh I've been thinking about this a bit too. I thought separate refs
was a requirement for submodules, but if you don't actively use
branches in submodules and go with detached HEAD, it might work.
Duy Nguyen Sept. 27, 2018, 5:24 p.m. UTC | #10
On Wed, Sep 26, 2018 at 8:25 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Sun, Sep 23 2018, Nguyễn Thái Ngọc Duy wrote:
>
> > +extensions.worktreeConfig::
> > +     If set, by default "git config" reads from both "config" and
> > +     "config.worktree" file in that order.
>
> How does this interact with config that's now only used if it's in
> .git/config? E.g. you can't set remote.<remote>.<url> in ~/.gitconfig,
> will that be inherited across the two of these?

Er... we can't? If I remember correctly we don't have any enforcement
on where what config vars must or must not go. The only exception is
core.bare and core.worktree which is only read from $GIT_DIR/config
because of the way they are involved in .git directory discovery. If I
put remote "foo" in ~/.gitconfig, "git remote" happily reports remote
"foo" to me.

To sum up, we always inherit config from higher levels, with
/etc/gitconfig being the highest and $GIT_DIR/config the lowest. It's
up to the user to share config between repos responsibly. This patch
only adds one more level, $GIT_DIR/config.worktree which is now the
lowest level.

> > In multiple working
> > +     directory mode, "config" file is shared while
> > +     "config.worktree" is per-working directory.
>
> "But then how will it work with more than one?" I found myself thinking
> before reading some more and remembering .git/worktree. Shouldn't we
> consistently say:
>
>     [...]"config" and "worktrees/<worktree name>/config"[...]
>
> Or something like that?

Point taken. Maybe I'm trying to hide implementation details too much.
Ævar Arnfjörð Bjarmason Sept. 27, 2018, 6:34 p.m. UTC | #11
On Thu, Sep 27 2018, Duy Nguyen wrote:

> On Wed, Sep 26, 2018 at 8:25 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> On Sun, Sep 23 2018, Nguyễn Thái Ngọc Duy wrote:
>>
>> > +extensions.worktreeConfig::
>> > +     If set, by default "git config" reads from both "config" and
>> > +     "config.worktree" file in that order.
>>
>> How does this interact with config that's now only used if it's in
>> .git/config? E.g. you can't set remote.<remote>.<url> in ~/.gitconfig,
>> will that be inherited across the two of these?
>
> Er... we can't? If I remember correctly we don't have any enforcement
> on where what config vars must or must not go. The only exception is
> core.bare and core.worktree which is only read from $GIT_DIR/config
> because of the way they are involved in .git directory discovery. If I
> put remote "foo" in ~/.gitconfig, "git remote" happily reports remote
> "foo" to me.
>
> To sum up, we always inherit config from higher levels, with
> /etc/gitconfig being the highest and $GIT_DIR/config the lowest. It's
> up to the user to share config between repos responsibly. This patch
> only adds one more level, $GIT_DIR/config.worktree which is now the
> lowest level.

I see I'm misremembering most of the details here. I thought that if I put:

    [remote "whatever]
    url = ...

Into my ~/.gitconfig that it wouldn't work, but it does, e.g. here in my
~/g/git:

    $ grep -A1 whatever .git/config
    $
    $ grep -A1 whatever ~/.gitconfig
    [remote "whatever"]
        url = git@github.com:test/git.git

But there's still some special casing for .git/config going on,
e.g. here:

    $ git config remote.origin.url
    git@github.com:git/git.git
    $ git config remote.whatever.url
    git@github.com:test/git.git
    $ git remote get-url origin
    git@github.com:git/git.git
    $ git remote get-url whatever
    fatal: No such remote 'whatever'

And:

    $ git remote set-url whatever git@github.com:test2/git.git
    fatal: No such remote 'whatever'

So there is some special casing of .git/config somewhere. I looked into
this ages ago, and don't remember where that's done.

I was wondering if these patches introduced any unwanted edge cases in
this regard, e.g. if you're using the per-worktree config, and you have
remotes in .git/config, does "git remote get/set-url" do the right
thing?

Then if we have remotes in .git/config, should we add new remotes to
.git/config or the per-worktree file? I'd lean towards .git/config,
since remotes are orthagonal to worktrees, and closely tied with refs
which are shared no matter what this extension says, but maybe there's a
good argument for doing it the other way around.



>> > In multiple working
>> > +     directory mode, "config" file is shared while
>> > +     "config.worktree" is per-working directory.
>>
>> "But then how will it work with more than one?" I found myself thinking
>> before reading some more and remembering .git/worktree. Shouldn't we
>> consistently say:
>>
>>     [...]"config" and "worktrees/<worktree name>/config"[...]
>>
>> Or something like that?
>
> Point taken. Maybe I'm trying to hide implementation details too much.
Duy Nguyen Sept. 27, 2018, 6:49 p.m. UTC | #12
On Thu, Sep 27, 2018 at 8:34 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> ...
>
> So there is some special casing of .git/config somewhere. I looked into
> this ages ago, and don't remember where that's done.

Thanks! At least know I have some clues to look into (and will do).
Duy Nguyen Sept. 29, 2018, 6:36 a.m. UTC | #13
On Thu, Sep 27, 2018 at 8:34 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> I see I'm misremembering most of the details here. I thought that if I put:
>
>     [remote "whatever]
>     url = ...
>
> Into my ~/.gitconfig that it wouldn't work, but it does, e.g. here in my
> ~/g/git:
>
>     $ grep -A1 whatever .git/config
>     $
>     $ grep -A1 whatever ~/.gitconfig
>     [remote "whatever"]
>         url = git@github.com:test/git.git
>
> But there's still some special casing for .git/config going on,
> e.g. here:
>
>     $ git config remote.origin.url
>     git@github.com:git/git.git
>     $ git config remote.whatever.url
>     git@github.com:test/git.git
>     $ git remote get-url origin
>     git@github.com:git/git.git
>     $ git remote get-url whatever
>     fatal: No such remote 'whatever'
>
> And:
>
>     $ git remote set-url whatever git@github.com:test2/git.git
>     fatal: No such remote 'whatever'
>
> So there is some special casing of .git/config somewhere. I looked into
> this ages ago, and don't remember where that's done.

To conclude this thread. Yes some code does know about where the
config variable is from and it looks like only "git remote" and "git
upload-pack" takes advantage of it [1] [2].

I considered documentation improvement for the git-remote part, but I
don't see anywhere I can fit "some remote attributes can be shared
from ~/.gitconfig, but a remote can only be added from repo-level
config" in. Jeff already documented it well. I found one minor problem
there in the documentation, which I will send separately.

Thanks again for making me aware of this.

[1] e459b073fb (remote rename: more carefully determine whether a
remote is configured - 2017-01-19)
[2] 20b20a22f8 (upload-pack: provide a hook for running pack-objects -
2016-05-18)
Duy Nguyen Sept. 29, 2018, 1:53 p.m. UTC | #14
On Mon, Sep 24, 2018 at 4:21 PM Taylor Blau <me@ttaylorr.com> wrote:
> > @@ -602,6 +605,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
> >                            PARSE_OPT_STOP_AT_NON_OPTION);
> >
> >       if (use_global_config + use_system_config + use_local_config +
> > +         use_worktree_config +
> >           !!given_config_source.file + !!given_config_source.blob > 1) {
>
> I feel like we're getting into "let's extract a function" territory,
> here, since this line is growing in width. Perhaps:
>
>   static int config_files_count()
>   {
>     return use_global_config + use_system_config + use_local_config +
>                 use_worktree_config +
>                 !!given_config_source.file +
>                 !!given_config_source.blob;
>   }
>
> Simplifying the call to:

I think there's a bigger problem here because we have no good way to
describe that a bunch of options are mutually exclusive. We get by ok
if the number of options is two, but when it gets bigger (and this is
not the only place), it gets messier. Besides the long "if" condition,
I consider the error message "only one config file at a time"
inadequate. We should tell the user what exact options are
conflicting.

So, I'm going to bury my head in the sand this time and ignore the
problem, including adding config_files_count(). It could be a
interesting mini project if someone wants to jump in. Meanwhile it'll
go to the very bottom of my long long backlog.

Patch
diff mbox series

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 8d85d1a324..c24abf5871 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2,8 +2,9 @@  CONFIGURATION FILE
 ------------------
 
 The Git configuration file contains a number of variables that affect
-the Git commands' behavior. The `.git/config` file in each repository
-is used to store the configuration for that repository, and
+the Git commands' behavior. The files `.git/config` and optionally
+`config.worktree` (see `extensions.worktreeConfig` below) are each
+repository is used to store the configuration for that repository, and
 `$HOME/.gitconfig` is used to store a per-user configuration as
 fallback values for the `.git/config` file. The file `/etc/gitconfig`
 can be used to store a system-wide default configuration.
@@ -371,6 +372,12 @@  advice.*::
 		editor input from the user.
 --
 
+extensions.worktreeConfig::
+	If set, by default "git config" reads from both "config" and
+	"config.worktree" file in that order. In multiple working
+	directory mode, "config" file is shared while
+	"config.worktree" is per-working directory.
+
 core.fileMode::
 	Tells Git if the executable bit of files in the working tree
 	is to be honored.
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 8e240435be..4870e00b89 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -45,13 +45,15 @@  unset an existing `--type` specifier with `--no-type`.
 
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options
-`--system`, `--global`, `--local` and `--file <filename>` can be
-used to tell the command to read from only that location (see <<FILES>>).
+`--system`, `--global`, `--local`, `--worktree` and
+`--file <filename>` can be used to tell the command to read from only
+that location (see <<FILES>>).
 
 When writing, the new value is written to the repository local
 configuration file by default, and options `--system`, `--global`,
-`--file <filename>` can be used to tell the command to write to
-that location (you can say `--local` but that is the default).
+`--worktree`, `--file <filename>` can be used to tell the command to
+write to that location (you can say `--local` but that is the
+default).
 
 This command will fail with non-zero status upon error.  Some exit
 codes are:
@@ -131,6 +133,11 @@  from all available files.
 +
 See also <<FILES>>.
 
+--worktree::
+	Similar to `--local` except that `.git/config.worktree` is
+	read from or written to if `extensions.worktreeConfig` is
+	present. If not it's the same as `--local`.
+
 -f config-file::
 --file config-file::
 	Use the given config file instead of the one specified by GIT_CONFIG.
@@ -281,6 +288,10 @@  $XDG_CONFIG_HOME/git/config::
 $GIT_DIR/config::
 	Repository specific configuration file.
 
+$GIT_DIR/config.worktree::
+	This is optional and is only searched when
+	`extensions.worktreeConfig` is present in $GIT_DIR/config.
+
 If no further options are given, all reading options will read all of these
 files that are available. If the global or the system-wide configuration
 file are not available they will be ignored. If the repository configuration
@@ -299,9 +310,10 @@  configuration file. Note that this also affects options like `--replace-all`
 and `--unset`. *'git config' will only ever change one file at a time*.
 
 You can override these rules either by command-line options or by environment
-variables. The `--global` and the `--system` options will limit the file used
-to the global or system-wide file respectively. The `GIT_CONFIG` environment
-variable has a similar effect, but you can specify any filename you want.
+variables. The `--global`, `--system` and `--worktree` options will limit
+the file used to the global, system-wide or per-worktree file respectively.
+The `GIT_CONFIG` environment variable has a similar effect, but you
+can specify any filename you want.
 
 
 ENVIRONMENT
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e2ee9fc21b..3f9112db56 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -204,6 +204,43 @@  working trees, it can be used to identify worktrees. For example if
 you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
 then "ghi" or "def/ghi" is enough to point to the former working tree.
 
+CONFIGURATION FILE
+------------------
+By default, the repository "config" file is shared across all working
+directories. If the config variables `core.bare` or `core.worktree`
+are already present in the config file, they will be applied to the
+main working directory only.
+
+In order to have configuration specific to working directories, you
+can turn on "worktreeConfig" extension, e.g.:
+
+------------
+$ git config extensions.worktreeConfig true
+------------
+
+In this mode, specific configuration stays in the path pointed by `git
+rev-parse --git-path config.worktree`. You can add or update
+configuration in this file with `git config --worktree`. Git before
+version 2.20.0 will refuse to access repositories with this extension.
+
+Note that in this file, the exception for `core.bare` and
+`core.worktree` is gone. If you have them before, you need to move
+them to the `config.worktree` of the main working directory. You may
+also take this opportunity to move other configuration that you do not
+want to share to all working directories:
+
+ - `core.worktree` and `core.bare` should never be shared
+
+ - `core.sparseCheckout` is recommended per working directory, unless
+   you are sure you always use sparse checkout for all working
+   directories.
+
+When `git config --worktree` is used to set a configuration variable
+in multiple working directory setup, `extensions.worktreeConfig` will
+be automatically set. The two variables `core.worktree` and
+`core.bare` if present will be moved to `config.worktree` of the main
+working tree.
+
 DETAILS
 -------
 Each linked working tree has a private sub-directory in the repository's
diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
index e85148f05e..36fcca8087 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -143,6 +143,11 @@  config::
 	if $GIT_COMMON_DIR is set and "$GIT_COMMON_DIR/config" will be
 	used instead.
 
+config.worktree::
+	Working directory specific configuration file for the main
+	working directory in multiple working directory setup (see
+	linkgit:git-worktree[1]).
+
 branches::
 	A slightly deprecated way to store shorthands to be used
 	to specify a URL to 'git fetch', 'git pull' and 'git push'.
@@ -275,6 +280,9 @@  worktrees/<id>/locked::
 	or manually by `git worktree prune`. The file may contain a string
 	explaining why the repository is locked.
 
+worktrees/<id>/config.worktree::
+	Working directory specific configuration file.
+
 SEE ALSO
 --------
 linkgit:git-init[1],
diff --git a/builtin/config.c b/builtin/config.c
index 97b58c4aea..337db7b552 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -5,6 +5,7 @@ 
 #include "parse-options.h"
 #include "urlmatch.h"
 #include "quote.h"
+#include "worktree.h"
 
 static const char *const builtin_config_usage[] = {
 	N_("git config [<options>]"),
@@ -24,6 +25,7 @@  static char key_delim = ' ';
 static char term = '\n';
 
 static int use_global_config, use_system_config, use_local_config;
+static int use_worktree_config;
 static struct git_config_source given_config_source;
 static int actions, type;
 static char *default_value;
@@ -123,6 +125,7 @@  static struct option builtin_config_options[] = {
 	OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
 	OPT_BOOL(0, "system", &use_system_config, N_("use system config file")),
 	OPT_BOOL(0, "local", &use_local_config, N_("use repository config file")),
+	OPT_BOOL(0, "worktree", &use_worktree_config, N_("use per-worktree config file")),
 	OPT_STRING('f', "file", &given_config_source.file, N_("file"), N_("use given config file")),
 	OPT_STRING(0, "blob", &given_config_source.blob, N_("blob-id"), N_("read config from given blob object")),
 	OPT_GROUP(N_("Action")),
@@ -602,6 +605,7 @@  int cmd_config(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 
 	if (use_global_config + use_system_config + use_local_config +
+	    use_worktree_config +
 	    !!given_config_source.file + !!given_config_source.blob > 1) {
 		error(_("only one config file at a time"));
 		usage_builtin_config();
@@ -645,7 +649,17 @@  int cmd_config(int argc, const char **argv, const char *prefix)
 		given_config_source.file = git_etc_gitconfig();
 	else if (use_local_config)
 		given_config_source.file = git_pathdup("config");
-	else if (given_config_source.file) {
+	else if (use_worktree_config) {
+		struct worktree **worktrees = get_worktrees(0);
+		if (repository_format_worktree_config)
+			given_config_source.file = git_pathdup("config.worktree");
+		else if (worktrees[0] && worktrees[1]) {
+			migrate_worktree_config(the_repository);
+			given_config_source.file = git_pathdup("config.worktree");
+		} else
+			given_config_source.file = git_pathdup("config");
+		free_worktrees(worktrees);
+	} else if (given_config_source.file) {
 		if (!is_absolute_path(given_config_source.file) && prefix)
 			given_config_source.file =
 				prefix_filename(prefix, given_config_source.file);
diff --git a/cache.h b/cache.h
index d508f3d4f8..9e9b917e99 100644
--- a/cache.h
+++ b/cache.h
@@ -957,11 +957,13 @@  extern int grafts_replace_parents;
 extern int repository_format_precious_objects;
 extern char *repository_format_partial_clone;
 extern const char *core_partial_clone_filter_default;
+extern int repository_format_worktree_config;
 
 struct repository_format {
 	int version;
 	int precious_objects;
 	char *partial_clone; /* value of extensions.partialclone */
+	int worktree_config;
 	int is_bare;
 	int hash_algo;
 	char *work_tree;
diff --git a/config.c b/config.c
index 3461993f0a..24f68a5208 100644
--- a/config.c
+++ b/config.c
@@ -1695,6 +1695,13 @@  static int do_git_config_sequence(const struct config_options *opts,
 	if (repo_config && !access_or_die(repo_config, R_OK, 0))
 		ret += git_config_from_file(fn, repo_config, data);
 
+	if (repository_format_worktree_config) {
+		char *path = git_pathdup("config.worktree");
+		if (!access_or_die(path, R_OK, 0))
+			ret += git_config_from_file(fn, path, data);
+		free(path);
+	}
+
 	current_parsing_scope = CONFIG_SCOPE_CMDLINE;
 	if (git_config_from_parameters(fn, data) < 0)
 		die(_("unable to parse command-line config"));
diff --git a/environment.c b/environment.c
index 3f3c8746c2..268310b3dc 100644
--- a/environment.c
+++ b/environment.c
@@ -33,6 +33,7 @@  int ref_paranoia = -1;
 int repository_format_precious_objects;
 char *repository_format_partial_clone;
 const char *core_partial_clone_filter_default;
+int repository_format_worktree_config;
 const char *git_commit_encoding;
 const char *git_log_output_encoding;
 const char *apply_default_whitespace;
diff --git a/setup.c b/setup.c
index b24c811c1c..6169193946 100644
--- a/setup.c
+++ b/setup.c
@@ -423,7 +423,9 @@  static int check_repo_format(const char *var, const char *value, void *vdata)
 			if (!value)
 				return config_error_nonbool(var);
 			data->partial_clone = xstrdup(value);
-		} else
+		} else if (!strcmp(ext, "worktreeconfig"))
+			data->worktree_config = git_config_bool(var, value);
+		else
 			string_list_append(&data->unknown_extensions, ext);
 	} else if (strcmp(var, "core.bare") == 0) {
 		data->is_bare = git_config_bool(var, value);
@@ -466,6 +468,7 @@  static int check_repository_format_gently(const char *gitdir, struct repository_
 
 	repository_format_precious_objects = candidate->precious_objects;
 	repository_format_partial_clone = candidate->partial_clone;
+	repository_format_worktree_config = candidate->worktree_config;
 	string_list_clear(&candidate->unknown_extensions, 0);
 	if (!has_common) {
 		if (candidate->is_bare != -1) {
diff --git a/t/t2029-worktree-config.sh b/t/t2029-worktree-config.sh
new file mode 100755
index 0000000000..4ebdf13cf9
--- /dev/null
+++ b/t/t2029-worktree-config.sh
@@ -0,0 +1,82 @@ 
+#!/bin/sh
+
+test_description="config file in multi worktree"
+
+. ./test-lib.sh
+
+cmp_config() {
+	if [ "$1" = "-C" ]; then
+		shift &&
+		GD="-C $1" &&
+		shift
+	else
+		GD=
+	fi &&
+	echo "$1" >expected &&
+	shift &&
+	git $GD config "$@" >actual &&
+	test_cmp expected actual
+}
+
+test_expect_success 'setup' '
+	test_commit start &&
+	git config --worktree per.worktree is-ok &&
+	git worktree add wt1 &&
+	git worktree add wt2 &&
+	git config --worktree per.worktree is-ok &&
+	cmp_config true extensions.worktreeConfig
+'
+
+test_expect_success 'config is shared as before' '
+	git config this.is shared &&
+	cmp_config shared this.is &&
+	cmp_config -C wt1 shared this.is &&
+	cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config is shared (set from another worktree)' '
+	git -C wt1 config that.is also-shared &&
+	cmp_config also-shared that.is &&
+	cmp_config -C wt1 also-shared that.is &&
+	cmp_config -C wt2 also-shared that.is
+'
+
+test_expect_success 'config private to main worktree' '
+	git config --worktree this.is for-main &&
+	cmp_config for-main this.is &&
+	cmp_config -C wt1 shared this.is &&
+	cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config private to linked worktree' '
+	git -C wt1 config --worktree this.is for-wt1 &&
+	cmp_config for-main this.is &&
+	cmp_config -C wt1 for-wt1 this.is &&
+	cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'core.bare no longer for main only' '
+	git config core.bare true &&
+	cmp_config true core.bare &&
+	cmp_config -C wt1 true core.bare &&
+	cmp_config -C wt2 true core.bare &&
+	git config --unset core.bare
+'
+
+test_expect_success 'config.worktree no longer read without extension' '
+	git config --unset extensions.worktreeConfig &&
+	cmp_config shared this.is &&
+	cmp_config -C wt1 shared this.is &&
+	cmp_config -C wt2 shared this.is
+'
+
+test_expect_success 'config --worktree migrate core.bare and core.worktree' '
+	git config core.bare true &&
+	git config --worktree foo.bar true &&
+	cmp_config true extensions.worktreeConfig &&
+	cmp_config true foo.bar &&
+	cmp_config true core.bare &&
+	! git -C wt1 config core.bare
+'
+
+test_done
diff --git a/worktree.c b/worktree.c
index b0d0b5426d..a239f99bdc 100644
--- a/worktree.c
+++ b/worktree.c
@@ -5,6 +5,7 @@ 
 #include "worktree.h"
 #include "dir.h"
 #include "wt-status.h"
+#include "config.h"
 
 void free_worktrees(struct worktree **worktrees)
 {
@@ -508,3 +509,43 @@  int other_head_refs(each_ref_fn fn, void *cb_data)
 	free_worktrees(worktrees);
 	return ret;
 }
+
+void migrate_worktree_config(struct repository *r)
+{
+	struct strbuf worktree_path = STRBUF_INIT;
+	struct strbuf main_path = STRBUF_INIT;
+	struct repository_format format;
+
+	assert(repository_format_worktree_config == 0);
+
+	strbuf_git_common_path(&worktree_path, r, "config.worktree");
+	strbuf_git_path(&main_path, "config");
+
+	read_repository_format(&format, main_path.buf);
+	assert(format.worktree_config == 0);
+
+	if (format.is_bare >= 0) {
+		git_config_set_in_file(worktree_path.buf,
+				       "core.bare", "true");
+		git_config_set_in_file(main_path.buf,
+				       "core.bare", NULL);
+	}
+	if (format.work_tree) {
+		git_config_set_in_file(worktree_path.buf,
+				       "core.worktree",
+				       format.work_tree);
+		git_config_set_in_file(main_path.buf,
+				       "core.worktree", NULL);
+	}
+
+	git_config_set_in_file(main_path.buf,
+			       "extensions.worktreeConfig", "true");
+	if (format.version == 0)
+		git_config_set_in_file(main_path.buf,
+				       "core.repositoryFormatVersion", "1");
+
+	repository_format_worktree_config = 1;
+
+	strbuf_release(&main_path);
+	strbuf_release(&worktree_path);
+}
diff --git a/worktree.h b/worktree.h
index df3fc30f73..b27b407d1b 100644
--- a/worktree.h
+++ b/worktree.h
@@ -108,4 +108,10 @@  extern const char *worktree_git_path(const struct worktree *wt,
 				     const char *fmt, ...)
 	__attribute__((format (printf, 2, 3)));
 
+/*
+ * Called to add extensions.worktreeConfig to $GIT_DIR/config and move
+ * main worktree specific config variables to $GIT_DIR/config.worktree.
+ */
+void migrate_worktree_config(struct repository *r);
+
 #endif