* [PATCH/RFD 0/3] worktree.* config keys and submodule and multiple worktrees
@ 2015-02-08 13:16 Nguyễn Thái Ngọc Duy
2015-02-08 13:16 ` [PATCH 1/3] config.c: new config namespace worktree.* stored in $GIT_DIR/config.worktree Nguyễn Thái Ngọc Duy
` (3 more replies)
0 siblings, 4 replies; 22+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2015-02-08 13:16 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jens Lehmann, Max Kirillov,
Nguyễn Thái Ngọc Duy
1/3 is a more complete version of a patch I posted earlier [1]. It
makes sure that worktree.* config keys are stored in a different place
than $GIT_DIR/config. This allows us to work around the core.worktree
issue in multiple worktree setting.
I think 1/3 and 2/3 are fine. 3/3 is probably not. It's more of a
proof of concept. The tests pass, but there's no migration path for
existing submodules. Submodules modified by new Git will confuse old
Git because the old ones do not understand worktree.path (the
replacement for core.worktree)
But I think it's a start..
[1] http://article.gmane.org/gmane.comp.version-control.git/263134
Nguyễn Thái Ngọc Duy (3):
config.c: new config namespace worktree.* stored in $GIT_DIR/config.worktree
setup: add worktree.path to shadow core.worktree
submodule: use worktree.path instead of core.worktree
Documentation/config.txt | 7 ++++++-
builtin/config.c | 8 +++++++
config.c | 38 ++++++++++++++++++++++++++++++++++
git-submodule.sh | 2 +-
setup.c | 7 ++++++-
submodule.c | 6 +++---
t/lib-submodule-update.sh | 8 +++----
t/t1300-repo-config.sh | 31 +++++++++++++++++++++++++++
t/t7400-submodule-basic.sh | 4 ++--
t/t7409-submodule-detached-worktree.sh | 6 +++---
10 files changed, 102 insertions(+), 15 deletions(-)
--
2.3.0.rc1.137.g477eb31
^ permalink raw reply [flat|nested] 22+ messages in thread* [PATCH 1/3] config.c: new config namespace worktree.* stored in $GIT_DIR/config.worktree 2015-02-08 13:16 [PATCH/RFD 0/3] worktree.* config keys and submodule and multiple worktrees Nguyễn Thái Ngọc Duy @ 2015-02-08 13:16 ` Nguyễn Thái Ngọc Duy 2015-02-08 13:16 ` [PATCH 2/3] setup: add worktree.path to shadow core.worktree Nguyễn Thái Ngọc Duy ` (2 subsequent siblings) 3 siblings, 0 replies; 22+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2015-02-08 13:16 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jens Lehmann, Max Kirillov, Nguyễn Thái Ngọc Duy On the surface, worktree.* variables are just like any other variables. You can read or modify them with `git config` command, or config_* API. Underneath, worktree.* are always stored in $GIT_DIR/config.worktree instead of $GIT_DIR/config (and never in $HOME/.gitconfig or /etc/gitconfig) The reason for this split is, as the name implies, for worktree-specific configuration. When the same repo is attached to more than one worktree, we can't use $GIT_DIR/config to store worktree specific stuff because it's shared across worktrees. With this, both git-new-workdir and nd/multiple-work-trees will work. git-new-workdir creates a symlink to $GIT_DIR/config in the new worktree, but not $GIT_DIR/config.worktree. That file will be created new when worktree.* are written in the new worktree. `git checkout --to` can remap the path $GIT_DIR/config.worktree to make it worktree-specific. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- Documentation/config.txt | 4 +++- builtin/config.c | 8 ++++++++ config.c | 38 ++++++++++++++++++++++++++++++++++++++ t/t1300-repo-config.sh | 31 +++++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 04e2a71..26e4e07 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2,11 +2,13 @@ CONFIGURATION FILE ------------------ The Git configuration file contains a number of variables that affect -the Git commands' behavior. The `.git/config` file in each repository +the Git commands' behavior. The `.git/config` and `.git/config.worktree` +files 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. +worktree.* variables can only be contained in `.git/config.worktree` The configuration variables are used by both the Git plumbing and the porcelains. The variables are divided into sections, wherein diff --git a/builtin/config.c b/builtin/config.c index 15a7bea..d9333cd 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -552,6 +552,14 @@ int cmd_config(int argc, const char **argv, const char *prefix) usage_with_options(builtin_config_usage, builtin_config_options); } + /* + * For set operations, --local could be either config or + * config.worktree. Let config.c determine the path based on + * config keys. + */ + if (use_local_config && actions != ACTION_LIST) + given_config_source.file = NULL; + if (actions == ACTION_LIST) { check_argc(argc, 0, 0); if (git_config_with_options(show_all_config, NULL, diff --git a/config.c b/config.c index 752e2e2..3d46192 100644 --- a/config.c +++ b/config.c @@ -12,6 +12,7 @@ #include "quote.h" #include "hashmap.h" #include "string-list.h" +#include "dir.h" struct config_source { struct config_source *prev; @@ -1177,6 +1178,15 @@ int git_config_system(void) return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0); } +static int config_worktree_filter(const char *var, const char *value, void *data) +{ + struct config_include_data *inc = data; + + if (!starts_with(var, "worktree.")) + return error("$GIT_DIR/config.worktree can only contain worktree.*"); + return inc->fn(var, value, inc->data); +} + int git_config_early(config_fn_t fn, void *data, const char *repo_config) { int ret = 0, found = 0; @@ -1202,8 +1212,19 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config) } if (repo_config && !access_or_die(repo_config, R_OK, 0)) { + char *wt_config; ret += git_config_from_file(fn, repo_config, data); found += 1; + wt_config = git_pathdup("config.worktree"); + if (!access_or_die(wt_config, R_OK, 0)) { + struct config_include_data inc = CONFIG_INCLUDE_INIT; + inc.fn = fn; + inc.data = data; + ret += git_config_from_file(config_worktree_filter, + wt_config, &inc); + found += 1; + } + free(wt_config); } switch (git_config_from_parameters(fn, data)) { @@ -1942,6 +1963,23 @@ int git_config_set_multivar_in_file(const char *config_filename, store.multi_replace = multi_replace; + if (starts_with(key, "worktree.")) { + if (!config_filename) + config_filename = filename_buf = git_pathdup("config.worktree"); + /* cheap trick, but should work 90% of time */ + else if (!ends_with(config_filename, ".worktree")) + die("worktree.* can only be stored in %s", + git_path("config.worktree")); + else { + struct strbuf sb = STRBUF_INIT; + strbuf_addstr(&sb, real_path(config_filename)); + if (strcmp_icase(sb.buf, + real_path(git_path("config.worktree")))) + die("worktree.* can only be stored in %s", + git_path("config.worktree")); + strbuf_release(&sb); + } + } if (!config_filename) config_filename = filename_buf = git_pathdup("config"); diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index 938fc8b..58a5009 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -1179,4 +1179,35 @@ test_expect_success POSIXPERM,PERL 'preserves existing permissions' ' "die q(badrename) if ((stat(q(.git/config)))[2] & 07777) != 0600" ' +test_expect_success 'setting worktree.foo goes to config.worktree' ' + test_when_finished "rm .git/config.worktree" && + git config worktree.foo bar && + cat >expect <<\EOF && +[worktree] + foo = bar +EOF + test_cmp expect .git/config.worktree && + test "`git config worktree.foo`" = bar +' + +test_expect_success 'setting --local worktree.foo goes to config.worktree' ' + test_when_finished "rm .git/config.worktree" && + git config --local worktree.fooo barr && + cat >expect <<\EOF && +[worktree] + fooo = barr +EOF + test_cmp expect .git/config.worktree && + test "`git config worktree.fooo`" = barr +' + +test_expect_success 'setting worktree.foo outside config.worktree' ' + test_must_fail git config --global worktree.foo bar 2>err && + grep config.worktree err && + test_must_fail git config --system worktree.foo bar 2>err && + grep config.worktree err && + test_must_fail git config --file foo.worktree worktree.foo bar 2>err && + grep config.worktree err +' + test_done -- 2.3.0.rc1.137.g477eb31 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/3] setup: add worktree.path to shadow core.worktree 2015-02-08 13:16 [PATCH/RFD 0/3] worktree.* config keys and submodule and multiple worktrees Nguyễn Thái Ngọc Duy 2015-02-08 13:16 ` [PATCH 1/3] config.c: new config namespace worktree.* stored in $GIT_DIR/config.worktree Nguyễn Thái Ngọc Duy @ 2015-02-08 13:16 ` Nguyễn Thái Ngọc Duy 2015-02-08 13:16 ` [PATCH 3/3] submodule: use worktree.path instead of core.worktree Nguyễn Thái Ngọc Duy 2015-02-08 17:36 ` [PATCH/RFD 0/3] worktree.* config keys and submodule and multiple worktrees Jens Lehmann 3 siblings, 0 replies; 22+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2015-02-08 13:16 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jens Lehmann, Max Kirillov, Nguyễn Thái Ngọc Duy They have the same purpose. But they are located in different places: core.worktree in $GIT_DIR/config while worktree.path in $GIT_DIR/config.worktree Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- Documentation/config.txt | 3 +++ setup.c | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 26e4e07..b717881 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -410,6 +410,7 @@ false), while all other repositories are assumed to be bare (bare = true). core.worktree:: +worktree.path:: Set the path to the root of the working tree. This can be overridden by the GIT_WORK_TREE environment variable and the '--work-tree' command-line option. @@ -430,6 +431,8 @@ still use "/different/path" as the root of the work tree and can cause confusion unless you know what you are doing (e.g. you are creating a read-only snapshot of the same index to a location different from the repository's usual working tree). ++ +worktree.path takes precedence over core.worktree. core.logAllRefUpdates:: Enable the reflog. Updates to a ref <ref> is logged to the file diff --git a/setup.c b/setup.c index 979b13f..bc27f8b 100644 --- a/setup.c +++ b/setup.c @@ -4,6 +4,7 @@ static int inside_git_dir = -1; static int inside_work_tree = -1; +static int worktree_path_set; /* * The input parameter must contain an absolute path, and it must already be @@ -807,12 +808,16 @@ int check_repository_format_version(const char *var, const char *value, void *cb is_bare_repository_cfg = git_config_bool(var, value); if (is_bare_repository_cfg == 1) inside_work_tree = -1; - } else if (strcmp(var, "core.worktree") == 0) { + } else if (strcmp(var, "worktree.path") == 0 || + (strcmp(var, "core.worktree") == 0 && + !worktree_path_set)) { if (!value) return config_error_nonbool(var); free(git_work_tree_cfg); git_work_tree_cfg = xstrdup(value); inside_work_tree = -1; + if (!strcmp(var, "worktree.path")) + worktree_path_set = 1; } return 0; } -- 2.3.0.rc1.137.g477eb31 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/3] submodule: use worktree.path instead of core.worktree 2015-02-08 13:16 [PATCH/RFD 0/3] worktree.* config keys and submodule and multiple worktrees Nguyễn Thái Ngọc Duy 2015-02-08 13:16 ` [PATCH 1/3] config.c: new config namespace worktree.* stored in $GIT_DIR/config.worktree Nguyễn Thái Ngọc Duy 2015-02-08 13:16 ` [PATCH 2/3] setup: add worktree.path to shadow core.worktree Nguyễn Thái Ngọc Duy @ 2015-02-08 13:16 ` Nguyễn Thái Ngọc Duy 2015-02-08 17:36 ` [PATCH/RFD 0/3] worktree.* config keys and submodule and multiple worktrees Jens Lehmann 3 siblings, 0 replies; 22+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2015-02-08 13:16 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jens Lehmann, Max Kirillov, Nguyễn Thái Ngọc Duy This opens a door of using submodule with multiple worktrees Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- git-submodule.sh | 2 +- submodule.c | 6 +++--- t/lib-submodule-update.sh | 8 ++++---- t/t7400-submodule-basic.sh | 4 ++-- t/t7409-submodule-detached-worktree.sh | 6 +++--- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 9245abf..6e9e1d1 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -309,7 +309,7 @@ module_clone() printf '%s\n' "gitdir: $rel/$a" >"$sm_path/.git" rel=$(printf '%s\n' "$a" | sed -e 's|[^/][^/]*|..|g') - (clear_local_git_env; cd "$sm_path" && GIT_WORK_TREE=. git config core.worktree "$rel/$b") + (clear_local_git_env; cd "$sm_path" && GIT_WORK_TREE=. git config worktree.path "$rel/$b") } isnumber() diff --git a/submodule.c b/submodule.c index d37d400..f886fa6 100644 --- a/submodule.c +++ b/submodule.c @@ -1115,11 +1115,11 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir) /* Update core.worktree setting */ strbuf_reset(&file_name); - strbuf_addf(&file_name, "%s/config", git_dir); - if (git_config_set_in_file(file_name.buf, "core.worktree", + strbuf_addf(&file_name, "%s/config.worktree", git_dir); + if (git_config_set_in_file(file_name.buf, "worktree.path", relative_path(real_work_tree, git_dir, &rel_path))) - die(_("Could not set core.worktree in %s"), + die(_("Could not set worktree.path in %s"), file_name.buf); strbuf_release(&file_name); diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh index 79cdd34..ce140cf 100755 --- a/t/lib-submodule-update.sh +++ b/t/lib-submodule-update.sh @@ -105,7 +105,7 @@ replace_gitfile_with_git_dir () { git_dir="$(git rev-parse --git-dir)" && rm -f .git && cp -R "$git_dir" .git && - GIT_WORK_TREE=. git config --unset core.worktree + GIT_WORK_TREE=. git config --unset worktree.path ) } @@ -120,16 +120,16 @@ test_git_directory_is_unchanged () { ( cd ".git/modules/$1" && # does core.worktree point at the right place? - test "$(git config core.worktree)" = "../../../$1" && + test "$(git config worktree.path)" = "../../../$1" && # remove it temporarily before comparing, as # "$1/.git/config" lacks it... - git config --unset core.worktree + git config --unset worktree.path ) && diff -r ".git/modules/$1" "$1/.git" && ( # ... and then restore. cd ".git/modules/$1" && - git config core.worktree "../../../$1" + git config worktree.path "../../../$1" ) } diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index 7c88245..def28e6 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -93,7 +93,7 @@ test_expect_success 'submodule add' ' test_cmp expect submod/.git && ( cd submod && - git config core.worktree >actual && + git config worktree.path >actual && echo "../../../submod" >expect && test_cmp expect actual && rm -f actual expect @@ -945,7 +945,7 @@ test_expect_success 'submodule deinit fails when submodule has a .git directory cd init && rm .git && cp -R ../.git/modules/example .git && - GIT_WORK_TREE=. git config --unset core.worktree + GIT_WORK_TREE=. git config --unset worktree.path ) && test_must_fail git submodule deinit init && test_must_fail git submodule deinit -f init && diff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh index c207171..ba50d77 100755 --- a/t/t7409-submodule-detached-worktree.sh +++ b/t/t7409-submodule-detached-worktree.sh @@ -55,7 +55,7 @@ test_expect_success 'submodule on detached working tree' ' ) ' -test_expect_success 'submodule on detached working pointed by core.worktree' ' +test_expect_success 'submodule on detached working pointed by worktree.path' ' mkdir home3 && ( cd home3 && @@ -63,7 +63,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' ' export GIT_DIR && git clone --bare ../remote "$GIT_DIR" && git config core.bare false && - git config core.worktree .. && + git config worktree.path .. && git checkout master && git submodule add ../bundle1 .vim/bundle/dupe && test_commit "dupe" && @@ -74,7 +74,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' ' GIT_DIR="$(pwd)/.dotfiles" && export GIT_DIR && git config core.bare false && - git config core.worktree .. && + git config worktree.path .. && git pull && git submodule update --init && test -f .vim/bundle/dupe/shoot.t -- 2.3.0.rc1.137.g477eb31 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH/RFD 0/3] worktree.* config keys and submodule and multiple worktrees 2015-02-08 13:16 [PATCH/RFD 0/3] worktree.* config keys and submodule and multiple worktrees Nguyễn Thái Ngọc Duy ` (2 preceding siblings ...) 2015-02-08 13:16 ` [PATCH 3/3] submodule: use worktree.path instead of core.worktree Nguyễn Thái Ngọc Duy @ 2015-02-08 17:36 ` Jens Lehmann 2015-02-08 17:41 ` Jens Lehmann ` (2 more replies) 3 siblings, 3 replies; 22+ messages in thread From: Jens Lehmann @ 2015-02-08 17:36 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy, git; +Cc: Junio C Hamano, Max Kirillov Am 08.02.2015 um 05:16 schrieb Nguyễn Thái Ngọc Duy: > 1/3 is a more complete version of a patch I posted earlier [1]. It > makes sure that worktree.* config keys are stored in a different place > than $GIT_DIR/config. This allows us to work around the core.worktree > issue in multiple worktree setting. > > I think 1/3 and 2/3 are fine. 3/3 is probably not. It's more of a > proof of concept. The tests pass, but there's no migration path for > existing submodules. Submodules modified by new Git will confuse old > Git because the old ones do not understand worktree.path (the > replacement for core.worktree) Yeah, breaking every submodule cloned with v1.7.9 or newer is not the way to go. You'd still have to support the old configuration name for a very long time. I wonder if it's worth all the hassle to invent new names. Wouldn't it be much better to just keep a list of per-worktree configuration value names and use that inside the config code to decide where to find them for multiple work trees. That would also work easily for stuff like EOL-config and would push the complexity in the config machinery and not onto the user. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH/RFD 0/3] worktree.* config keys and submodule and multiple worktrees 2015-02-08 17:36 ` [PATCH/RFD 0/3] worktree.* config keys and submodule and multiple worktrees Jens Lehmann @ 2015-02-08 17:41 ` Jens Lehmann 2015-02-09 9:35 ` Duy Nguyen 2015-03-18 21:33 ` per-repository and per-worktree config variables Max Kirillov 2 siblings, 0 replies; 22+ messages in thread From: Jens Lehmann @ 2015-02-08 17:41 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy, git; +Cc: Junio C Hamano, Max Kirillov Am 08.02.2015 um 09:36 schrieb Jens Lehmann: > Am 08.02.2015 um 05:16 schrieb Nguyễn Thái Ngọc Duy: >> 1/3 is a more complete version of a patch I posted earlier [1]. It >> makes sure that worktree.* config keys are stored in a different place >> than $GIT_DIR/config. This allows us to work around the core.worktree >> issue in multiple worktree setting. >> >> I think 1/3 and 2/3 are fine. 3/3 is probably not. It's more of a >> proof of concept. The tests pass, but there's no migration path for >> existing submodules. Submodules modified by new Git will confuse old >> Git because the old ones do not understand worktree.path (the >> replacement for core.worktree) > > Yeah, breaking every submodule cloned with v1.7.9 or newer is not That's v1.7.8 of course ... > the way to go. You'd still have to support the old configuration > name for a very long time. > > I wonder if it's worth all the hassle to invent new names. Wouldn't > it be much better to just keep a list of per-worktree configuration > value names and use that inside the config code to decide where to > find them for multiple work trees. That would also work easily for > stuff like EOL-config and would push the complexity in the config > machinery and not onto the user. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH/RFD 0/3] worktree.* config keys and submodule and multiple worktrees 2015-02-08 17:36 ` [PATCH/RFD 0/3] worktree.* config keys and submodule and multiple worktrees Jens Lehmann 2015-02-08 17:41 ` Jens Lehmann @ 2015-02-09 9:35 ` Duy Nguyen 2015-03-18 21:33 ` per-repository and per-worktree config variables Max Kirillov 2 siblings, 0 replies; 22+ messages in thread From: Duy Nguyen @ 2015-02-09 9:35 UTC (permalink / raw) To: Jens Lehmann; +Cc: Git Mailing List, Junio C Hamano, Max Kirillov On Mon, Feb 9, 2015 at 12:36 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote: > I wonder if it's worth all the hassle to invent new names. Wouldn't > it be much better to just keep a list of per-worktree configuration > value names and use that inside the config code to decide where to > find them for multiple work trees. That would also work easily for > stuff like EOL-config and would push the complexity in the config > machinery and not onto the user. It's certainly possible to relocate core.worktree to outside $GIT_DIR/config. But I don't think it helps. If anything it'll make it harder to distinguish the old setup and the new one. I think we need a clear signal that will make old git barf on new setup, to be safe. Maybe stepping core.repositoryformatversion, or breaking .git file format when we switch to the new setup (with $GIT_COMMON_DIR). Or.. perhaps we could use the old setup for "primary" worktree and the new one for secondary worktrees of the same submodule. In these secondary submodules, $GIT_COMMON_DIR is enough to make old git reject them. And we could just reuse core.worktree, if we can make core.worktree in $GIT_DIR/config.worktree shadow one in $GIT_DIR/config.. Need to study git-submodule.sh some more.. -- Duy ^ permalink raw reply [flat|nested] 22+ messages in thread
* per-repository and per-worktree config variables 2015-02-08 17:36 ` [PATCH/RFD 0/3] worktree.* config keys and submodule and multiple worktrees Jens Lehmann 2015-02-08 17:41 ` Jens Lehmann 2015-02-09 9:35 ` Duy Nguyen @ 2015-03-18 21:33 ` Max Kirillov 2015-03-24 13:48 ` Duy Nguyen 2015-03-25 21:33 ` per-repository and per-worktree config variables Jens Lehmann 2 siblings, 2 replies; 22+ messages in thread From: Max Kirillov @ 2015-03-18 21:33 UTC (permalink / raw) To: Jens Lehmann, Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano On Sun, Feb 08, 2015 at 09:36:43AM -0800, Jens Lehmann wrote: > I wonder if it's worth all the hassle to invent new names. Wouldn't > it be much better to just keep a list of per-worktree configuration > value names and use that inside the config code to decide where to > find them for multiple work trees. That would also work easily for > stuff like EOL-config and would push the complexity in the config > machinery and not onto the user. I actually thought about the same, and now tend to think that most of config variables make sense to be per-worktree in some cases. Only few variable must always be per repository. I tried to summarize the variables which now (in current pu) should be common, also listed all the rest so somebody could scan through the list and spot anything I could miss. PS: some variables are with older case, they were mostly collected before I got aware of their rename. -- Max Strictly common only ~~~~~~~~~~~~~~~~~~~~ Distributiveness ^^^^^^^^^^^^^^^^ A repository should behave same way regartdless of which worktree is being accessed. core.gitProxy:: core.askpass:: credential.helper:: credential.useHttpPath:: credential.username:: credential.<url>.*:: fetch.fsckObjects:: fetch.prune:: gitcvs.*:: gitweb.*:: gui.pruneduringfetch:: http.*:: push.followTags:: receive.*:: remote.<name>.*:: remotes.<group>:: transfer.fsckObjects:: transfer.hiderefs:: uploadarchive.allowUnreachable:: uploadpack.*:: url.<base>.insteadOf:: url.<base>.pushInsteadOf:: imap.*:: Repository storage options ^^^^^^^^^^^^^^^^^^^^^^^^^^ There should be same options of purging, compression etc. core.compression:: core.loosecompression:: core.packedGitWindowSize:: core.packedGitLimit:: core.deltaBaseCacheLimit:: core.bigFileThreshold:: core.fsyncobjectfiles:: core.createObject:: core.logAllRefUpdates:: core.protectHFS:: core.protectNTFS:: fetch.unpackLimit:: fsck.skipList:: fsck.error:: fsck.warn:: fsck.ignore:: gc.* pack.* repack.* Branch tracking settings ^^^^^^^^^^^^^^^^^^^^^^^^ Since branch are shared, they should be tracked same way regardless of where are they updated. branch.<name>.remote:: branch.<name>.pushremote:: branch.<name>.merge:: branch.<name>.mergeoptions:: branch.<name>.rebase:: Notes handling ^^^^^^^^^^^^^^ I don't know much about notes, and not sure about these ones, but notes looks like a part of common history, and the options affect all of them. notes.rewrite.<command>:: notes.rewriteMode:: notes.rewriteRef:: Misc ^^^^ core.preferSymlinkRefs:: As description says, useful mostly for older scripts, which definitely not going to understand the new format. Probably incompatible with multiple worktrees at all. core.repositoryFormatVersion:: Looks like this is not used. init.templatedir:: This makes sense only as a global option. Prefer common ~~~~~~~~~~~~~ Technically can be used per-worktree, but does not seem to have much sense. Many of them are are actualy expected to be global. advice.*:: core.ignorecase:: core.precomposeunicode:: core.trustctime:: core.checkstat:: core.quotepath:: core.symlinks:: core.ignoreStat:: core.warnAmbiguousRefs:: core.attributesfile:: This should be common if it names tracked file. core.editor:: core.commentchar:: sequence.editor:: core.pager:: core.preloadindex:: alias.*:: browser.<tool>.path:: color.branch:: color.diff:: color.diff.<slot>:: color.decorate.<slot>:: color.grep:: color.grep.<slot>:: color.interactive:: color.interactive.<slot>:: color.pager:: color.showbranch:: color.status:: color.status.<slot>:: color.ui:: color.branch.<slot>:: column.*:: commit.cleanup:: commit.gpgsign:: commit.status:: commit.template:: difftool.*:: fetch.recurseSubmodules:: Since submodules can differ, this also can. gui.commitmsgwidth:: gui.diffcontext:: gui.encoding:: While in theory possible to use per-workree value, probably this is intended to correspond to the actual data in repository. Not sure though. gui.trustmtime:: gui.spellingdictionary:: help.* i18n.commitEncoding:: i18n.logOutputEncoding:: index.version:: instaweb.*:: mailmap.*:: man.*:: mergetool.*:: pager.*:: sendemail.*:: tar.umask:: user.email:: user.name:: user.signingkey:: web.browser:: mailinfo.scissors:: Ok to be per-worktree ~~~~~~~~~~~~~~~~~~~~~ core.fileMode:: core.eol:: core.safecrlf:: core.autocrlf:: core.bare:: core.worktree:: core.excludesfile:: core.whitespace:: core.notesRef:: core.sparseCheckout:: core.abbrev:: add.ignore-errors:: add.ignoreErrors:: am.keepcr:: apply.ignorewhitespace:: apply.whitespace:: branch.autosetupmerge:: branch.autosetuprebase:: clean.requireForce:: format.*:: filter.*:: For some worktrees it may be desired to have them unsmudged. grep.*:: gpg.program:: gui.displayuntracked:: gui.matchtrackingbranch:: gui.newbranchtemplate:: gui.fastcopyblame:: gui.copyblamethreshold:: gui.blamehistoryctx:: guitool.* interactive.singlekey:: log.*:: notes.displayRef:: pretty.<name>:: pull.ff:: pull.rebase:: pull.octopus:: pull.twohead:: push.default:: rebase.stat:: rebase.autosquash:: rebase.autostash:: remote.pushdefault:: rerere.*:: showbranch.*:: status.*:: submodule.*:: tag.sort:: diff.*:: not sure about "diff.<driver>" commands, but I could imagine cases when it is desirable merge.*:: not sure about "merge.<driver>" commands, but I could imagine cases when it is desirable versionsort.prereleaseSuffix:: ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: per-repository and per-worktree config variables 2015-03-18 21:33 ` per-repository and per-worktree config variables Max Kirillov @ 2015-03-24 13:48 ` Duy Nguyen 2015-03-26 12:04 ` [PATCH v2] config.c: split some variables to $GIT_DIR/config.worktree Nguyễn Thái Ngọc Duy 2015-03-25 21:33 ` per-repository and per-worktree config variables Jens Lehmann 1 sibling, 1 reply; 22+ messages in thread From: Duy Nguyen @ 2015-03-24 13:48 UTC (permalink / raw) To: Max Kirillov; +Cc: Jens Lehmann, Git Mailing List, Junio C Hamano On Thu, Mar 19, 2015 at 4:33 AM, Max Kirillov <max@max630.net> wrote: > On Sun, Feb 08, 2015 at 09:36:43AM -0800, Jens Lehmann wrote: >> I wonder if it's worth all the hassle to invent new names. Wouldn't >> it be much better to just keep a list of per-worktree configuration >> value names and use that inside the config code to decide where to >> find them for multiple work trees. That would also work easily for >> stuff like EOL-config and would push the complexity in the config >> machinery and not onto the user. > > I actually thought about the same, and now tend to think > that most of config variables make sense to be per-worktree > in some cases. Only few variable must always be per > repository. I tried to summarize the variables which now > (in current pu) should be common, also listed all the rest > so somebody could scan through the list and spot anything I > could miss. Thanks for compiling the list. At this point I think it may not be sensible to hard code some config vars (e.g. core.worktree) to be local or shared. So I'm thinking (out loud) that we may have a file $GIT_DIR/worktrees/<id>/local-config-patterns (or some other name) that would define what vars are local. gitignore syntax will be reused for this. The file would provide more flexibility.. -- Duy ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v2] config.c: split some variables to $GIT_DIR/config.worktree 2015-03-24 13:48 ` Duy Nguyen @ 2015-03-26 12:04 ` Nguyễn Thái Ngọc Duy 2015-03-26 22:19 ` Max Kirillov 2015-03-31 12:14 ` [PATCH v3] " Nguyễn Thái Ngọc Duy 0 siblings, 2 replies; 22+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2015-03-26 12:04 UTC (permalink / raw) To: git Cc: Junio C Hamano, Max Kirillov, Jens.Lehmann, Nguyễn Thái Ngọc Duy When you define $GIT_DIR/info/config.worktree, which contains of gitignore-style patterns (*), config variables that match these patterns will be saved in $GIT_DIR/config.worktree instead of $GIT_DIR/config. On the surface, they are just like any other variables. You can read or modify them with `git config` command, or config_* API. Underneath, they are always stored in $GIT_DIR/config.worktree instead of $GIT_DIR/config (and never in $HOME/.gitconfig or /etc/gitconfig) The reason for this split is, as the name implies, for worktree-specific configuration. When the same repo is attached to more than one worktree, we can't use $GIT_DIR/config to store worktree specific stuff because it's shared across worktrees. (*) Config variable names are separated by dots. However as this is a quick and dirty patch to experiment with submodules and multiple worktrees, you must substitute dots with slashes when writing these patterns, for now. E.g. write "remote/*/foo" instead of "remote.*.foo" Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- Something for Max to play with if he's still experimenting with multiple worktree support for submodules :-) It's far from perfect, but good enough for this purpose. Documentation/config.txt | 6 ++++- builtin/config.c | 8 ++++++ config.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++ t/t1300-repo-config.sh | 34 ++++++++++++++++++++++++ 4 files changed, 114 insertions(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 2700a1b..6d00f49 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2,11 +2,15 @@ CONFIGURATION FILE ------------------ The Git configuration file contains a number of variables that affect -the Git commands' behavior. The `.git/config` file in each repository +the Git commands' behavior. The `.git/config` and `.git/config.worktree` +files 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. +If `.git/info/core.worktree` exists, it contains gitignore-style +patterns. Variables that match these patterns can only be contained in +`.git/config.worktree`. The configuration variables are used by both the Git plumbing and the porcelains. The variables are divided into sections, wherein diff --git a/builtin/config.c b/builtin/config.c index 8cc2604..4ca8fc2 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -555,6 +555,14 @@ int cmd_config(int argc, const char **argv, const char *prefix) usage_with_options(builtin_config_usage, builtin_config_options); } + /* + * For set operations, --local could be either config or + * config.worktree. Let config.c determine the path based on + * config keys. + */ + if (use_local_config && actions != ACTION_LIST) + given_config_source.file = NULL; + if (actions == ACTION_LIST) { check_argc(argc, 0, 0); if (git_config_with_options(show_all_config, NULL, diff --git a/config.c b/config.c index 15a2983..f68eb6a 100644 --- a/config.c +++ b/config.c @@ -12,6 +12,7 @@ #include "quote.h" #include "hashmap.h" #include "string-list.h" +#include "dir.h" struct config_source { struct config_source *prev; @@ -37,6 +38,7 @@ struct config_source { }; static struct config_source *cf; +static struct exclude_list config_local; static int zlib_compression_seen; @@ -84,6 +86,34 @@ static long config_buf_ftell(struct config_source *conf) return conf->u.buf.pos; } +static int is_config_local(const char *key_) +{ + static struct strbuf key = STRBUF_INIT; + static int loaded; + int i, dtype; + + if (!loaded) { + add_excludes_from_file_to_list(git_path("info/config.worktree"), + "", 0, &config_local, 0); + loaded = 1; + } + + if (!config_local.nr) + return 0; + + strbuf_reset(&key); + strbuf_addstr(&key, key_); + for (i = 0; i < key.len; i++) { + if (key.buf[i] == '.') + key.buf[i] = '/'; + else + key.buf[i] = tolower(key.buf[i]); + } + dtype = DT_REG; + return is_excluded_from_list(key.buf, key.len, "", &dtype, + &config_local) > 0; +} + #define MAX_INCLUDE_DEPTH 10 static const char include_depth_advice[] = "exceeded maximum include depth (%d) while including\n" @@ -1167,6 +1197,15 @@ int git_config_system(void) return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0); } +static int config_local_filter(const char *var, const char *value, void *data) +{ + struct config_include_data *inc = data; + + if (!is_config_local(var)) + return error("%s does not belong to config.worktree", var); + return inc->fn(var, value, inc->data); +} + int git_config_early(config_fn_t fn, void *data, const char *repo_config) { int ret = 0, found = 0; @@ -1192,8 +1231,19 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config) } if (repo_config && !access_or_die(repo_config, R_OK, 0)) { + char *wt_config; ret += git_config_from_file(fn, repo_config, data); found += 1; + wt_config = git_pathdup("config.worktree"); + if (!access_or_die(wt_config, R_OK, 0)) { + struct config_include_data inc = CONFIG_INCLUDE_INIT; + inc.fn = fn; + inc.data = data; + ret += git_config_from_file(config_local_filter, + wt_config, &inc); + found += 1; + } + free(wt_config); } switch (git_config_from_parameters(fn, data)) { @@ -1932,6 +1982,23 @@ int git_config_set_multivar_in_file(const char *config_filename, store.multi_replace = multi_replace; + if (is_config_local(key)) { + if (!config_filename) + config_filename = filename_buf = git_pathdup("config.worktree"); + /* cheap trick, but should work 90% of time */ + else if (!ends_with(config_filename, ".worktree")) + die("%s can only be stored in %s", + key, git_path("config.worktree")); + else { + struct strbuf sb = STRBUF_INIT; + strbuf_addstr(&sb, real_path(config_filename)); + if (strcmp_icase(sb.buf, + real_path(git_path("config.worktree")))) + die("%s can only be stored in %s", + key, git_path("config.worktree")); + strbuf_release(&sb); + } + } if (!config_filename) config_filename = filename_buf = git_pathdup("config"); diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index 938fc8b..c01effb 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -1179,4 +1179,38 @@ test_expect_success POSIXPERM,PERL 'preserves existing permissions' ' "die q(badrename) if ((stat(q(.git/config)))[2] & 07777) != 0600" ' +test_expect_success 'setting worktree.foo goes to config.worktree' ' + test_when_finished "rm .git/config.worktree" && + echo "worktree/*" >.git/info/config.worktree && + git config wOrktree.foo bar && + cat >expect <<\EOF && +[wOrktree] + foo = bar +EOF + test_cmp expect .git/config.worktree && + test "`git config woRktree.foo`" = bar +' + +test_expect_success 'setting --local worktree.foo goes to config.worktree' ' + test_when_finished "rm .git/config.worktree" && + echo "worktree/*" >.git/info/config.worktree && + git config --local worKtree.fooo barr && + cat >expect <<\EOF && +[worKtree] + fooo = barr +EOF + test_cmp expect .git/config.worktree && + test "`git config worktreE.fooo`" = barr +' + +test_expect_success 'setting worktree.foo outside config.worktree' ' + echo "worktree/*" >.git/info/config.worktree && + test_must_fail git config --global Worktree.foo bar 2>err && + grep config.worktree err && + test_must_fail git config --system workTree.foo bar 2>err && + grep config.worktree err && + test_must_fail git config --file foo.worktree worktree.foo bar 2>err && + grep config.worktree err +' + test_done -- 2.3.0.rc1.137.g477eb31 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2] config.c: split some variables to $GIT_DIR/config.worktree 2015-03-26 12:04 ` [PATCH v2] config.c: split some variables to $GIT_DIR/config.worktree Nguyễn Thái Ngọc Duy @ 2015-03-26 22:19 ` Max Kirillov 2015-03-29 1:25 ` Duy Nguyen 2015-03-31 12:14 ` [PATCH v3] " Nguyễn Thái Ngọc Duy 1 sibling, 1 reply; 22+ messages in thread From: Max Kirillov @ 2015-03-26 22:19 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, Jens.Lehmann On Thu, Mar 26, 2015 at 07:04:24PM +0700, Nguyễn Thái Ngọc Duy wrote: > When you define $GIT_DIR/info/config.worktree, which contains of > gitignore-style patterns (*), config variables that match these > patterns will be saved in $GIT_DIR/config.worktree instead of > $GIT_DIR/config. Should it rather be in GIT_COMMON_DIR? As far as I understand, its meaning is "variables which we allow to use per-worktree because we intend to have them different in different worktrees, and sure no bad issues can happen because this. It is not hardcored in git because the list is going to extend, and we'd like to allow older versions of git (and other git implementations) to be still able to understand newer repositories". So there should be no sense to make the list worktree-specific. Also, probably the per-worktree variables should be searched for in both common config and per-worktree config, and the main repository should not have config.worktree, to be able to work with implementations which are not aware of the whole multiple worktrees feature. And in worktrees, if the variable is not defined in config.wortree, the default vaalue should come from common config. This though has downside that worktree cannot use the more global vlue for variable implicitly. -- Max > On the surface, they are just like any other variables. You can read > or modify them with `git config` command, or config_* API. Underneath, > they are always stored in $GIT_DIR/config.worktree instead of > $GIT_DIR/config (and never in $HOME/.gitconfig or /etc/gitconfig) > > The reason for this split is, as the name implies, for > worktree-specific configuration. When the same repo is attached to > more than one worktree, we can't use $GIT_DIR/config to store worktree > specific stuff because it's shared across worktrees. > > (*) Config variable names are separated by dots. However as this is a > quick and dirty patch to experiment with submodules and multiple > worktrees, you must substitute dots with slashes when writing these > patterns, for now. E.g. write "remote/*/foo" instead of "remote.*.foo" > > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> > --- > Something for Max to play with if he's still experimenting with > multiple worktree support for submodules :-) It's far from perfect, > but good enough for this purpose. > > Documentation/config.txt | 6 ++++- > builtin/config.c | 8 ++++++ > config.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++ > t/t1300-repo-config.sh | 34 ++++++++++++++++++++++++ > 4 files changed, 114 insertions(+), 1 deletion(-) > > diff --git a/Documentation/config.txt b/Documentation/config.txt > index 2700a1b..6d00f49 100644 > --- a/Documentation/config.txt > +++ b/Documentation/config.txt > @@ -2,11 +2,15 @@ CONFIGURATION FILE > ------------------ > > The Git configuration file contains a number of variables that affect > -the Git commands' behavior. The `.git/config` file in each repository > +the Git commands' behavior. The `.git/config` and `.git/config.worktree` > +files 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. > +If `.git/info/core.worktree` exists, it contains gitignore-style > +patterns. Variables that match these patterns can only be contained in > +`.git/config.worktree`. > > The configuration variables are used by both the Git plumbing > and the porcelains. The variables are divided into sections, wherein > diff --git a/builtin/config.c b/builtin/config.c > index 8cc2604..4ca8fc2 100644 > --- a/builtin/config.c > +++ b/builtin/config.c > @@ -555,6 +555,14 @@ int cmd_config(int argc, const char **argv, const char *prefix) > usage_with_options(builtin_config_usage, builtin_config_options); > } > > + /* > + * For set operations, --local could be either config or > + * config.worktree. Let config.c determine the path based on > + * config keys. > + */ > + if (use_local_config && actions != ACTION_LIST) > + given_config_source.file = NULL; > + > if (actions == ACTION_LIST) { > check_argc(argc, 0, 0); > if (git_config_with_options(show_all_config, NULL, > diff --git a/config.c b/config.c > index 15a2983..f68eb6a 100644 > --- a/config.c > +++ b/config.c > @@ -12,6 +12,7 @@ > #include "quote.h" > #include "hashmap.h" > #include "string-list.h" > +#include "dir.h" > > struct config_source { > struct config_source *prev; > @@ -37,6 +38,7 @@ struct config_source { > }; > > static struct config_source *cf; > +static struct exclude_list config_local; > > static int zlib_compression_seen; > > @@ -84,6 +86,34 @@ static long config_buf_ftell(struct config_source *conf) > return conf->u.buf.pos; > } > > +static int is_config_local(const char *key_) > +{ > + static struct strbuf key = STRBUF_INIT; > + static int loaded; > + int i, dtype; > + > + if (!loaded) { > + add_excludes_from_file_to_list(git_path("info/config.worktree"), > + "", 0, &config_local, 0); > + loaded = 1; > + } > + > + if (!config_local.nr) > + return 0; > + > + strbuf_reset(&key); > + strbuf_addstr(&key, key_); > + for (i = 0; i < key.len; i++) { > + if (key.buf[i] == '.') > + key.buf[i] = '/'; > + else > + key.buf[i] = tolower(key.buf[i]); > + } > + dtype = DT_REG; > + return is_excluded_from_list(key.buf, key.len, "", &dtype, > + &config_local) > 0; > +} > + > #define MAX_INCLUDE_DEPTH 10 > static const char include_depth_advice[] = > "exceeded maximum include depth (%d) while including\n" > @@ -1167,6 +1197,15 @@ int git_config_system(void) > return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0); > } > > +static int config_local_filter(const char *var, const char *value, void *data) > +{ > + struct config_include_data *inc = data; > + > + if (!is_config_local(var)) > + return error("%s does not belong to config.worktree", var); > + return inc->fn(var, value, inc->data); > +} > + > int git_config_early(config_fn_t fn, void *data, const char *repo_config) > { > int ret = 0, found = 0; > @@ -1192,8 +1231,19 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config) > } > > if (repo_config && !access_or_die(repo_config, R_OK, 0)) { > + char *wt_config; > ret += git_config_from_file(fn, repo_config, data); > found += 1; > + wt_config = git_pathdup("config.worktree"); > + if (!access_or_die(wt_config, R_OK, 0)) { > + struct config_include_data inc = CONFIG_INCLUDE_INIT; > + inc.fn = fn; > + inc.data = data; > + ret += git_config_from_file(config_local_filter, > + wt_config, &inc); > + found += 1; > + } > + free(wt_config); > } > > switch (git_config_from_parameters(fn, data)) { > @@ -1932,6 +1982,23 @@ int git_config_set_multivar_in_file(const char *config_filename, > > store.multi_replace = multi_replace; > > + if (is_config_local(key)) { > + if (!config_filename) > + config_filename = filename_buf = git_pathdup("config.worktree"); > + /* cheap trick, but should work 90% of time */ > + else if (!ends_with(config_filename, ".worktree")) > + die("%s can only be stored in %s", > + key, git_path("config.worktree")); > + else { > + struct strbuf sb = STRBUF_INIT; > + strbuf_addstr(&sb, real_path(config_filename)); > + if (strcmp_icase(sb.buf, > + real_path(git_path("config.worktree")))) > + die("%s can only be stored in %s", > + key, git_path("config.worktree")); > + strbuf_release(&sb); > + } > + } > if (!config_filename) > config_filename = filename_buf = git_pathdup("config"); > > diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh > index 938fc8b..c01effb 100755 > --- a/t/t1300-repo-config.sh > +++ b/t/t1300-repo-config.sh > @@ -1179,4 +1179,38 @@ test_expect_success POSIXPERM,PERL 'preserves existing permissions' ' > "die q(badrename) if ((stat(q(.git/config)))[2] & 07777) != 0600" > ' > > +test_expect_success 'setting worktree.foo goes to config.worktree' ' > + test_when_finished "rm .git/config.worktree" && > + echo "worktree/*" >.git/info/config.worktree && > + git config wOrktree.foo bar && > + cat >expect <<\EOF && > +[wOrktree] > + foo = bar > +EOF > + test_cmp expect .git/config.worktree && > + test "`git config woRktree.foo`" = bar > +' > + > +test_expect_success 'setting --local worktree.foo goes to config.worktree' ' > + test_when_finished "rm .git/config.worktree" && > + echo "worktree/*" >.git/info/config.worktree && > + git config --local worKtree.fooo barr && > + cat >expect <<\EOF && > +[worKtree] > + fooo = barr > +EOF > + test_cmp expect .git/config.worktree && > + test "`git config worktreE.fooo`" = barr > +' > + > +test_expect_success 'setting worktree.foo outside config.worktree' ' > + echo "worktree/*" >.git/info/config.worktree && > + test_must_fail git config --global Worktree.foo bar 2>err && > + grep config.worktree err && > + test_must_fail git config --system workTree.foo bar 2>err && > + grep config.worktree err && > + test_must_fail git config --file foo.worktree worktree.foo bar 2>err && > + grep config.worktree err > +' > + > test_done > -- > 2.3.0.rc1.137.g477eb31 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] config.c: split some variables to $GIT_DIR/config.worktree 2015-03-26 22:19 ` Max Kirillov @ 2015-03-29 1:25 ` Duy Nguyen 2015-03-30 21:26 ` Max Kirillov 0 siblings, 1 reply; 22+ messages in thread From: Duy Nguyen @ 2015-03-29 1:25 UTC (permalink / raw) To: Max Kirillov; +Cc: Git Mailing List, Junio C Hamano, Jens Lehmann On Fri, Mar 27, 2015 at 5:19 AM, Max Kirillov <max@max630.net> wrote: > On Thu, Mar 26, 2015 at 07:04:24PM +0700, Nguyễn Thái Ngọc Duy wrote: >> When you define $GIT_DIR/info/config.worktree, which contains of >> gitignore-style patterns (*), config variables that match these >> patterns will be saved in $GIT_DIR/config.worktree instead of >> $GIT_DIR/config. > > Should it rather be in GIT_COMMON_DIR? As far as I > understand, its meaning is "variables which we allow to use > per-worktree because we intend to have them different in > different worktrees, and sure no bad issues > can happen because this. It is not hardcored in git because > the list is going to extend, and we'd like to allow older > versions of git (and other git implementations) to be still > able to understand newer repositories". So there should be > no sense to make the list worktree-specific. I'm not sure if "it" means $GIT_DIR/config.worktree or $GIT_DIR/info/config.worktree. At this point $GIT_COMMON_DIR is not involved (i.e. you can still spit config even in a normal repo). .../info/config.worktree may be shared, I guess. The "older versions of git (and other git implementations)" raises an issue with this patch. Older impl just ignore config.worktree. I think I need to bump core.repositoryformatversion up to avoid that. > Also, probably the per-worktree variables should be searched > for in both common config and per-worktree config, and the > main repository should not have config.worktree, to be able > to work with implementations which are not aware of the > whole multiple worktrees feature. And in worktrees, if the > variable is not defined in config.wortree, the default > vaalue should come from common config. This though has > downside that worktree cannot use the more global vlue for > variable implicitly. The main worktree may or may not use per-worktree config (it's technically possible): if we enforce config.worktree on the main worktree, we don't have to worry about the same variable defined in both common and per-worktree config. Enforcing may require more work: imagine the worktree list is updated, some in the common config may become per-worktree and need to be moved to config.worktree.. If we allow per-worktree vars in the common config, other worktrees should ignore them in common config. -- Duy ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2] config.c: split some variables to $GIT_DIR/config.worktree 2015-03-29 1:25 ` Duy Nguyen @ 2015-03-30 21:26 ` Max Kirillov 0 siblings, 0 replies; 22+ messages in thread From: Max Kirillov @ 2015-03-30 21:26 UTC (permalink / raw) To: Duy Nguyen; +Cc: Git Mailing List, Junio C Hamano, Jens Lehmann On Sun, Mar 29, 2015 at 08:25:33AM +0700, Duy Nguyen wrote: > I'm not sure if "it" means $GIT_DIR/config.worktree or > $GIT_DIR/info/config.worktree. At this point $GIT_COMMON_DIR is not > involved (i.e. you can still spit config even in a normal repo). > .../info/config.worktree may be shared, I guess. Yes, I meant info/config.worktree > The "older versions of git (and other git implementations)" raises an > issue with this patch. Older impl just ignore config.worktree. I think > I need to bump core.repositoryformatversion up to avoid that. As a user I would like to still use some older tools, so I cannot say I like it. And, I guess bumping repository verion is something the whole git ecosystem has not experienced yet. So it should be much more work and much more time, I cannot even imagine how much. I would still search for option without bumping version. >> Also, probably the per-worktree variables should be searched >> for in both common config and per-worktree config, and the >> main repository should not have config.worktree, to be able >> to work with implementations which are not aware of the >> whole multiple worktrees feature. And in worktrees, if the >> variable is not defined in config.wortree, the default >> vaalue should come from common config. This though has >> downside that worktree cannot use the more global vlue for >> variable implicitly. > > The main worktree may or may not use per-worktree config (it's > technically possible): if we enforce config.worktree on the main > worktree, we don't have to worry about the same variable defined in > both common and per-worktree config. Enforcing may require more work: > imagine the worktree list is updated, some in the common config may > become per-worktree and need to be moved to config.worktree.. If we > allow per-worktree vars in the common config, other worktrees should > ignore them in common config. Yes, probably always ignoring is good idea. They could be initialized from the common config at worktree creation. PS: I wrote that nearly all variables could be per worktree. For maintainability probably a better idea would be to start with as few as possible and extend their default list as it's clear nothing will break. -- Max ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v3] config.c: split some variables to $GIT_DIR/config.worktree 2015-03-26 12:04 ` [PATCH v2] config.c: split some variables to $GIT_DIR/config.worktree Nguyễn Thái Ngọc Duy 2015-03-26 22:19 ` Max Kirillov @ 2015-03-31 12:14 ` Nguyễn Thái Ngọc Duy 2015-03-31 12:17 ` Duy Nguyen ` (2 more replies) 1 sibling, 3 replies; 22+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2015-03-31 12:14 UTC (permalink / raw) To: git Cc: Junio C Hamano, Max Kirillov, Jens.Lehmann, Nguyễn Thái Ngọc Duy .git/info/config.worktree is a pattern list that splits .git/config in to sets, the common set that does not match the patterns and the worktree set. In normal worktree, or in the main worktree when "git checkout --to" is used, both sets are stored in .git/config. Nothing interesting. In linked worktrees, the common and worktree sets are read from and saved to .git/config and .git/config.worktree respectively. Config keys in .git/config that belong to the worktree set is ignored. Those are for the main worktree only. The effect is similar to the $GIT_DIR/$GIT_COMMON_DIR split, we can define that some vars can be shared and some cannot. core.worktree and core.bare, which are treated specially in 31e26eb [1], are now moved to info/core.worktree and the special treatment reverted. [1] 31e26eb (setup.c: support multi-checkout repo setup - 2014-11-30) Helped-by: Max Kirillov <max@max630.net> Helped-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- Much happier with this version, especially when we can now generalize the treatment for core.worktree and core.bare. The general principle is like in the last mail: .git/config is for both shared and private keys of main worktree (i.e. nothing is changed from today). .git/worktrees/xx/config.worktree is for private keys only (and private keys in .git/config are ignored) With this we don't have to bump core.repository_format_version for main worktree because nothing is changed. There will be problems with info/config.worktree: - it's customizable, so expect the user to break it (*) - if we add new stuff to the template, we'll need to help migrate current info/core.worktree (which does not have new stuff). Auto updating this file could be risky. I'm tend to just warn the user that this and that keys should be included and let them modify the file. (*) but I don't want to keep the hard coded version of this in git binary either, simply to avoid this customization problem. The list compiled by Max shows that many keys may or may not be shared. So let the user decide what's best for them. Documentation/config.txt | 10 ++++ Documentation/gitrepository-layout.txt | 6 +++ builtin/config.c | 8 +++ cache.h | 2 +- config.c | 97 ++++++++++++++++++++++++++++++++-- dir.c | 41 ++++++++++++++ dir.h | 1 + setup.c | 8 +-- t/t2025-checkout-to.sh | 26 +++++++++ templates/info--config.worktree (new) | 2 + 10 files changed, 189 insertions(+), 12 deletions(-) create mode 100644 templates/info--config.worktree diff --git a/Documentation/config.txt b/Documentation/config.txt index 2700a1b..ca4de1f 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -8,6 +8,16 @@ is used to store the configuration for that repository, and fallback values for the `.git/config` file. The file `/etc/gitconfig` can be used to store a system-wide default configuration. +Linked worktrees (see "multple working trees" in +linkgit:git-checkout[1]) have two config files under its ".git" +directory: "config" is shared with other worktrees while +"config.worktree" is per worktree. Configuration variables that match +patterns in $GIT_DIR/info/config.worktree are stored and read from +"config.worktree". Matched configuration variables in "config" is +ignored by linked worktrees. They are only used by the main +worktree. The patterns follow gitignore syntax, where the separator is +'.' instead of '/'. + The configuration variables are used by both the Git plumbing and the porcelains. The variables are divided into sections, wherein the fully qualified variable name of the variable itself is the last diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt index 7173b38..6228280 100644 --- a/Documentation/gitrepository-layout.txt +++ b/Documentation/gitrepository-layout.txt @@ -212,6 +212,12 @@ info/sparse-checkout:: This file stores sparse checkout patterns. See also: linkgit:git-read-tree[1]. +info/checkout.worktree: + This file stores the configuration variable pattern list where + $GIT_DIR/config.worktree is used as storage instead of + $GIT_DIR/config. The syntax is the same as .gitignore except + that '.' is considered the separator instead of '/'. + remotes:: Stores shorthands for URL and default refnames for use when interacting with remote repositories via 'git fetch', diff --git a/builtin/config.c b/builtin/config.c index 8cc2604..4ca8fc2 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -555,6 +555,14 @@ int cmd_config(int argc, const char **argv, const char *prefix) usage_with_options(builtin_config_usage, builtin_config_options); } + /* + * For set operations, --local could be either config or + * config.worktree. Let config.c determine the path based on + * config keys. + */ + if (use_local_config && actions != ACTION_LIST) + given_config_source.file = NULL; + if (actions == ACTION_LIST) { check_argc(argc, 0, 0); if (git_config_with_options(show_all_config, NULL, diff --git a/cache.h b/cache.h index 670e861..9431117 100644 --- a/cache.h +++ b/cache.h @@ -1355,7 +1355,7 @@ extern void git_config(config_fn_t fn, void *); extern int git_config_with_options(config_fn_t fn, void *, struct git_config_source *config_source, int respect_includes); -extern int git_config_early(config_fn_t fn, void *, const char *repo_config); +extern int git_config_early(config_fn_t fn, void *, const char *repo_config, const char *worktree_config); extern int git_parse_ulong(const char *, unsigned long *); extern int git_config_int(const char *, const char *); extern int64_t git_config_int64(const char *, const char *); diff --git a/config.c b/config.c index 15a2983..3857023 100644 --- a/config.c +++ b/config.c @@ -12,6 +12,7 @@ #include "quote.h" #include "hashmap.h" #include "string-list.h" +#include "dir.h" struct config_source { struct config_source *prev; @@ -37,6 +38,7 @@ struct config_source { }; static struct config_source *cf; +static struct exclude_list config_local; static int zlib_compression_seen; @@ -84,6 +86,33 @@ static long config_buf_ftell(struct config_source *conf) return conf->u.buf.pos; } +static int is_config_local(const char *key_) +{ + static struct strbuf key = STRBUF_INIT; + static int loaded; + int i, dtype; + + if (!loaded) { + load_config_worktree(&config_local, git_path("info/config.worktree")); + loaded = 1; + } + + if (!config_local.nr) + return 0; + + strbuf_reset(&key); + strbuf_addstr(&key, key_); + for (i = 0; i < key.len; i++) { + if (key.buf[i] == '.') + key.buf[i] = '/'; + else + key.buf[i] = tolower(key.buf[i]); + } + dtype = DT_REG; + return is_excluded_from_list(key.buf, key.len, "", &dtype, + &config_local) > 0; +} + #define MAX_INCLUDE_DEPTH 10 static const char include_depth_advice[] = "exceeded maximum include depth (%d) while including\n" @@ -1167,7 +1196,29 @@ int git_config_system(void) return !git_env_bool("GIT_CONFIG_NOSYSTEM", 0); } -int git_config_early(config_fn_t fn, void *data, const char *repo_config) +static int config_worktree_filter_in(const char *var, + const char *value, void *data) +{ + struct config_include_data *inc = data; + + if (!is_config_local(var)) + return error("%s in config.worktree is ignored", var); + return inc->fn(var, value, inc->data); +} + +static int config_worktree_filter_out(const char *var, + const char *value, void *data) +{ + struct config_include_data *inc = data; + + if (is_config_local(var)) + return 0; /* these are for main worktree only */ + + return inc->fn(var, value, inc->data); +} + +int git_config_early(config_fn_t fn, void *data, const char *repo_config, + const char *worktree_config) { int ret = 0, found = 0; char *xdg_config = NULL; @@ -1191,7 +1242,23 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config) found += 1; } - if (repo_config && !access_or_die(repo_config, R_OK, 0)) { + if (worktree_config) { + struct config_include_data inc = CONFIG_INCLUDE_INIT; + + inc.fn = fn; + inc.data = data; + if (!access_or_die(worktree_config, R_OK, 0)) { + ret += git_config_from_file(config_worktree_filter_in, + worktree_config, &inc); + found += 1; + } + + if (repo_config && !access_or_die(repo_config, R_OK, 0)) { + ret += git_config_from_file(config_worktree_filter_out, + repo_config, &inc); + found += 1; + } + } else if (repo_config && !access_or_die(repo_config, R_OK, 0)) { ret += git_config_from_file(fn, repo_config, data); found += 1; } @@ -1217,6 +1284,7 @@ int git_config_with_options(config_fn_t fn, void *data, int respect_includes) { char *repo_config = NULL; + char *worktree_config = NULL; int ret; struct config_include_data inc = CONFIG_INCLUDE_INIT; @@ -1239,9 +1307,11 @@ int git_config_with_options(config_fn_t fn, void *data, return git_config_from_blob_ref(fn, config_source->blob, data); repo_config = git_pathdup("config"); - ret = git_config_early(fn, data, repo_config); - if (repo_config) - free(repo_config); + if (git_common_dir_env) + worktree_config = git_pathdup("config.worktree"); + ret = git_config_early(fn, data, repo_config, worktree_config); + free(repo_config); + free(worktree_config); return ret; } @@ -1932,6 +2002,23 @@ int git_config_set_multivar_in_file(const char *config_filename, store.multi_replace = multi_replace; + if (git_common_dir_env && is_config_local(key)) { + if (!config_filename) + config_filename = filename_buf = git_pathdup("config.worktree"); + /* cheap trick, but should work 90% of time */ + else if (!ends_with(config_filename, ".worktree")) + die("%s can only be stored in %s", + key, git_path("config.worktree")); + else { + struct strbuf sb = STRBUF_INIT; + strbuf_addstr(&sb, real_path(config_filename)); + if (strcmp_icase(sb.buf, + real_path(git_path("config.worktree")))) + die("%s can only be stored in %s", + key, git_path("config.worktree")); + strbuf_release(&sb); + } + } if (!config_filename) config_filename = filename_buf = git_pathdup("config"); diff --git a/dir.c b/dir.c index 3f7a025..69bf491 100644 --- a/dir.c +++ b/dir.c @@ -589,6 +589,47 @@ int add_excludes_from_file_to_list(const char *fname, return 0; } +void load_config_worktree(struct exclude_list *el, const char *path) +{ + struct strbuf sb = STRBUF_INIT; + int i, lineno = 1; + char *buf, *entry; + size_t size; + + if (strbuf_read_file(&sb, path, 128) <= 0) { + strbuf_release(&sb); + return; + } + strbuf_addch(&sb, '\n'); + el->filebuf = buf = strbuf_detach(&sb, &size); + + for (i = 0; i < size; i++) + if (buf[i] == '.') + buf[i] = '/'; + else + buf[i] = tolower(buf[i]); + + entry = buf; + for (i = 0; i < size; i++) { + if (buf[i] == '\n') { + if (entry != buf + i && entry[0] != '#') { + buf[i - (i && buf[i-1] == '\r')] = 0; + trim_trailing_spaces(entry); + add_exclude(entry, "", 0, el, lineno); + } + lineno++; + entry = buf + i + 1; + } + } + + /* + * avoid base name matching because it may confusion in + * non-directory context. + */ + for (i = 0; i < el->nr; i++) + el->excludes[i]->flags &= ~EXC_FLAG_NODIR; +} + struct exclude_list *add_exclude_list(struct dir_struct *dir, int group_type, const char *src) { diff --git a/dir.h b/dir.h index 6c45e9d..539423d 100644 --- a/dir.h +++ b/dir.h @@ -165,6 +165,7 @@ extern struct exclude_list *add_exclude_list(struct dir_struct *dir, int group_type, const char *src); extern int add_excludes_from_file_to_list(const char *fname, const char *base, int baselen, struct exclude_list *el, int check_index); +extern void load_config_worktree(struct exclude_list *el, const char *fname); extern void add_excludes_from_file(struct dir_struct *, const char *fname); extern void parse_exclude_pattern(const char **string, int *patternlen, int *flags, int *nowildcardlen); extern void add_exclude(const char *string, const char *base, diff --git a/setup.c b/setup.c index fb61860..61e0403 100644 --- a/setup.c +++ b/setup.c @@ -356,13 +356,9 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok) { struct strbuf sb = STRBUF_INIT; const char *repo_config; - config_fn_t fn; int ret = 0; - if (get_common_dir(&sb, gitdir)) - fn = check_repo_format; - else - fn = check_repository_format_version; + get_common_dir(&sb, gitdir); strbuf_addstr(&sb, "/config"); repo_config = sb.buf; @@ -375,7 +371,7 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok) * Use a gentler version of git_config() to check if this repo * is a good one. */ - git_config_early(fn, NULL, repo_config); + git_config_early(check_repository_format_version, NULL, repo_config, NULL); if (GIT_REPO_VERSION < repository_format_version) { if (!nongit_ok) die ("Expected git repo version <= %d, found %d", diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh index f8e4df4..6f98279 100755 --- a/t/t2025-checkout-to.sh +++ b/t/t2025-checkout-to.sh @@ -126,4 +126,30 @@ test_expect_success 'checkout with grafts' ' test_cmp expected actual ' +test_expect_success 'setting worktree.foo goes to config.worktree' ' + echo worKtree.Foo >> .git/info/config.worktree && + git checkout --to wt.foo HEAD && + git config woRKtree.FOO barrrr && + git --git-dir=wt.foo/.git config woRKtree.FOO bar && + cat >expect <<\EOF && +[woRKtree] + FOO = bar +EOF + test_cmp expect .git/worktrees/wt.foo/config.worktree && + git --git-dir=wt.foo/.git config woRktree.foo >actual2 && + echo bar >expect2 && + test_cmp expect2 actual2 && + test_path_is_missing .git/config.worktree && + git config WORKTREE.FOO >actual3 && + echo barrrr >expect3 && + test_cmp expect3 actual3 +' + +test_expect_success 'shared config still goes to config' ' + git config random.key randomValue && + git --git-dir=wt.foo/.git config random.key >actual && + echo randomValue >expect && + test_cmp expect actual +' + test_done diff --git a/templates/info--config.worktree b/templates/info--config.worktree new file mode 100644 index 0000000..f358230 --- /dev/null +++ b/templates/info--config.worktree @@ -0,0 +1,2 @@ +core.worktree +core.bare -- 2.3.0.rc1.137.g477eb31 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3] config.c: split some variables to $GIT_DIR/config.worktree 2015-03-31 12:14 ` [PATCH v3] " Nguyễn Thái Ngọc Duy @ 2015-03-31 12:17 ` Duy Nguyen 2015-04-01 20:56 ` Max Kirillov 2015-04-13 23:37 ` Max Kirillov 2 siblings, 0 replies; 22+ messages in thread From: Duy Nguyen @ 2015-03-31 12:17 UTC (permalink / raw) To: Git Mailing List Cc: Junio C Hamano, Max Kirillov, Jens Lehmann, Nguyễn Thái Ngọc Duy On Tue, Mar 31, 2015 at 7:14 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote: > The general principle is like in the last mail: .git/config is for > both shared and private keys of main worktree (i.e. nothing is > changed from today). .git/worktrees/xx/config.worktree is for > private keys only (and private keys in .git/config are ignored) When I put it this way, I realize that .git/worktrees/xx/config.worktree can simply be .git/worktrees/xx/config. No ".worktree". Looks nice (or it's just a source of more confusion later) -- Duy ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3] config.c: split some variables to $GIT_DIR/config.worktree 2015-03-31 12:14 ` [PATCH v3] " Nguyễn Thái Ngọc Duy 2015-03-31 12:17 ` Duy Nguyen @ 2015-04-01 20:56 ` Max Kirillov 2015-04-03 10:30 ` Duy Nguyen 2015-04-13 23:37 ` Max Kirillov 2 siblings, 1 reply; 22+ messages in thread From: Max Kirillov @ 2015-04-01 20:56 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, Jens.Lehmann On Tue, Mar 31, 2015 at 07:14:39PM +0700, Nguyễn Thái Ngọc Duy wrote: > The general principle is like in the last mail: .git/config is for > both shared and private keys of main worktree (i.e. nothing is > changed from today). .git/worktrees/xx/config.worktree is for > private keys only (and private keys in .git/config are ignored) > > With this we don't have to bump core.repository_format_version for > main worktree because nothing is changed. There will be problems > with info/config.worktree: > > - it's customizable, so expect the user to break it (*) I would rather say it's manual tuning rather than a break. > - if we add new stuff to the template, we'll need to help migrate > current info/core.worktree (which does not have new stuff). > Auto updating this file could be risky. I'm tend to just > warn the user that this and that keys should be included and let > them modify the file. I don't think there even should be warning. Just don't change the info/config.worktree in the existing repositories and let users extend it as they feel a need for it. Later there could be a tool (an option to git config for example) which adds a variable or pattern to the info/core.worktree and copied existing variable(s) from commong config to worktree-specific ones. .... > diff --git a/config.c b/config.c > index 15a2983..3857023 100644 > --- a/config.c > +++ b/config.c .... > @@ -1932,6 +2002,23 @@ int git_config_set_multivar_in_file(const char *config_filename, > > store.multi_replace = multi_replace; > > + if (git_common_dir_env && is_config_local(key)) { > + if (!config_filename) > + config_filename = filename_buf = git_pathdup("config.worktree"); > + /* cheap trick, but should work 90% of time */ > + else if (!ends_with(config_filename, ".worktree")) > + die("%s can only be stored in %s", > + key, git_path("config.worktree")); Is `config_filename` set only for cases when config file is explicitly set for "git config" command with "--file" option? Then probably here should not be any intelligence at all; if user resort to manual picking the file he must be allowed to do stupid things. -- Max ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3] config.c: split some variables to $GIT_DIR/config.worktree 2015-04-01 20:56 ` Max Kirillov @ 2015-04-03 10:30 ` Duy Nguyen 0 siblings, 0 replies; 22+ messages in thread From: Duy Nguyen @ 2015-04-03 10:30 UTC (permalink / raw) To: Max Kirillov; +Cc: Git Mailing List, Junio C Hamano, Jens Lehmann On Thu, Apr 2, 2015 at 3:56 AM, Max Kirillov <max@max630.net> wrote: > On Tue, Mar 31, 2015 at 07:14:39PM +0700, Nguyễn Thái Ngọc Duy wrote: >> The general principle is like in the last mail: .git/config is for >> both shared and private keys of main worktree (i.e. nothing is >> changed from today). .git/worktrees/xx/config.worktree is for >> private keys only (and private keys in .git/config are ignored) >> >> With this we don't have to bump core.repository_format_version for >> main worktree because nothing is changed. There will be problems >> with info/config.worktree: >> >> - it's customizable, so expect the user to break it (*) > > I would rather say it's manual tuning rather than a break. But if the user accidentally deletes core.worktree then something will break. >> - if we add new stuff to the template, we'll need to help migrate >> current info/core.worktree (which does not have new stuff). >> Auto updating this file could be risky. I'm tend to just >> warn the user that this and that keys should be included and let >> them modify the file. > > I don't think there even should be warning. Just don't > change the info/config.worktree in the existing repositories > and let users extend it as they feel a need for it. > > Later there could be a tool (an option to git config for > example) which adds a variable or pattern to the > info/core.worktree and copied existing variable(s) from > commong config to worktree-specific ones. I was thinking "git checkout --to" would do this. It checks if the main worktree has info/core.worktree, if not, re-init the repo to add this file from default template. >> @@ -1932,6 +2002,23 @@ int git_config_set_multivar_in_file(const char *config_filename, >> >> store.multi_replace = multi_replace; >> >> + if (git_common_dir_env && is_config_local(key)) { >> + if (!config_filename) >> + config_filename = filename_buf = git_pathdup("config.worktree"); >> + /* cheap trick, but should work 90% of time */ >> + else if (!ends_with(config_filename, ".worktree")) >> + die("%s can only be stored in %s", >> + key, git_path("config.worktree")); > > Is `config_filename` set only for cases when config file is > explicitly set for "git config" command with "--file" > option? Then probably here should not be any intelligence at > all; if user resort to manual picking the file he must be > allowed to do stupid things. Yes. It may make sense in the previous versions where this feature could work in a normal worktree, but when it becomes multiworktree-specific, I guess we can drop this. -- Duy ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3] config.c: split some variables to $GIT_DIR/config.worktree 2015-03-31 12:14 ` [PATCH v3] " Nguyễn Thái Ngọc Duy 2015-03-31 12:17 ` Duy Nguyen 2015-04-01 20:56 ` Max Kirillov @ 2015-04-13 23:37 ` Max Kirillov 2015-04-18 11:10 ` Duy Nguyen 2 siblings, 1 reply; 22+ messages in thread From: Max Kirillov @ 2015-04-13 23:37 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, Jens.Lehmann On Tue, Mar 31, 2015 at 07:14:39PM +0700, Nguyễn Thái Ngọc Duy wrote: > core.worktree and core.bare, which are treated specially in 31e26eb [1], > are now moved to info/core.worktree and the special treatment reverted. <...> > - if (get_common_dir(&sb, gitdir)) > - fn = check_repo_format; > - else > - fn = check_repository_format_version; By the way, after this '$GIT_DIR/common overrides core.worktree' from t1501 started failing. I don't know what would be better to do with the case, just remove maybe? -- Max ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3] config.c: split some variables to $GIT_DIR/config.worktree 2015-04-13 23:37 ` Max Kirillov @ 2015-04-18 11:10 ` Duy Nguyen 2015-04-20 2:51 ` Max Kirillov 0 siblings, 1 reply; 22+ messages in thread From: Duy Nguyen @ 2015-04-18 11:10 UTC (permalink / raw) To: Max Kirillov; +Cc: git, Junio C Hamano, Jens.Lehmann On Tue, Apr 14, 2015 at 02:37:39AM +0300, Max Kirillov wrote: > On Tue, Mar 31, 2015 at 07:14:39PM +0700, Nguyễn Thái Ngọc Duy wrote: > > core.worktree and core.bare, which are treated specially in 31e26eb [1], > > are now moved to info/core.worktree and the special treatment reverted. > <...> > > - if (get_common_dir(&sb, gitdir)) > > - fn = check_repo_format; > > - else > > - fn = check_repository_format_version; > > By the way, after this '$GIT_DIR/common overrides core.worktree' > from t1501 started failing. I don't know what would be > better to do with the case, just remove maybe? I think that test spots a real problem. In this function, I ignore the config split when I pass NULL as worktree_config to git_config_early(). Something like this should fix it. -- 8< -- diff --git a/setup.c b/setup.c index 61e0403..4799e7d 100644 --- a/setup.c +++ b/setup.c @@ -355,10 +355,15 @@ static int check_repo_format(const char *var, const char *value, void *cb) static int check_repository_format_gently(const char *gitdir, int *nongit_ok) { struct strbuf sb = STRBUF_INIT; + struct strbuf sb_wt = STRBUF_INIT; const char *repo_config; + const char *worktree_config = NULL; int ret = 0; - get_common_dir(&sb, gitdir); + if (get_common_dir(&sb, gitdir)) { + strbuf_addf(&sb_wt, "%s/config.worktree", get_git_dir()); + worktree_config = sb_wt.buf; + } strbuf_addstr(&sb, "/config"); repo_config = sb.buf; @@ -371,7 +376,8 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok) * Use a gentler version of git_config() to check if this repo * is a good one. */ - git_config_early(check_repository_format_version, NULL, repo_config, NULL); + git_config_early(check_repository_format_version, NULL, + repo_config, worktree_config); if (GIT_REPO_VERSION < repository_format_version) { if (!nongit_ok) die ("Expected git repo version <= %d, found %d", @@ -383,6 +389,7 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok) ret = -1; } strbuf_release(&sb); + strbuf_release(&sb_wt); return ret; } -- 8< -- ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v3] config.c: split some variables to $GIT_DIR/config.worktree 2015-04-18 11:10 ` Duy Nguyen @ 2015-04-20 2:51 ` Max Kirillov 2015-04-20 3:22 ` Duy Nguyen 0 siblings, 1 reply; 22+ messages in thread From: Max Kirillov @ 2015-04-20 2:51 UTC (permalink / raw) To: Duy Nguyen; +Cc: Max Kirillov, git, Junio C Hamano, Jens.Lehmann On Sat, Apr 18, 2015 at 06:10:23PM +0700, Duy Nguyen wrote: > On Tue, Apr 14, 2015 at 02:37:39AM +0300, Max Kirillov wrote: > > On Tue, Mar 31, 2015 at 07:14:39PM +0700, Nguyễn Thái Ngọc Duy wrote: > > > core.worktree and core.bare, which are treated specially in 31e26eb [1], > > > are now moved to info/core.worktree and the special treatment reverted. > > <...> > > > - if (get_common_dir(&sb, gitdir)) > > > - fn = check_repo_format; > > > - else > > > - fn = check_repository_format_version; > > > > By the way, after this '$GIT_DIR/common overrides core.worktree' > > from t1501 started failing. I don't know what would be > > better to do with the case, just remove maybe? > > I think that test spots a real problem. In this function, I ignore the > config split when I pass NULL as worktree_config to git_config_early(). > Something like this should fix it. I just realized that the testcase does have info/config.worktree with the variable included, because any repository has it. Then, of course, the variable from common directory should be ignored and the testcase is correct. By the way, if checkout --to run on repository created before the feaure added, the linked checkout will not be able to use own variables. Should checkout --to check that the file exists and create it in case it does not? ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v3] config.c: split some variables to $GIT_DIR/config.worktree 2015-04-20 2:51 ` Max Kirillov @ 2015-04-20 3:22 ` Duy Nguyen 0 siblings, 0 replies; 22+ messages in thread From: Duy Nguyen @ 2015-04-20 3:22 UTC (permalink / raw) To: Max Kirillov; +Cc: Git Mailing List, Junio C Hamano, Jens Lehmann On Mon, Apr 20, 2015 at 9:51 AM, Max Kirillov <max@max630.net> wrote: > By the way, if checkout --to run on repository created > before the feaure added, the linked checkout will not be > able to use own variables. Should checkout --to check that > the file exists and create it in case it does not? Yes it should. I made some changes for that [1] but haven't posted in the list yet because it's incomplete imo. I should check again after reinitializing the repo, whether core.worktree is in info/config.worktree, if not warn the user. This could happen if the user defines a custom template. [1] https://github.com/pclouds/git/commit/f2276a37039d7a1f99fbcbc72e7d87b0782e4663 -- Duy ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: per-repository and per-worktree config variables 2015-03-18 21:33 ` per-repository and per-worktree config variables Max Kirillov 2015-03-24 13:48 ` Duy Nguyen @ 2015-03-25 21:33 ` Jens Lehmann 1 sibling, 0 replies; 22+ messages in thread From: Jens Lehmann @ 2015-03-25 21:33 UTC (permalink / raw) To: Max Kirillov, Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano Am 18.03.2015 um 22:33 schrieb Max Kirillov: > On Sun, Feb 08, 2015 at 09:36:43AM -0800, Jens Lehmann wrote: >> I wonder if it's worth all the hassle to invent new names. Wouldn't >> it be much better to just keep a list of per-worktree configuration >> value names and use that inside the config code to decide where to >> find them for multiple work trees. That would also work easily for >> stuff like EOL-config and would push the complexity in the config >> machinery and not onto the user. > > I actually thought about the same, and now tend to think > that most of config variables make sense to be per-worktree > in some cases. Only few variable must always be per > repository. I tried to summarize the variables which now > (in current pu) should be common, also listed all the rest > so somebody could scan through the list and spot anything I > could miss. Thanks for your effort! Looks like my suspicion that not only submodule specific configuration should be kept per worktree wasn't completely wrong ;-) Me thinks Duy's proposal to configure the per worktree settings in a config file might make lots of sense. We could then provide a default version of that file with settings that fit most use cases and the user could then adapt that to her special needs if needed. ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2015-04-20 3:23 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-02-08 13:16 [PATCH/RFD 0/3] worktree.* config keys and submodule and multiple worktrees Nguyễn Thái Ngọc Duy 2015-02-08 13:16 ` [PATCH 1/3] config.c: new config namespace worktree.* stored in $GIT_DIR/config.worktree Nguyễn Thái Ngọc Duy 2015-02-08 13:16 ` [PATCH 2/3] setup: add worktree.path to shadow core.worktree Nguyễn Thái Ngọc Duy 2015-02-08 13:16 ` [PATCH 3/3] submodule: use worktree.path instead of core.worktree Nguyễn Thái Ngọc Duy 2015-02-08 17:36 ` [PATCH/RFD 0/3] worktree.* config keys and submodule and multiple worktrees Jens Lehmann 2015-02-08 17:41 ` Jens Lehmann 2015-02-09 9:35 ` Duy Nguyen 2015-03-18 21:33 ` per-repository and per-worktree config variables Max Kirillov 2015-03-24 13:48 ` Duy Nguyen 2015-03-26 12:04 ` [PATCH v2] config.c: split some variables to $GIT_DIR/config.worktree Nguyễn Thái Ngọc Duy 2015-03-26 22:19 ` Max Kirillov 2015-03-29 1:25 ` Duy Nguyen 2015-03-30 21:26 ` Max Kirillov 2015-03-31 12:14 ` [PATCH v3] " Nguyễn Thái Ngọc Duy 2015-03-31 12:17 ` Duy Nguyen 2015-04-01 20:56 ` Max Kirillov 2015-04-03 10:30 ` Duy Nguyen 2015-04-13 23:37 ` Max Kirillov 2015-04-18 11:10 ` Duy Nguyen 2015-04-20 2:51 ` Max Kirillov 2015-04-20 3:22 ` Duy Nguyen 2015-03-25 21:33 ` per-repository and per-worktree config variables Jens Lehmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).