Re: [PATCH v2 2/2] worktree: add per-worktree config files

2018-09-30 Thread Duy Nguyen
On Sun, Sep 30, 2018 at 9:24 AM Eric Sunshine  wrote:
>
> On Sun, Sep 30, 2018 at 3:16 AM Duy Nguyen  wrote:
> > On Sun, Sep 30, 2018 at 6:33 AM Eric Sunshine  
> > wrote:
> > > > diff --git a/builtin/config.c b/builtin/config.c
> > > > @@ -645,7 +649,20 @@ int cmd_config(int argc, const char **argv, const 
> > > > char *prefix)
> > > > +   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])
> > > > +   die(_("--worktree cannot be used with multiple "
> > > > + "working trees unless the config\n"
> > > > + "extension worktreeConfig is enabled. "
> > > > + "Please read \"CONFIGURATION FILE\"\n"
> > > > + "section in \"git help worktree\" for 
> > > > details"));
> > > > +   else
> > > > +   given_config_source.file = 
> > > > git_pathdup("config");
> > >
> > > I'm not sure I understand the purpose of allowing --worktree when only
> > > a single worktree is present and extensions.worktreeConfig is not set.
> > > Can you talk about it a bit more?
> >
> > Unified API. If I write a script to do stuff and want it to work in
> > multiple worktrees as well, I should not need to do "if single
> > worktree, use "git config --local", if multiple use "git config
> > --worktree"". By using "git config --worktree" I tell git "this config
> > is per-worktree" and git should do the right thing, single worktree or
> > not.
>
> That's what I thought, but I still don't understand how that unified
> API is going to help if the script writer happens to have multiple
> worktrees but doesn't have extensions.worktreeConfig set, in which
> case the command will error out. I don't see how that simplifies
> things, but perhaps I'm missing something obvious.

No it does not simplify that step. The user has to enable it manually
(at least for now). v1 tried to enable it automatically, but I think
it's safer to go one step at a time.
-- 
Duy


Re: [PATCH v2 2/2] worktree: add per-worktree config files

2018-09-30 Thread Eric Sunshine
On Sun, Sep 30, 2018 at 3:16 AM Duy Nguyen  wrote:
> On Sun, Sep 30, 2018 at 6:33 AM Eric Sunshine  wrote:
> > > diff --git a/builtin/config.c b/builtin/config.c
> > > @@ -645,7 +649,20 @@ int cmd_config(int argc, const char **argv, const 
> > > char *prefix)
> > > +   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])
> > > +   die(_("--worktree cannot be used with multiple "
> > > + "working trees unless the config\n"
> > > + "extension worktreeConfig is enabled. "
> > > + "Please read \"CONFIGURATION FILE\"\n"
> > > + "section in \"git help worktree\" for 
> > > details"));
> > > +   else
> > > +   given_config_source.file = git_pathdup("config");
> >
> > I'm not sure I understand the purpose of allowing --worktree when only
> > a single worktree is present and extensions.worktreeConfig is not set.
> > Can you talk about it a bit more?
>
> Unified API. If I write a script to do stuff and want it to work in
> multiple worktrees as well, I should not need to do "if single
> worktree, use "git config --local", if multiple use "git config
> --worktree"". By using "git config --worktree" I tell git "this config
> is per-worktree" and git should do the right thing, single worktree or
> not.

That's what I thought, but I still don't understand how that unified
API is going to help if the script writer happens to have multiple
worktrees but doesn't have extensions.worktreeConfig set, in which
case the command will error out. I don't see how that simplifies
things, but perhaps I'm missing something obvious.


Re: [PATCH v2 2/2] worktree: add per-worktree config files

2018-09-30 Thread Duy Nguyen
On Sun, Sep 30, 2018 at 6:33 AM Eric Sunshine  wrote:
> > diff --git a/builtin/config.c b/builtin/config.c
> > @@ -645,7 +649,20 @@ int cmd_config(int argc, const char **argv, const char 
> > *prefix)
> > +   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])
> > +   die(_("--worktree cannot be used with multiple "
> > + "working trees unless the config\n"
> > + "extension worktreeConfig is enabled. "
> > + "Please read \"CONFIGURATION FILE\"\n"
> > + "section in \"git help worktree\" for 
> > details"));
> > +   else
> > +   given_config_source.file = git_pathdup("config");
>
> I'm not sure I understand the purpose of allowing --worktree when only
> a single worktree is present and extensions.worktreeConfig is not set.
> Can you talk about it a bit more?

Unified API. If I write a script to do stuff and want it to work in
multiple worktrees as well, I should not need to do "if single
worktree, use "git config --local", if multiple use "git config
--worktree"". By using "git config --worktree" I tell git "this config
is per-worktree" and git should do the right thing, single worktree or
not.
-- 
Duy


Re: [PATCH v2 2/2] worktree: add per-worktree config files

2018-09-29 Thread Eric Sunshine
On Sat, Sep 29, 2018 at 11:30 AM Nguyễn Thái Ngọc Duy  wrote:
> A new repo extension is added, worktreeConfig. When it is present:
> [...]
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
> 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 files `.git/config` and optionally
> +`config.worktree` (see `extensions.worktreeConfig` below) in each
> +repository is used to store the configuration for that repository, and

s/is used/are/used/

>  `$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.
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> @@ -204,6 +204,36 @@ working trees, it can be used to identify worktrees. For 
> example if
> +CONFIGURATION FILE
> +--
> +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`. Older Git
> +versions may will refuse to access repositories with this extension.

s/may will/will/

> diff --git a/Documentation/gitrepository-layout.txt 
> b/Documentation/gitrepository-layout.txt
> @@ -275,6 +280,9 @@ worktrees//locked::
> +worktrees//config.worktree::
> +   Working directory specific configuration file.

I wonder if this deserves a quick mention in
Documentation/git-worktree.txt since other worktree-related files,
such as "worktrees//locked", are mentioned there.

> diff --git a/builtin/config.c b/builtin/config.c
> @@ -645,7 +649,20 @@ int cmd_config(int argc, const char **argv, const char 
> *prefix)
> +   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])
> +   die(_("--worktree cannot be used with multiple "
> + "working trees unless the config\n"
> + "extension worktreeConfig is enabled. "
> + "Please read \"CONFIGURATION FILE\"\n"
> + "section in \"git help worktree\" for 
> details"));
> +   else
> +   given_config_source.file = git_pathdup("config");

I'm not sure I understand the purpose of allowing --worktree when only
a single worktree is present and extensions.worktreeConfig is not set.
Can you talk about it a bit more?


[PATCH v2 2/2] worktree: add per-worktree config files

2018-09-29 Thread Nguyễn Thái Ngọc Duy
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 settings 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 ".

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 and there is only one worktree so that it
works in any both single and multiple worktree setups.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/config.txt   | 12 +++-
 Documentation/git-config.txt   | 26 ++---
 Documentation/git-worktree.txt | 30 ++
 Documentation/gitrepository-layout.txt |  8 +++
 builtin/config.c   | 19 ++-
 cache.h|  2 +
 config.c   | 11 
 environment.c  |  1 +
 setup.c| 40 ++---
 t/t2029-worktree-config.sh | 79 ++
 10 files changed, 210 insertions(+), 18 deletions(-)
 create mode 100755 t/t2029-worktree-config.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 8d85d1a324..44407e69db 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) in 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,13 @@ advice.*::
editor input from the user.
 --
 
+extensions.worktreeConfig::
+   If set, by default "git config" reads from both "config" and
+   "config.worktree" file from GIT_DIR in that order. In
+   multiple working directory mode, "config" file is shared while
+   "config.worktree" is per-working directory (i.e., it's in
+   GIT_COMMON_DIR/worktrees//config.worktree)
+
 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 ` can be
-used to tell the command to read from only that location (see <>).
+`--system`, `--global`, `--local`, `--worktree` and
+`--file ` can be used to tell the command to read from only
+that location (see <>).
 
 When writing, the new value is written to the repository local
 configuration file by default, and options `--system`, `--global`,
-`--file ` can be used to tell the command to write to
-that location (you can say `--local` but that is the default).
+`--worktree`, `--file ` 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