Re: [PATCH 1/3] fdopen_lock_file(): access a lockfile using stdio
On 01.10.14 13:14, Michael Haggerty wrote:
[]
Nice done, small comments inline
diff --git a/lockfile.c b/lockfile.c
index d27e61c..e046027 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -7,20 +7,29 @@
static struct lock_file *volatile lock_file_list;
-static void remove_lock_files(void)
+static void remove_lock_files(int skip_fclose)
Even if the motivation to skip is clear now and here,
I would consider to do it the other way around,
and actively order the fclose():
static void remove_lock_files(int call_fclose)
{
pid_t me = getpid();
while (lock_file_list) {
- if (lock_file_list-owner == me)
+ if (lock_file_list-owner == me) {
+ /* fclose() is not safe to call in a signal handler */
+ if (skip_fclose)
+ lock_file_list-fp = NULL;
rollback_lock_file(lock_file_list);
+ }
lock_file_list = lock_file_list-next;
}
}
+static void remove_lock_files_on_exit(void)
+{
+ remove_lock_files(0);
What does 0 mean ?
remove_lock_files(LK_DO_FCLOSE) ?
+}
+
static void remove_lock_files_on_signal(int signo)
{
- remove_lock_files();
+ remove_lock_files(1);
And what does this 1 mean ?
remove_lock_files(LK_SKIP_FCLOSE) ?
We can even have an emum, or use #define
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] init - Honour the global core.filemode setting
On 2014-10-01 19.10, Junio C Hamano wrote: Hilco Wijbenga hilco.wijbe...@gmail.com writes: Perhaps I completely misunderstand the meaning of core.filemode but I thought it determined whether Git cared about changes in file properties? By setting it to false, you tell Git that the filesystem you placed the repository does not correctly represent the filemode (especially the executable bit). core.fileMode in git config --help reads: core.fileMode If false, the executable bit differences between the index and the working tree are ignored; useful on broken filesystems like FAT. See git-update- index(1). Out of my head: Could the following be a starting point: core.fileMode If false, the executable bit differences between the index and the working tree are ignored. This may be usefull when visiting a cygwin repo with a non-cygwin Git client. (should we mention msysgit ? should we mention JGit/EGit ?) This may even be useful for a repo on a SAMBA network mount, which may show all file permissions as 0755. See git-update-index(1) for changing the executable bit in the index. The default is true, except git-clone(1) or git-init(1) will probe and set core.fileMode false if appropriate when the repository is created. Maybe our documentation is not clear enough. A contribution from somebody new to Git we would appreciate would be to point out which part of these sentences are unclear; that way, people can work on improving its phrasing. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Failing tests in t0027-autocrlf.sh under msysgit/git-win-sdk
Hi, I've enabled EXPENSIVE and ran the git test suite under msysgit/git-win-sdk with git version 2.1.0.9753.g360f311.dirty. Now I have some failing tests in t0027-autocrlf.sh in the MINGW only section which puzzle me. The offending test sets are diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh index 72dd3e8..90c4cd1 100755 --- a/t/t0027-auto-crlf.sh +++ b/t/t0027-auto-crlf.sh @@ -245,18 +245,18 @@ if test_have_prereq MINGW then check_files_in_ws false LFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul check_files_in_ws true CRLF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul -check_files_in_ws false autoLFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul +# check_files_in_ws false autoLFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul # first broken check_files_in_ws true autoCRLF CRLF CRLF LF_mix_CR CRLF_nul -check_files_in_ws false textLFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul +# check_files_in_ws false textLFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul # broken check_files_in_ws true textCRLF CRLF CRLF CRLF_mix_CR CRLF_nul check_files_in_ws false -text LFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul check_files_in_ws true -text LFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul check_files_in_ws native false LFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul check_files_in_ws native true CRLF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul -check_files_in_ws native false autoLFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul +# check_files_in_ws native false autoLFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul # broken check_files_in_ws native true autoCRLF CRLF CRLF LF_mix_CR CRLF_nul -check_files_in_ws native false textLFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul +# check_files_in_ws native false textLFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul # broken check_files_in_ws native true textCRLF CRLF CRLF CRLF_mix_CR CRLF_nul check_files_in_ws native false -text LFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul check_files_in_ws native true -text LFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul I tried with NATIVE_CRLF = YesPlease but 117 failed too. First question, under what MINGW system do these tests pass? Second question any hints how to tackle this? The first failing test is not ok 117 - checkout core.eol= core.autocrlf=false gitattributes=auto file=LF # # compare_ws_file eol__crlf_false_attr_auto_ LF crlf_false_attr__LF.txt # where I have in the trash directory $ diff -Nur *expect* *actual* --- LF.expect 2014-10-02 12:15:17 + +++ eol__crlf_false_attr_auto_.actual.crlf_false_attr__LF.txt 2014-10-02 12:15 :17 + @@ -1,3 +1,3 @@ -000 l i n e 1 \n l i n e 2 \n l i n e -020 3 -021 +000 l i n e 1 \r \n l i n e 2 \r \n l i +020 n e 3 +023 Reading convert.h tells me that for undefined NATIVE_CRLF the native EOL is LF. Which looks like the test is correct. Thomas -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 0/5] add unset.variable for unsetting previously set variables
Hi, This series aims to add a method to filter previously set variables. The patch series can be best described by the 3/5 log message which I have pasted below verbatim. Add a new config variable unset.variable which unsets previously set variables. It affects `git_config()` and `git_config_get_*()` family of functions. It removes the matching variables from the `configset` which were added previously. Those matching variables which come after the unset.variable in parsing order will not be deleted and will be left untouched. It affects the result of git config -l and similar calls. It may be used in cases where the user can not access the config files, for example, the system wide config files may be only accessible to the system administrator. We can unset an unwanted variable declared in the system config file by using unset.variable in a local config file. for example, /etc/gitconfig may look like this, [foo] bar = baz in the repo config file, we will write, [unset] variable = foo.bar to unset foo.bar previously declared in system wide config file. Now, I have some points of contention which I like to clarify, 1 The name of the variable, I could not decide between unset.variable and config.unset, or may be some other name would be more appropriate. 2 It affects both the C git_config() calls and, git config shell invocations. Due to this some variables may be absent from the git config -l result which might confuse the user. 3 I also have an another implementation for this series which just marks the config variables instead of deleting them from the configset. This can be used to provide two versions of git_config(), one with filtered variables other without it. 4 While hacking on this series, I saw that git_config_int() does not print the file name of the invalid variable when values are fed by the configset. I will correct this regression in another patch. Cheers, Tanay Documentation/config.txt | 12 +++ config.c | 93 +--- t/t1300-repo-config.sh | 56 - 3 files changed, 139 insertions(+), 22 deletions(-) -- 1.9.0.GIT -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 4/5] document the new unset.variable variable
Helped-by: Matthieu Moy matthieu@imag.fr Signed-off-by: Tanay Abhra tanay...@gmail.com --- Documentation/config.txt | 12 1 file changed, 12 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 3b5b24a..7f36d35 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2382,6 +2382,18 @@ transfer.unpackLimit:: not set, the value of this variable is used instead. The default value is 100. +unset.variable:: + This variable can be used to unset previously set variables + which had been already declared in files of lower priority + or declared before in the same file. It does not unset + matching variables declared after its position in the file + or in files of higher priority. It can be used to unset + pesky variables declared in files which the user might not + be able to open due to not having the required security + privileges, for example, system wide configuration file + `/etc/gitconfig` which may be accessible to the system + administrator only. + uploadarchive.allowUnreachable:: If true, allow clients to use `git archive --remote` to request any tree, whether reachable from the ref tips or not. See the -- 1.9.0.GIT -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 2/5] make git_config_with_options() to use a configset
Make git_config_with_options() to use a configset to feed values
in the callback function. This change gives us the power to filter
variables we feed to the callback using custom constraints.
A slight behaviour change, git_config_int() loses the ability to
print the file name of the invalid variable while dying.
Helped-by: Matthieu Moy matthieu@imag.fr
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
config.c | 21 +++--
t/t1300-repo-config.sh | 2 +-
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/config.c b/config.c
index cb474b2..09cf009 100644
--- a/config.c
+++ b/config.c
@@ -1214,7 +1214,7 @@ int git_config_early(config_fn_t fn, void *data, const
char *repo_config)
return ret == 0 ? found : ret;
}
-int git_config_with_options(config_fn_t fn, void *data,
+static int git_config_with_options_raw(config_fn_t fn, void *data,
struct git_config_source *config_source,
int respect_includes)
{
@@ -1247,9 +1247,26 @@ int git_config_with_options(config_fn_t fn, void *data,
return ret;
}
+static int config_set_callback(const char *key, const char *value, void *cb);
+
+int git_config_with_options(config_fn_t fn, void *data,
+ struct git_config_source *config_source,
+ int respect_includes)
+{
+ int ret;
+ struct config_set options_config;
+ git_configset_init(options_config);
+ ret = git_config_with_options_raw(config_set_callback, options_config,
+ config_source, respect_includes);
+ if (ret = 0)
+ configset_iter(options_config, fn, data);
+ git_configset_clear(options_config);
+ return ret;
+}
+
static void git_config_raw(config_fn_t fn, void *data)
{
- if (git_config_with_options(fn, data, NULL, 1) 0)
+ if (git_config_with_options_raw(fn, data, NULL, 1) 0)
/*
* git_config_with_options() normally returns only
* positive values, as most errors are fatal, and
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 938fc8b..ce5ea01 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -678,7 +678,7 @@ test_expect_success 'invalid unit' '
git config aninvalid.unit actual
test_cmp expect actual
cat expect -\EOF
- fatal: bad numeric config value '\''1auto'\'' for
'\''aninvalid.unit'\'' in .git/config: invalid unit
+ fatal: bad numeric config value '\''1auto'\'' for
'\''aninvalid.unit'\'': invalid unit
EOF
test_must_fail git config --int --get aninvalid.unit 2actual
test_i18ncmp expect actual
--
1.9.0.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 3/5] add unset.variable for unsetting previously set variables
Add a new config variable unset.variable which unsets previously set
variables. It affects `git_config()` and `git_config_get_*()` family
of functions. It removes the matching variables from the `configset`
which were added previously. Those matching variables which come after
the unset.variable in parsing order will not be deleted and will
be left untouched.
It affects the result of git config -l and similar calls.
It may be used in cases where the user can not access the config files,
for example, the system wide config files may be only accessible to
the system administrator. We can unset an unwanted variable declared in
the system config file by using unset.variable in a local config file.
for example, /etc/gitconfig may look like this,
[foo]
bar = baz
in the repo config file, we will write,
[unset]
variable = foo.bar
to unset foo.bar previously declared in system wide config file.
Helped-by: Matthieu Moy matthieu@imag.fr
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
config.c | 34 ++
1 file changed, 34 insertions(+)
diff --git a/config.c b/config.c
index 09cf009..a80832d 100644
--- a/config.c
+++ b/config.c
@@ -1311,6 +1311,38 @@ static struct config_set_element
*configset_find_element(struct config_set *cs,
return found_entry;
}
+static void delete_config_variable(struct config_set *cs, const char *key,
const char *value)
+{
+ char *normalized_value;
+ struct config_set_element *e = NULL;
+ int ret, current = 0, updated = 0;
+ struct configset_list *list = cs-list;
+ /*
+* if we find a key value pair with key as unset.variable, unset all
variables
+* in the configset with keys equivalent to the value in
unset.variable.
+* unsetting a variable means that the variable is permanently deleted
from the
+* configset.
+*/
+ ret = git_config_parse_key(value, normalized_value, NULL);
+ if (!ret) {
+ /* first remove matching variables from the configset_list */
+ while (current list-nr) {
+ if (!strcmp(list-items[current].e-key,
normalized_value))
+ current++;
+ else
+ list-items[updated++] = list-items[current++];
+ }
+ list-nr = updated;
+ /* then delete the matching entry from the configset hashmap */
+ e = configset_find_element(cs, normalized_value);
+ if (e) {
+ free(e-key);
+ string_list_clear(e-value_list, 1);
+ hashmap_remove(cs-config_hash, e, NULL);
+ }
+ }
+}
+
static int configset_add_value(struct config_set *cs, const char *key, const
char *value)
{
struct config_set_element *e;
@@ -1331,6 +1363,8 @@ static int configset_add_value(struct config_set *cs,
const char *key, const cha
hashmap_add(cs-config_hash, e);
}
si = string_list_append_nodup(e-value_list, value ? xstrdup(value) :
NULL);
+ if (!strcmp(key, unset.variable))
+ delete_config_variable(cs, key, value);
ALLOC_GROW(cs-list.items, cs-list.nr + 1, cs-list.alloc);
l_item = cs-list.items[cs-list.nr++];
--
1.9.0.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 5/5] add tests for checking the behaviour of unset.variable
Helped-by: Matthieu Moy matthieu@imag.fr Signed-off-by: Tanay Abhra tanay...@gmail.com --- t/t1300-repo-config.sh | 54 ++ 1 file changed, 54 insertions(+) diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index ce5ea01..f75c001 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -1179,4 +1179,58 @@ test_expect_success POSIXPERM,PERL 'preserves existing permissions' ' die q(badrename) if ((stat(q(.git/config)))[2] 0) != 0600 ' +test_expect_success 'unset.variable unsets all previous matching keys' ' + cat .git/config -\EOF + [alias] + checkconfig = -c foo.check=baz config foo.check + checkconfig = -c foo.check=bar config foo.check + [unset] + variable = alias.checkconfig + EOF + + test_expect_code 1 git checkconfig +' + +test_expect_success 'unset.variable does not touch all matching keys after it' ' + cat .git/config -\EOF + [alias] + checkconfig = -c foo.check=foo config foo.check + [unset] + variable = alias.checkconfig + [alias] + checkconfig = -c foo.check=baz config foo.check + checkconfig = -c foo.check=bar config foo.check + EOF + + cat expect -\EOF + bar + EOF + + test_expect_code 0 git checkconfig actual + test_cmp expect actual +' + +test_expect_success 'document how unset.variable will behave in shell scripts' ' + rm -f .git/config + cat expect -\EOF + EOF + git config foo.bar boz1 + git config --add foo.bar boz2 + git config unset.variable foo.bar + git config --add foo.bar boz3 + test_must_fail git config --get-all foo.bar actual + test_cmp expect actual +' + +test_expect_success 'unset.variable declared after in shell scripts' ' + rm -f .git/config + cat expect -\EOF + EOF + git config foo.bar boz1 + git config --add foo.bar boz2 + git config unset.variable foo.bar + test_must_fail git config --get-all foo.bar actual + test_cmp expect actual +' + test_done -- 1.9.0.GIT -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 1/5] config.c : move configset_iter() to an appropriate position
Move configset_iter() to an appropriate position where it
can be called by git_config_*() family without putting
a forward declaration for it.
Helped-by: Matthieu Moy matthieu@imag.fr
Signed-off-by: Tanay Abhra tanay...@gmail.com
---
config.c | 38 +++---
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/config.c b/config.c
index a677eb6..cb474b2 100644
--- a/config.c
+++ b/config.c
@@ -1150,6 +1150,25 @@ int git_config_system(void)
return !git_env_bool(GIT_CONFIG_NOSYSTEM, 0);
}
+static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
+{
+ int i, value_index;
+ struct string_list *values;
+ struct config_set_element *entry;
+ struct configset_list *list = cs-list;
+ struct key_value_info *kv_info;
+
+ for (i = 0; i list-nr; i++) {
+ entry = list-items[i].e;
+ value_index = list-items[i].value_index;
+ values = entry-value_list;
+ if (fn(entry-key, values-items[value_index].string, data)
0) {
+ kv_info = values-items[value_index].util;
+ git_die_config_linenr(entry-key, kv_info-filename,
kv_info-linenr);
+ }
+ }
+}
+
int git_config_early(config_fn_t fn, void *data, const char *repo_config)
{
int ret = 0, found = 0;
@@ -1245,25 +1264,6 @@ static void git_config_raw(config_fn_t fn, void *data)
die(_(unknown error occured while reading the configuration
files));
}
-static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
-{
- int i, value_index;
- struct string_list *values;
- struct config_set_element *entry;
- struct configset_list *list = cs-list;
- struct key_value_info *kv_info;
-
- for (i = 0; i list-nr; i++) {
- entry = list-items[i].e;
- value_index = list-items[i].value_index;
- values = entry-value_list;
- if (fn(entry-key, values-items[value_index].string, data)
0) {
- kv_info = values-items[value_index].util;
- git_die_config_linenr(entry-key, kv_info-filename,
kv_info-linenr);
- }
- }
-}
-
static void git_config_check_init(void);
void git_config(config_fn_t fn, void *data)
--
1.9.0.GIT
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Failing tests in t0027-autocrlf.sh under msysgit/git-win-sdk
On 2014-10-02 14.39, Thomas Braun wrote: Hi, I've enabled EXPENSIVE and ran the git test suite under msysgit/git-win-sdk with git version 2.1.0.9753.g360f311.dirty. Now I have some failing tests in t0027-autocrlf.sh in the MINGW only section which puzzle me. The offending test sets are diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh index 72dd3e8..90c4cd1 100755 --- a/t/t0027-auto-crlf.sh +++ b/t/t0027-auto-crlf.sh @@ -245,18 +245,18 @@ if test_have_prereq MINGW then check_files_in_ws false LFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul check_files_in_ws true CRLF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul -check_files_in_ws false autoLFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul +# check_files_in_ws false autoLFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul # first broken check_files_in_ws true autoCRLF CRLF CRLF LF_mix_CR CRLF_nul -check_files_in_ws false textLFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul +# check_files_in_ws false textLFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul # broken check_files_in_ws true textCRLF CRLF CRLF CRLF_mix_CR CRLF_nul check_files_in_ws false -text LFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul check_files_in_ws true -text LFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul check_files_in_ws native false LFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul check_files_in_ws native true CRLF CRLF CRLF_mix_LF LF_mix_CR CRLF_nul -check_files_in_ws native false autoLFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul +# check_files_in_ws native false autoLFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul # broken check_files_in_ws native true autoCRLF CRLF CRLF LF_mix_CR CRLF_nul -check_files_in_ws native false textLFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul +# check_files_in_ws native false textLFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul # broken check_files_in_ws native true textCRLF CRLF CRLF CRLF_mix_CR CRLF_nul check_files_in_ws native false -text LFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul check_files_in_ws native true -text LFCRLF CRLF_mix_LF LF_mix_CR CRLF_nul I tried with NATIVE_CRLF = YesPlease but 117 failed too. First question, under what MINGW system do these tests pass? Second question any hints how to tackle this? The first failing test is not ok 117 - checkout core.eol= core.autocrlf=false gitattributes=auto file=LF # # compare_ws_file eol__crlf_false_attr_auto_ LF crlf_false_attr__LF.txt # where I have in the trash directory $ diff -Nur *expect* *actual* --- LF.expect 2014-10-02 12:15:17 + +++ eol__crlf_false_attr_auto_.actual.crlf_false_attr__LF.txt 2014-10-02 12:15 First things first: We have a file with LF in the repo, and check it out. Read it like this: eol__crlf_false_attr_auto_.actual ^ *.txt auto in .gitconfig ^ core.autocrlf is false ^ core.eol is unset The file is expected to have LF in the working tree, but has CRLF :17 + @@ -1,3 +1,3 @@ -000 l i n e 1 \n l i n e 2 \n l i n e -020 3 -021 +000 l i n e 1 \r \n l i n e 2 \r \n l i +020 n e 3 +023 Reading convert.h tells me that for undefined NATIVE_CRLF the native EOL is LF. Which looks like the test is correct. Thomas Which version of t0027 do you have: The latest version in git.git is this one, and should pass (but I may have missed something) commit f6975a6b119128de1c5a89e6cd64f75ed1de2177 Author: Torsten Bögershausen tbo...@web.de Date: Sat Aug 16 22:16:58 2014 +0200 t0027: Tests for core.eol=native, eol=lf, eol=crlf Add test cases for core.eol native and (unset). (MINGW uses CRLF, all other systems LF as native line endings) Add test cases for the attributes eol=lf and eol=crlf Other minor changes: - Use the more portable 'tr' instead of 'od -c' to convert '\n' into 'Q' and '\0' into 'N' - Style fixes for shell functions according to the coding guide lines - Replace txtbin with attr Signed-off-by: Torsten Bögershausen tbo...@web.de Signed-off-by: Junio C Hamano gits...@pobox.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Can I fetch an arbitrary commit by sha1?
I always though during fetch I have to specify a refspec and that a sha1 would not be accepted as a ref. Firing some like 'git fetch origin sha1' should be forbidden. But in fact I see that such a fetch command succeeds if you already have that object in your local repo. My question: is it allowed to fetch sha1's? Shouldn't fetch fail if you try it? See here: git clone -q https://github.com/chalstrick/dondalfi.git cd dondalfi git ls-remote From https://github.com/chalstrick/dondalfi.git ce08dcc41104383f3cca2b95bd41e9054a957f5b HEAD af00f4c39bcc8dc29ed8f59a47066d5993c279e4 refs/foo/b1 ... git show af00f4c39bcc8dc29ed8f59a47066d5993c279e4 fatal: bad object af00f4c39bcc8dc29ed8f59a47066d5993c279e4 git fetch origin af00f4c39bcc8dc29ed8f59a47066d5993c279e4 error: no such remote ref af00f4c39bcc8dc29ed8f59a47066d5993c279e4 git fetch origin refs/foo/b1 remote: Counting objects: 3, done. remote: Compressing objects: 100% (2/2), done. remote: Total 3 (delta 0), reused 0 (delta 0) Unpacking objects: 100% (3/3), done. From https://github.com/chalstrick/dondalfi * branchrefs/foo/b1 - FETCH_HEAD git fetch origin af00f4c39bcc8dc29ed8f59a47066d5993c279e4 From https://github.com/chalstrick/dondalfi * branchaf00f4c39bcc8dc29ed8f59a47066d5993c279e4 - FETCH_HEAD Ciao Chris -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Failing tests in t0027-autocrlf.sh under msysgit/git-win-sdk
Am 02.10.2014 um 15:42 schrieb Torsten Bögershausen: On 2014-10-02 14.39, Thomas Braun wrote: Hi, I've enabled EXPENSIVE and ran the git test suite under msysgit/git-win-sdk with git version 2.1.0.9753.g360f311.dirty. Now I have some failing tests in t0027-autocrlf.sh in the MINGW only section which puzzle me. The offending test sets are diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh index 72dd3e8..90c4cd1 100755 --- a/t/t0027-auto-crlf.sh +++ b/t/t0027-auto-crlf.sh @@ -245,18 +245,18 @@ if test_have_prereq MINGW then check_files_in_ws false LFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul check_files_in_ws true CRLF CRLF CRLF_mix_LF LF_mix_CRCRLF_nul -check_files_in_ws false autoLFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul +# check_files_in_ws false autoLFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul # first broken check_files_in_ws true autoCRLF CRLF CRLF LF_mix_CRCRLF_nul -check_files_in_ws false textLFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul +# check_files_in_ws false textLFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul # broken check_files_in_ws true textCRLF CRLF CRLF CRLF_mix_CR CRLF_nul check_files_in_ws false -text LFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul check_files_in_ws true -text LFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul check_files_in_ws native false LFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul check_files_in_ws native true CRLF CRLF CRLF_mix_LF LF_mix_CRCRLF_nul -check_files_in_ws native false autoLFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul +# check_files_in_ws native false autoLFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul # broken check_files_in_ws native true autoCRLF CRLF CRLF LF_mix_CRCRLF_nul -check_files_in_ws native false textLFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul +# check_files_in_ws native false textLFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul # broken check_files_in_ws native true textCRLF CRLF CRLF CRLF_mix_CR CRLF_nul check_files_in_ws native false -text LFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul check_files_in_ws native true -text LFCRLF CRLF_mix_LF LF_mix_CRCRLF_nul I tried with NATIVE_CRLF = YesPlease but 117 failed too. First question, under what MINGW system do these tests pass? Second question any hints how to tackle this? The first failing test is not ok 117 - checkout core.eol= core.autocrlf=false gitattributes=auto file=LF # # compare_ws_file eol__crlf_false_attr_auto_ LF crlf_false_attr__LF.txt # where I have in the trash directory $ diff -Nur *expect* *actual* --- LF.expect 2014-10-02 12:15:17 + +++ eol__crlf_false_attr_auto_.actual.crlf_false_attr__LF.txt 2014-10-02 12:15 First things first: We have a file with LF in the repo, and check it out. Read it like this: eol__crlf_false_attr_auto_.actual ^ *.txt auto in .gitconfig ^ core.autocrlf is false ^ core.eol is unset Thanks for the explanation. The file is expected to have LF in the working tree, but has CRLF :17 + @@ -1,3 +1,3 @@ -000 l i n e 1 \n l i n e 2 \n l i n e -020 3 -021 +000 l i n e 1 \r \n l i n e 2 \r \n l i +020 n e 3 +023 Reading convert.h tells me that for undefined NATIVE_CRLF the native EOL is LF. Which looks like the test is correct. Thomas Which version of t0027 do you have: The latest version in git.git is this one, and should pass (but I may have missed something) commit f6975a6b119128de1c5a89e6cd64f75ed1de2177 Author: Torsten Bögershausen tbo...@web.de Date: Sat Aug 16 22:16:58 2014 +0200 t0027: Tests for core.eol=native, eol=lf, eol=crlf Add test cases for core.eol native and (unset). (MINGW uses CRLF, all other systems LF as native line endings) Add test cases for the attributes eol=lf and eol=crlf Other minor changes: - Use the more portable 'tr' instead of 'od -c' to convert '\n' into 'Q' and '\0' into 'N' - Style fixes for shell functions according to the coding guide lines - Replace txtbin with attr Signed-off-by: Torsten Bögershausen tbo...@web.de Signed-off-by: Junio C Hamano gits...@pobox.com Correct guess! I've been testing the original version 343151dc (t0027: combinations of core.autocrlf, core.eol and text, 2014-07-08). f6975a6 of t0027-autocrlf.sh passes now completely. Thanks for the quick reply. Thomas -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Can I fetch an arbitrary commit by sha1?
On Thu, Oct 2, 2014 at 9:57 AM, Christian Halstrick christian.halstr...@gmail.com wrote: I always though during fetch I have to specify a refspec and that a sha1 would not be accepted as a ref. Firing some like 'git fetch origin sha1' should be forbidden. But in fact I see that such a fetch command succeeds if you already have that object in your local repo. My question: is it allowed to fetch sha1's? Shouldn't fetch fail if you try it? See here: git clone -q https://github.com/chalstrick/dondalfi.git cd dondalfi git ls-remote From https://github.com/chalstrick/dondalfi.git ce08dcc41104383f3cca2b95bd41e9054a957f5b HEAD af00f4c39bcc8dc29ed8f59a47066d5993c279e4 refs/foo/b1 ... git show af00f4c39bcc8dc29ed8f59a47066d5993c279e4 fatal: bad object af00f4c39bcc8dc29ed8f59a47066d5993c279e4 git fetch origin af00f4c39bcc8dc29ed8f59a47066d5993c279e4 error: no such remote ref af00f4c39bcc8dc29ed8f59a47066d5993c279e4 git fetch origin refs/foo/b1 remote: Counting objects: 3, done. remote: Compressing objects: 100% (2/2), done. remote: Total 3 (delta 0), reused 0 (delta 0) Unpacking objects: 100% (3/3), done. From https://github.com/chalstrick/dondalfi * branchrefs/foo/b1 - FETCH_HEAD git fetch origin af00f4c39bcc8dc29ed8f59a47066d5993c279e4 From https://github.com/chalstrick/dondalfi * branchaf00f4c39bcc8dc29ed8f59a47066d5993c279e4 - FETCH_HEAD My understanding is that you are allowed to ask for a SHA1, but most git servers refuse the request. But if you already have the SHA locally, then git doesn't neet to bother asking the server for it, so there's no request to be refused. But it's been a while for me since I did any git development, so it's possible I missed something. -Dan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: $GIT_CONFIG should either apply to all commands, or none at all
On Wed, Oct 01, 2014 at 06:15:46PM -0700, Jonathan Nieder wrote: 3) Warn when 'git config' is called with GIT_CONFIG set, explaining that support will eventually be removed and that callers should pass --file= instead. 4) Once we're confident there are no scripts in the wild relying on that envvar, remove support for it. I think you could do just these two without worrying about the I_AM_PORCELAIN setting. It's completely redundant with `git config --file` these days. Another possibility (B): 1) Teach git's commands in C to respect the GIT_CONFIG environment variable. Semantics: only configuration from that file would be respected and all other configuration will be ignored. Advertise it in the git(1) manpage. I think this is a bad idea. It originally _did_ impact each command, but there were a lot of confusing corner cases to the semantics, and it led to bugs and misbehavior. That's what led to dc87183. I wish we had just dropped it for git-config then, too. We kept it for backwards compatibility, but we probably should have deprecated it more clearly. Yet another possibility (C): 1) Just skip to step (4) from plan (A). I agree this is tempting. We have never deprecated it formally, but it has been a little-used feature. C is kind of temping. Do you know if there are scripts in the wild that rely on the GIT_CONFIG setting working? Searching here: https://github.com/search?q=%22export+GIT_CONFIG%22type=Code reveals that some people do set it, but from the handful I investigated, it is probably not doing what they want. For example, in: https://github.com/GNOME/sysadmin-bin/blob/8ef4165a4b38fd1488c194f0c562c7fe24545bca/git/gnome-post-receive they are trying to use it as if it affects all git commands, but as we just discussed, it does not. So their script is potentially buggy as-is. Getting rid of GIT_CONFIG would make it _more_ buggy, so perhaps that is not an excuse, but I think it points to actually doing something. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Can I fetch an arbitrary commit by sha1?
On Thu, Oct 02, 2014 at 10:22:50AM -0400, Dan Johnson wrote: git show af00f4c39bcc8dc29ed8f59a47066d5993c279e4 fatal: bad object af00f4c39bcc8dc29ed8f59a47066d5993c279e4 git fetch origin af00f4c39bcc8dc29ed8f59a47066d5993c279e4 error: no such remote ref af00f4c39bcc8dc29ed8f59a47066d5993c279e4 git fetch origin refs/foo/b1 remote: Counting objects: 3, done. remote: Compressing objects: 100% (2/2), done. remote: Total 3 (delta 0), reused 0 (delta 0) Unpacking objects: 100% (3/3), done. From https://github.com/chalstrick/dondalfi * branchrefs/foo/b1 - FETCH_HEAD git fetch origin af00f4c39bcc8dc29ed8f59a47066d5993c279e4 From https://github.com/chalstrick/dondalfi * branchaf00f4c39bcc8dc29ed8f59a47066d5993c279e4 - FETCH_HEAD My understanding is that you are allowed to ask for a SHA1, but most git servers refuse the request. But if you already have the SHA locally, then git doesn't neet to bother asking the server for it, so there's no request to be refused. That's right. It is the server which enforces the you cannot fetch an arbitrary sha1 rule. But I think Christian is arguing that the client side should complain that $sha1 is not a remote ref, and therefore not something we can fetch. This used to be the behavior until 6e7b66e (fetch: fetch objects by their exact SHA-1 object names, 2013-01-29). The idea there is that some refs may be kept hidden from the ref advertisement, but clients who learn about the sha1 out-of-band may fetch the tips of hidden refs. I'm not sure it is a feature that has been particularly well-used to date, though. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] init - Honour the global core.filemode setting
Torsten Bögershausen tbo...@web.de writes: On 2014-10-01 19.10, Junio C Hamano wrote: Hilco Wijbenga hilco.wijbe...@gmail.com writes: Perhaps I completely misunderstand the meaning of core.filemode but I thought it determined whether Git cared about changes in file properties? By setting it to false, you tell Git that the filesystem you placed the repository does not correctly represent the filemode (especially the executable bit). core.fileMode in git config --help reads: core.fileMode If false, the executable bit differences between the index and the working tree are ignored; useful on broken filesystems like FAT. See git-update- index(1). Out of my head: Could the following be a starting point: core.fileMode If false, the executable bit differences between the index and the working tree are ignored. This may be usefull when visiting a cygwin repo with a non-cygwin Git client. (should we mention msysgit ? should we mention JGit/EGit ?) Between these two sentences, there may still be the same cognitive gap that may have lead to the original confusion. The first sentence says what happens, as it should. But it is not directly clear what makes the executable bit differ and when it is a useful thing to ignore the differences, so the second sentence that says This may be useful does not give the reader very much. Here is my attempt. Tells Git if the executable bit of files in the working tree is to be honored. Some filesystems lose the executable bit when a file that is marked as executable is checked out, or checks out an non-executable file with executable bit on. git init and git clone probe the filesystem to see if it records executable bit correctly when they create a new repository and this variable is automatically set as necessary. A repository, however, may be on a filesystem that records the filemode correctly, and this variable is set to 'true' when created, but later may be made accessible from another environment that loses the filemode (e.g. exporting ext4 via CIFS mount, visiting a Cygwin managed repository with MsysGit). In such a case, it may be necessary to set this to 'false'. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Can I fetch an arbitrary commit by sha1?
Jeff King wrote: But I think Christian is arguing that the client side should complain that $sha1 is not a remote ref, and therefore not something we can fetch. This used to be the behavior until 6e7b66e (fetch: fetch objects by their exact SHA-1 object names, 2013-01-29). The idea there is that some refs may be kept hidden from the ref advertisement, but clients who learn about the sha1 out-of-band may fetch the tips of hidden refs. I'm not sure it is a feature that has been particularly well-used to date, though. I use it pretty often. The commits I'm fetching are pointed to directly by refs, but I don't care about what the ref is called and I want exactly that commit. The context is that the commit is mentioned in the gerrit web UI. Fetching by commit name feels simpler than getting the refs/changes/something ref, since I think in terms of commits instead of in terms of change numbers. Thanks and hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Sep 2014, #09; Tue, 30)
David Aguilar dav...@gmail.com writes: On Tue, Sep 30, 2014 at 01:23:18PM -0700, Junio C Hamano wrote: Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. * da/include-compat-util-first-in-c (2014-09-15) 4 commits - SQUASH??? - check-headers: add header usage checks for .c files - Makefile: add check-headers target - cleanups: ensure that git-compat-util.h is included first So... what is happening to this topic? I think the bottom one is a reasonable clean-up without too much churn, but I am not sure about the rest. Agreed. The SQUASH??? commit could be squashed in, but it might just be better to take the bottom commit and drop the rest since they aren't really that helpful. OK, let's do that. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: $GIT_CONFIG should either apply to all commands, or none at all
Jonathan Nieder jrnie...@gmail.com writes: Yep. One possibility would be to do something like the following (A): 1) advertise in the git-config(1) manpage that the GIT_CONFIG environment variable only affects the behavior of the 'git config' command 2) introduce an environment variable GIT_I_AM_PORCELAIN. (If doing this, we could come up with a better name, but this is just an illustration.) Set and export that envvar in git-sh-setup.sh. When that environment variable is set, make git-config stop paying attention to GIT_CONFIG. That way, git commands that happen to be scripts would not be affected by the GIT_CONFIG setting any more. At the places you plan to update porcelains to set and export GIT_I_AM_PORCELAIN, you could unset GIT_CONFIG if set. Would that achieve the same goal? And you can stop there without doing 3 or 4, no? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 19/24] refs.c: allow listing and deleting badly named refs
Jonathan Nieder jrnie...@gmail.com writes:
From: Ronnie Sahlberg sahlb...@google.com
...
In resolving functions, refuse to resolve refs that don't pass the
check-ref-format(1) check unless the new RESOLVE_REF_ALLOW_BAD_NAME
flag is passed. Even with RESOLVE_REF_ALLOW_BAD_NAME, refuse to
resolve refs that escape the refs/ directory and do not match the
pattern [A-Z_]* (think HEAD and MERGE_HEAD).
In locking functions, refuse to act on badly named refs unless they
are being deleted and either are in the refs/ directory or match [A-Z_]*.
Just like other invalid refs, flag resolved, badly named refs with the
REF_ISBROKEN flag, treat them as resolving to null_sha1, and skip them
in all iteration functions except for for_each_rawref.
Flag badly named refs with a REF_BAD_NAME flag to make it easier for
future callers to notice and handle them specially.
In the transaction API, refuse to create or update badly named refs,
but allow deleting them (unless they escape refs/ and don't match
[A-Z_]*).
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
Thanks. We originally threw all the different kind of breakages
into ISBROKEN, but a ref can have a malformed name or can contain a
bad/non value and allowing us to tell them apart is a good direction
to go.
diff --git a/cache.h b/cache.h
index 5ca7f2b..0c0ac60 100644
--- a/cache.h
+++ b/cache.h
@@ -978,16 +978,26 @@ extern int read_ref(const char *refname, unsigned char
*sha1);
* If flags is non-NULL, set the value that it points to the
* combination of REF_ISPACKED (if the reference was found among the
* packed references), REF_ISSYMREF (if the initial reference was a
- * symbolic reference) and REF_ISBROKEN (if the ref is malformed).
+ * symbolic reference), REF_BAD_NAME (if the reference name is ill
+ * formed --- see RESOLVE_REF_ALLOW_BAD_NAME below), and REF_ISBROKEN
+ * (if the ref is malformed).
You want to define is malformed here.
The original defines REF_ISBROKEN as malformed because
(1) resolve_ref_unsafe() uses get_sha1_hex() and read_loose_refs()
uses read_ref_full(), both to catch malformed values stored;
(2) resolve_ref_unsafe() uses check_refname_format() and catches
malformed names stored as a symref target.
I _think_ you are introducing ALLOW_BAD_NAME to allow add the third
class .git/refs/remotes/origin/mal..formed..name. I do not know
if they should be the same class as a symref with a good name
.git/refs/remotes/origin/HEAD that points at a bad name
mal..formed..name, which is (2) above). Perhaps not. (2) is
still above what is stored in the ref, and the ref in question may
or may not have a well-formed name, which is orthogonal.
So probably you left only the value stored in the ref is malformed
(in other words, we expected to find 40-hex object name but didn't
find one) case for REF_ISBROKEN?
Do we want to separate value is not 40-hex and a symref points at
a malformed refname as separate malformed value errors?
+ * RESOLVE_REF_ALLOW_BAD_NAME allows resolving refs even when their
+ * name is invalid according to git-check-ref-format(1). If the name
+ * is bad then the value stored in sha1 will be null_sha1 and the
+ * REF_ISBROKEN and REF_BAD_NAME flags will be set.
Viewed with that light, I am not sure if a badly named ref should
yield null_sha1[] (REF_ISBROKEN, which I am assuming is about a
value that is badly formatted and cannot be read, should keep
yielding it as before). Wouldn't it make it harder for the user if
you give null_sha1[] back to somebody who is trying to recover by
reading refs/heads/mal..formed, creating refs/heads/sensible to
point at the same value and then removing the former?
Note that I am not saying we should give back the parsed value at
this step in the series. Perhaps there are some existing callers
that do not check for ISBROKEN flag and instead says null_sha1[]
ref is to be rejected/ignored, in which case they may need to be
corrected before that happens. Or there may be some reason I
overlooked that makes it not so useful if we returned the object
name stored in a ref whose name is malformed. Just wondering.
@@ -272,6 +272,37 @@ static struct ref_dir *get_ref_dir(struct ref_entry
*entry)
return dir;
}
+static int escapes_cwd(const char *path) {
+ char *buf;
+ int result;
+
+ if (is_absolute_path(path))
+ return 1;
+ buf = xmalloc(strlen(path) + 1);
+ result = !!normalize_path_copy(buf, path);
+ free(buf);
+ return result;
+}
I think this function is misnamed for two reasons.
- It does not have anything to do with cwd; it does not make any
difference to the outcome of this function given the same input,
if 'pwd' says /u/jc/git or /u/jc/git/Documentation, no?
- Even if this had something to do with cwd, I would expect a
function whose name is escapes_cwd(/u/jc/git/Documentation) to
yield false when 'pwd'
Re: [PATCH 09/24] refs.c: pass a list of names to skip to is_refname_available
Jonathan Nieder jrnie...@gmail.com writes:
diff --git a/refs.c b/refs.c
index f124c2b..6820c93 100644
--- a/refs.c
+++ b/refs.c
@@ -801,14 +801,16 @@ static int names_conflict(const char *refname1, const
char *refname2)
struct name_conflict_cb {
const char *refname;
- const char *oldrefname;
const char *conflicting_refname;
+ struct string_list *skiplist;
};
As cbe73331 (refs: speed up is_refname_available, 2014-09-10)
touches the same area and is now in 'master', the logic around here
in this series needs to be reworked.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/5] add unset.variable for unsetting previously set variables
Tanay Abhra tanay...@gmail.com writes: (just this point quick) 1 The name of the variable, I could not decide between unset.variable and config.unset, or may be some other name would be more appropriate. I'd prefer to see this as [config] something. I wish we did the include as [config] include = path/to/filename, not as [include] path = path/to/filename. Perhaps we can deprecate and move it over time? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/5] add unset.variable for unsetting previously set variables
Tanay Abhra tanay...@gmail.com writes: 2 It affects both the C git_config() calls and, git config shell invocations. Due to this some variables may be absent from the git config -l result which might confuse the user. I am not sure what you mean by this. If you process variable definitions as they come, and you read [some] variable = set to some value initially ... [unset] variable = some.variable ... [some] variable = set to some other value then a user might be able to see $ git config -l some.variable=set to some value initially !some.variable some.variable=set to some other value (here, I am using an imaginary !variable.name to denote this variable is unset at this point in the reading sequence). I would imagine if the result comes from a caching layer, the user would see $ git config -l some.variable=set to some other value and nothing else. Is that what you are referring to? I would think that the latter is probably desirable; otherwise we would need to come up with a way to say forget about everything we said about this variable so far (i.e. my !some.variable above) and also the scripts that parse git config -l output need to code the logic to forget and start afresh. 3 I also have an another implementation for this series which just marks the config variables instead of deleting them from the configset. This can be used to provide two versions of git_config(), one with filtered variables other without it. I do not see a value in the filtered version. What worries me _more_ is why people may want to put things in system-wide global, and if it is a wise thing to do to allow users to override. We may later want to add ways to mark variables in various ways, e.g. (thinking aloud) - [config] sealed = section.variable will prevent the variable from being reset, modified or appended. If an administrator wants to enforce a certain setting in /etc/gitconfig, she may mark sensitive variables as cofnig.sealed at the end of the file: [security] enforced = true ... [config] sealed = security.enforced and we would ignore [config] unset = security.enforce [security] enforce = false written in .git/config or ~/.gitconfig, perhaps? - [config] safeInclude = path will allow a configuration file to be included safely from the project's working tree. The path given as the value must be a relative path and it is relative to the top level of the project's working tree. - [config] safe = section.variable will list variables that can be included with the config.safeInclude mechanism. Any variable that is not marked as config.safe that appears in the file included by the config.safeInclude mechanism will be ignored. The 'frotz' project might have a .frotz-gitconfig file at the root level of its working tree that stores this: [diff] renames = true and your .git/config may have [config] safe = diff.renames safeInclude = .frotz-gitconfig You will not have to worry about a malicious participant committing a [diff] external = rm -fr . to .frotz-gitconfig and pushing it to the project because you do not mark diff.external as config.safe in your .git/config -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 5/5] add tests for checking the behaviour of unset.variable
Tanay Abhra tanay...@gmail.com writes: +test_expect_success 'document how unset.variable will behave in shell scripts' ' + rm -f .git/config + cat expect -\EOF + EOF + git config foo.bar boz1 + git config --add foo.bar boz2 + git config unset.variable foo.bar + git config --add foo.bar boz3 + test_must_fail git config --get-all foo.bar actual You make foo.bar a multi-valued one, then you unset it, so I would imagine that the value given after that, 'boz3', would be the only value foo.bar has. Why should --get-all fail? I am having a hard time imagining how this behaviour can make any sense. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 5/5] add tests for checking the behaviour of unset.variable
On 10/3/2014 1:39 AM, Junio C Hamano wrote: Tanay Abhra tanay...@gmail.com writes: +test_expect_success 'document how unset.variable will behave in shell scripts' ' +rm -f .git/config +cat expect -\EOF +EOF +git config foo.bar boz1 +git config --add foo.bar boz2 +git config unset.variable foo.bar +git config --add foo.bar boz3 +test_must_fail git config --get-all foo.bar actual You make foo.bar a multi-valued one, then you unset it, so I would imagine that the value given after that, 'boz3', would be the only value foo.bar has. Why should --get-all fail? I am having a hard time imagining how this behaviour can make any sense. git config -add appends the value to a existing header, after these two commands have executed the config file would look like, git config foo.bar boz1 git config --add foo.bar boz2 [foo] bar = boz1 bar = boz2 After git config unset.variable foo.bar, [foo] bar = boz1 bar = boz2 [unset] variable = foo.bar Now the tricky part, git config --add foo.bar boz3 append to the existing header, [foo] bar = boz1 bar = boz2 bar = boz3 [unset] variable = foo.bar Since unset.variable unsets all previous set values in parsing order, git config --get-all foo.bar gives us nothing in result. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 5/5] add tests for checking the behaviour of unset.variable
Tanay Abhra tanay...@gmail.com writes: On 10/3/2014 1:39 AM, Junio C Hamano wrote: Tanay Abhra tanay...@gmail.com writes: +test_expect_success 'document how unset.variable will behave in shell scripts' ' + rm -f .git/config + cat expect -\EOF + EOF + git config foo.bar boz1 + git config --add foo.bar boz2 + git config unset.variable foo.bar + git config --add foo.bar boz3 + test_must_fail git config --get-all foo.bar actual You make foo.bar a multi-valued one, then you unset it, so I would imagine that the value given after that, 'boz3', would be the only value foo.bar has. Why should --get-all fail? I am having a hard time imagining how this behaviour can make any sense. git config -add appends the value to a existing header, after these two commands have executed the config file would look like, ... I *know* how it is implemented before the changes of this series. And if the original implementation of add is left as-is, I can imagine how such a behaviour that is unintuitive to end-users can arise. I was and am having a hard time how this behaviour can make any sense from an end-user's point of view. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 5/5] add tests for checking the behaviour of unset.variable
On 10/3/2014 1:53 AM, Junio C Hamano wrote: Tanay Abhra tanay...@gmail.com writes: On 10/3/2014 1:39 AM, Junio C Hamano wrote: Tanay Abhra tanay...@gmail.com writes: +test_expect_success 'document how unset.variable will behave in shell scripts' ' + rm -f .git/config + cat expect -\EOF + EOF + git config foo.bar boz1 + git config --add foo.bar boz2 + git config unset.variable foo.bar + git config --add foo.bar boz3 + test_must_fail git config --get-all foo.bar actual You make foo.bar a multi-valued one, then you unset it, so I would imagine that the value given after that, 'boz3', would be the only value foo.bar has. Why should --get-all fail? I am having a hard time imagining how this behaviour can make any sense. git config -add appends the value to a existing header, after these two commands have executed the config file would look like, ... I *know* how it is implemented before the changes of this series. And if the original implementation of add is left as-is, I can imagine how such a behaviour that is unintuitive to end-users can arise. I was and am having a hard time how this behaviour can make any sense from an end-user's point of view. That's what I was trying to document. I think that it makes no sense to use the feature as I had shown above. I had envisioned unset.variable to be explicitly typed in on the appropriate place as can be seen in the first two tests but Matthieu had suggested to add tests using git config too. This is when I had discovered these inconsistencies. I can think of two solutions, one leave it as it is and advertise it to be explicitly typed in the config files at the appropriate position or to change the behavior of unset.variable to unset all matching variables in that file, before and after. We could also change git config --add to append at the end of the file regardless the variable exists or not. Which course of action do you think would be best? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 0/5] add unset.variable for unsetting previously set variables
On Thu, Oct 02, 2014 at 12:29:14PM -0700, Junio C Hamano wrote: Tanay Abhra tanay...@gmail.com writes: (just this point quick) 1 The name of the variable, I could not decide between unset.variable and config.unset, or may be some other name would be more appropriate. I'd prefer to see this as [config] something. I wish we did the include as [config] include = path/to/filename, not as [include] path = path/to/filename. Perhaps we can deprecate and move it over time? I chose [include] because I had intended there to be multiple include variables (include.path, include.ref, etc). The others were shot down for now. If we put it under [config], I'd still prefer to leave room by calling it: [config] includePath = path/to/filename I also wanted [include] as a section name to leave room for conditional includes. We've sometimes discussed things like: [include has-some-git-feature] path = ... to allow conditional inclusion only when git supports a certain feature-set (so that your config doesn't cause git to blow up when you use an old version of git). It's possible that I'm the only person in the world who really wants that, because I run old git versions all the time for testing and debugging. And it is kind of gross as a syntax. But it would still be nice to leave room for it. I don't think there's a reason we couldn't allow: [config condition...] includePath = ... in the same way if we wanted to (though aside from includes, I do not know of any other feature that would want the condition). So I'm not _opposed_ to adding [config], deprecating [include], and waiting a bit before dropping [include]. But I also don't really see the current name as a particularly bad thing. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC 5/5] add tests for checking the behaviour of unset.variable
Tanay Abhra tanay...@gmail.com writes: I can think of two solutions, one leave it as it is and advertise it to be explicitly typed in the config files at the appropriate position or to change the behavior of unset.variable to unset all matching variables in that file, before and after. We could also change git config --add to append at the end of the file regardless the variable exists or not. Which course of action do you think would be best? Off the top of my head, from an end-user's point of view, something like this would give a behaviour that is at least understandable: (1) forbid git config command line from touching unset.var, as there is no way for a user to control where a new unset.var goes. And (2) When adding or appending section.var (it may also apply to removing one--you need to think about it deeper), ignore everything that comes before the last appearance of unset.var that unsets the section.var variable. That way, if you do not have [section] after [unset] variable = section.var, you would end up adding a new [section] var = value, and if you already have [section], you would add a var = value in that existing [section] that appears after the last unset of the variable, so eerything will be kept neat. Alternatively, if the syntax to unset a section.var were not [unset] variable = section.var but rather [section] ! variable or soemthing, then the current find the section and append at the end code may work as-is. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 0/2] Introduce safe-include config feature
[trimming Ccs] This is an attempt at implementing the suggested safe-include config feature. It mostly has the semantics Junio suggested in the parent post, but it does not directly extend the current include directive; instead, it uses a separate safe-include directive. This is done so that if a repository is used with both old and new versions of git, the older versions will just silently ignore the safe-include, instead of ignoring include.safe and then proceeding to processing path = ../project.gitconfig. Config variables are whitelisted using safe-include.whitelist; the value is interpreted as a whitespace-separated list of, possibly negated, patterns. Later patterns override earlier ones. If the feature is deemed worthwhile and my approach is acceptable, I'll go ahead and try to write some documentation. For now, there is just a small test script. Rasmus Villemoes (2): config: Add safe-include directive config: Add test of safe-include feature config.c | 91 +-- t/t1309-config-safe-include.sh | 96 ++ 2 files changed, 184 insertions(+), 3 deletions(-) create mode 100755 t/t1309-config-safe-include.sh -- 2.0.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 1/2] config: Add safe-include directive
This adds a variant of the include directive, where only certain
config variables in the included files are honoured. The set of
honoured variables consists of those the user has mentioned in a
safe-include.whitelist directive, along with a small set of git.git
blessed ones.
This can, for example, be used by a project to supply a set of
suggested configuration variables, such as diff.renames = true. The
project would provide these in e.g project.gitconfig, and the user then
has to explicitly opt-in by putting
[safe-include]
path = ../project.gitconfig
into .git/config, possibly preceding the path directive with a
whitelist directive.
The problem with simply using the ordinary include directive for this
purpose is that certain configuration variables (e.g. diff.external)
can allow arbitrary programs to be run.
Older versions of git do not understand the safe-include directives,
so they will effectively just ignore them.
Obviously, we must ignore safe-include.whitelist directives when we
are processing a safe-included file.
Signed-off-by: Rasmus Villemoes r...@rasmusvillemoes.dk
---
config.c | 91 +---
1 file changed, 88 insertions(+), 3 deletions(-)
diff --git a/config.c b/config.c
index a677eb6..764cda1 100644
--- a/config.c
+++ b/config.c
@@ -11,6 +11,7 @@
#include quote.h
#include hashmap.h
#include string-list.h
+#include wildmatch.h
struct config_source {
struct config_source *prev;
@@ -39,6 +40,79 @@ static struct config_source *cf;
static int zlib_compression_seen;
+struct safe_var {
+ struct safe_var *next;
+ const char *pattern;
+ int blacklisted;
+};
+
+static int safe_include_depth;
+static struct safe_var *safe_var_head;
+
+static const char *builtin_safe_patterns[] = {
+ diff.renames,
+};
+
+static int config_name_is_safe(const char *var)
+{
+ struct safe_var *sv;
+ unsigned i;
+
+ for (sv = safe_var_head; sv; sv = sv-next) {
+ /* Handle malformed patterns? */
+ if (wildmatch(sv-pattern, var, WM_CASEFOLD, NULL) == WM_MATCH)
+ return !sv-blacklisted;
+ }
+ for (i = 0; i ARRAY_SIZE(builtin_safe_patterns); ++i) {
+ if (wildmatch(builtin_safe_patterns[i], var, WM_CASEFOLD, NULL)
== WM_MATCH)
+ return 1;
+ }
+
+ return 0;
+}
+
+static void config_add_safe_pattern(const char *p)
+{
+ struct safe_var *sv;
+ int blacklist = 0;
+
+ if (*p == '!') {
+ blacklist = 1;
+ ++p;
+ }
+ if (!*p)
+ return;
+ sv = xmalloc(sizeof(*sv));
+ sv-pattern = xstrdup(p);
+ sv-blacklisted = blacklist;
+ sv-next = safe_var_head;
+ safe_var_head = sv;
+}
+
+static void config_add_safe_names(const char *value)
+{
+ char *patterns = xstrdup(value);
+ char *p, *save;
+
+ /*
+* This allows giving multiple patterns in a single line, e.g.
+*
+* whitelist = !* foo.bar squirrel.*
+*
+* to override the builtin list of safe vars and only declare
+* foo.bar and the squirrel section safe. But it has the
+* obvious drawback that one cannot match subsection names
+* containing whitespace. The alternative is that the above
+* would have to be written on three separate whitelist lines.
+*/
+ for (p = strtok_r(patterns, \t, save); p; p = strtok_r(NULL, \t,
save)) {
+ config_add_safe_pattern(p);
+ }
+
+ free(patterns);
+}
+
+
/*
* Default config_set that contains key-value pairs from the usual set of
config
* config files (i.e repo specific .git/config, user wide ~/.gitconfig, XDG
@@ -142,12 +216,23 @@ int git_config_include(const char *var, const char
*value, void *data)
* Pass along all values, including include directives; this makes it
* possible to query information on the includes themselves.
*/
- ret = inc-fn(var, value, inc-data);
- if (ret 0)
- return ret;
+ if (safe_include_depth == 0 || config_name_is_safe(var)) {
+ ret = inc-fn(var, value, inc-data);
+ if (ret 0)
+ return ret;
+ }
if (!strcmp(var, include.path))
ret = handle_path_include(value, inc);
+ else if (safe_include_depth == 0
+ !strcmp(var, safe-include.whitelist)) {
+ config_add_safe_names(value);
+ }
+ else if (!strcmp(var, safe-include.path)) {
+ safe_include_depth++;
+ ret = handle_path_include(value, inc);
+ safe_include_depth--;
+ }
return ret;
}
--
2.0.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 2/2] config: Add test of safe-include feature
This adds a script for testing various aspects of the safe-include feature. Signed-off-by: Rasmus Villemoes r...@rasmusvillemoes.dk --- t/t1309-config-safe-include.sh | 96 ++ 1 file changed, 96 insertions(+) create mode 100755 t/t1309-config-safe-include.sh diff --git a/t/t1309-config-safe-include.sh b/t/t1309-config-safe-include.sh new file mode 100755 index 000..b8ccc94 --- /dev/null +++ b/t/t1309-config-safe-include.sh @@ -0,0 +1,96 @@ +#!/bin/sh + +test_description='test config file safe-include directives' +. ./test-lib.sh + + +test_expect_success 'blacklist by default' ' + echo [diff]external = badprog project + echo [safe-include]path = project .gitconfig + test_must_fail git config diff.external +' + + +test_expect_success 'builtin safe rules' ' + echo [diff]renames = true project + echo [safe-include]path = project .gitconfig + echo true expect + git config diff.renames actual + test_cmp expect actual +' + +test_expect_success 'user blacklist taking precedence' ' + echo [diff]renames = true project + cat .gitconfig -\EOF + [diff]renames = false + [safe-include]whitelist = !diff.renames + [safe-include]path = project + EOF + echo false expect + git config diff.renames actual + test_cmp expect actual +' + +test_expect_success 'wildcard matching' ' + cat project -\EOF + [test]beer = true + [test]bar = true + [test]foo = true + EOF + cat .gitconfig -\EOF + [safe-include]whitelist = test.b*r + [safe-include]path = project + EOF + printf test.bar true\ntest.beer true\n | sort expect + git config --get-regexp ^test | sort actual + test_cmp expect actual +' + +test_expect_success 'ignore whitelist directives in safe-included files' ' + cat project -\EOF + [safe-include]whitelist = * + [diff]external = badprog + EOF + echo [safe-include]path = project .gitconfig + test_must_fail git config diff.external +' + +test_expect_success 'multiple whitelist/blacklist patterns in one line' ' + cat .gitconfig -\EOF + [safe-include]whitelist = !* foo.bar squirrel.* !squirrel.xyz + [safe-include]path = project + EOF + cat project -\EOF + [diff]renames = true + [foo]bar = bar + [squirrel]abc = abc + [squirrel]xyz = xyz + EOF + test_must_fail git config diff.renames + test_must_fail git config squirrel.xyz + echo bar expect + git config foo.bar actual + test_cmp expect actual + echo abc expect + git config squirrel.abc actual + test_cmp expect actual +' + +test_expect_success 'case insensitivity' ' + cat .gitconfig -\EOF + [safe-include]whitelist = Test.Abc test.xyz + [safe-include]path = project + EOF + cat project -\EOF + [test]abc = abc + [TeST]XyZ = XyZ + EOF + echo abc expect + git config test.abc actual + test_cmp expect actual + echo XyZ expect + git config test.xyz actual + test_cmp expect actual +' + +test_done -- 2.0.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[TOY PATCH]: rebase: Add --show-files option
Hi,
When working on a new feature branch that touches a lot of files I
tend to make commits that affect only single files, and for very small
changes. Since at this stage I'm experimentating a lot - trying out
ideas, etc. - the commits tend to grow a lot (could be 50-70
individual commits, each modifying one or two files), and I don't
think much about the commit message beside making a one-liner that
explains only the gist.
Most of the times I include the filename in the commit message to help
me identify which commits should be squashed together later.
Only when the feature seems to be functional that I git rebase the
commits in order to shape the history into its final, proper form.
When rebasing these upwards of 40+ commits, it is helpful if the
rebase instruction sheet shows me the actual files that the commits
affect so I made this patch (sorry I couldn't attach it inline since
gmail eats all the tabs) that adds the --show-files option to
git-rebase to achieve something to this effect:
pick 996fa59 Remove autoconf submodule
# :100644 100644 cfc8a25... 28ddb02... M .gitmodules
# :16 00 0263a9f... 000... D autoconf
... more pick lines
pick 4c5070f Remove automake submodule
# :100644 100644 28ddb02... f907328... M .gitmodules
# :16 00 9042530... 000... D automake
Having the list of files shown below each commit, indented to reduce
cluttering the pick instruction, really does help in deciding the
reorder and squash candidates.
The files list came from this:
git show --raw $sha1|awk '/^:/ {print '${comment_char}'\t$0}'
Thoughts?
nazri
From 4826875c14554d4fa5098ddf9499c33cb7b9001b Mon Sep 17 00:00:00 2001
From: Nazri Ramliy ayieh...@gmail.com
Date: Fri, 3 Oct 2014 09:59:38 +0800
Subject: [PATCH] rebase: Add --show-files option
---
Documentation/git-rebase.txt | 8
git-rebase--interactive.sh | 13 +
git-rebase.sh| 5 +
3 files changed, 26 insertions(+)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index f14100a..4996bc4 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -383,6 +383,14 @@ If `--autosquash` is used, exec lines will not be appended for
the intermediate commits, and will only appear at the end of each
squash/fixup series.
+-F::
+--show-files::
+ Append the list of affected files after each line creating a commit in
+ the history.
++
+This option can only be used with the `--interactive` option
+(see INTERACTIVE MODE below).
+
--root::
Rebase all commits reachable from branch, instead of
limiting them with an upstream. This allows you to rebase
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b64dd28..32b4266 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -820,6 +820,11 @@ add_exec_commands () {
mv $1.new $1
}
+print_affected_files () {
+ commit_sha1=$1
+ git show --raw $commit_sha1|awk '/^:/ {print '${comment_char}' $0}'
+}
+
# The whole contents of this file is run by dot-sourcing it from
# inside a shell function. It used to be that returns we see
# below were not inside any function, and expected to return
@@ -978,6 +983,10 @@ do
if test t != $preserve_merges
then
printf '%s\n' ${comment_out}pick $shortsha1 $rest $todo
+ if test -n $show_files
+ then
+ print_affected_files $shortsha1 $todo
+ fi
else
sha1=$(git rev-parse $shortsha1)
if test -z $rebase_root
@@ -997,6 +1006,10 @@ do
then
touch $rewritten/$sha1
printf '%s\n' ${comment_out}pick $shortsha1 $rest $todo
+ if test -n $show_files
+ then
+print_affected_files $sha1 $todo
+ fi
fi
fi
done
diff --git a/git-rebase.sh b/git-rebase.sh
index 55da9db..4968b2c 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -24,6 +24,7 @@ m,merge! use merging strategies to rebase
i,interactive! let the user edit the list of commits to rebase
x,exec=! add exec lines after each commit of the editable list
k,keep-empty preserve empty commits during rebase
+F,show-files Show files affected by each list of commit to rebase
f,force-rebase!force rebase even if branch is up to date
X,strategy-option=! pass the argument through to the merge strategy
stat! display a diffstat of what changed upstream
@@ -88,6 +89,7 @@ autosquash=
keep_empty=
test $(git config --bool rebase.autosquash) = true autosquash=t
gpg_sign_opt=
+show_files=
read_basic_state () {
test -f $state_dir/head-name
@@ -336,6 +338,9 @@ do
--gpg-sign=*)
gpg_sign_opt=-S${1#--gpg-sign=}
;;
+ --show-files|-F)
+ show_files=t
+ ;;
--)
shift
break
--
2.1.0.244.g5796467.dirty
Re: [RFC/PATCH 1/2] config: Add safe-include directive
On Thu, Oct 2, 2014 at 6:37 PM, Rasmus Villemoes r...@rasmusvillemoes.dk wrote: This adds a variant of the include directive, where only certain config variables in the included files are honoured. The set of honoured variables consists of those the user has mentioned in a safe-include.whitelist directive, along with a small set of git.git blessed ones. This can, for example, be used by a project to supply a set of suggested configuration variables, such as diff.renames = true. The project would provide these in e.g project.gitconfig, and the user then has to explicitly opt-in by putting [safe-include] path = ../project.gitconfig into .git/config, possibly preceding the path directive with a whitelist directive. Good thinking to protect against accidental inclusion by older versions of Git. Even though I did allude to ../project.gitconfig in the original message, I think there should probably be an explicit syntax to name a path that is relative to the root level of the working tree. People do funky things using $GIT_DIR and $GIT_WORK_TREE to break the .. relative to the config file is the root level of the working tree assumption, and also a repository can have a regular file .git that points at the real location of the directory that has config in it, in which case its parent directory is very unlikely to be the root level of the working tree. That syntax _could_ be just a relative path (e.g. project.gitconfig names the file with that name at the top-level of the working tree), and if we are to do so, we should forbid any relative path that escapes from the working tree (e.g. ../project.gitconfig is forbidden, but down/down/../../.gitconfig could be OK as it is the same as .gitconfig). For that matter, anything with /./ and /../ in it can safely be forbidden without losing functionality. The reason why I think it is sufficient to take a relative path as relative to the working tree is primarily because I do not see a reason why we would want to do a safer inclusion of anything inside $GIT_DIR (which would be the natural interpretation if the relatigve path is taken as relative to the including config file, in the same way as how the regular include is processed). But I could be missing some other useful use cases. And we can allow absolute path, e.g. /etc/gitconfig, of course, but I'd prefer to at least initially forbid an absolute path, due to the same worries I have against the unset some variables defined in /etc/gitconfig topic we discussed earlier today in a separate thread. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/2] config: Add safe-include directive
On Thu, Oct 2, 2014 at 10:27 PM, Junio C Hamano gits...@pobox.com wrote: On Thu, Oct 2, 2014 at 6:37 PM, Rasmus Villemoes r...@rasmusvillemoes.dk wrote: This adds a variant of the include directive, where only certain config variables in the included files are honoured. The set of honoured variables consists of those the user has mentioned in a safe-include.whitelist directive, along with a small set of git.git blessed ones. Another design decision we would need to make is if it should be allowed for a safe-included file to use safe-include directive to include other files. Offhand I do not think of a reason we absolutely need to support it, but there may be an interesting workflow enabled if we did so. I dunno. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
