{"thread":{"id":"37664","subject":"[PATCH/RFC 0/5] add \"unset.variable\" for unsetting previously set variables","startedAt":"2014-10-02T13:24:47Z","lastAt":"2014-10-13T18:21:50Z","messageCount":30,"participants":["Tanay Abhra","Junio C Hamano","Jeff King","Matthieu Moy","Jakub Narębski"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"250162","messageId":"1412256292-4286-1-git-send-email-tanayabh@gmail.com","threadId":"37664","inReplyTo":null,"subject":"[PATCH/RFC 0/5] add \"unset.variable\" for unsetting previously set variables","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-10-02T13:24:47Z","receivedAt":"2014-10-02T13:24:47Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Hi,\n\nThis series aims to add a method to filter previously set variables.\nThe patch series can be best described by the 3/5 log message\nwhich I have pasted below verbatim.\n\n\"\nAdd a new config variable \"unset.variable\" which unsets previously set\nvariables. It affects `git_config()` and `git_config_get_*()` family\nof functions. It removes the matching variables from the `configset`\nwhich were added previously. Those matching variables which come after\nthe \"unset.variable\" in parsing order will not be deleted and will\nbe left untouched.\n\nIt affects the result of \"git config -l\" and similar calls.\nIt may be used in cases where the user can not access the config files,\nfor example, the system wide config files may be only accessible to\nthe system administrator. We can unset an unwanted variable declared in\nthe system config file by using \"unset.variable\" in a local config file.\n\nfor example, /etc/gitconfig may look like this,\n\t[foo]\n\t\tbar = baz\n\nin the repo config file, we will write,\n\t[unset]\n\t\tvariable  = foo.bar\nto unset foo.bar previously declared in system wide config file.\n\"\n\nNow, I have some points of\ncontention which I like to clarify,\n\n1> The name of the variable, I could not decide between \"unset.variable\"\nand \"config.unset\", or may be some other name would be more appropriate.\n\n2> It affects both the C git_config() calls and, git config shell\ninvocations. Due to this some variables may be absent from the git config -l\nresult which might confuse the user.\n\n3> I also have an another implementation for this series which just marks the config\nvariables instead of deleting them from the configset. This can be used to\nprovide two versions of git_config(), one with filtered variables other without\nit.\n\n4> While hacking on this series, I saw that git_config_int() does not print\nthe file name of the invalid variable when values are fed by the configset.\nI will correct this regression in another patch.\n\nCheers,\nTanay\n\n\n Documentation/config.txt | 12 +++++++\n config.c                 | 93 +++++++++++++++++++++++++++++++++++++-----------\n t/t1300-repo-config.sh   | 56 ++++++++++++++++++++++++++++-\n 3 files changed, 139 insertions(+), 22 deletions(-)\n\n-- \n1.9.0.GIT\n"},{"id":"250164","messageId":"1412256292-4286-2-git-send-email-tanayabh@gmail.com","threadId":"37664","inReplyTo":"1412256292-4286-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH/RFC 1/5] config.c : move configset_iter() to an appropriate position","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-10-02T13:24:48Z","receivedAt":"2014-10-02T13:24:48Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Move configset_iter() to an appropriate position where it\ncan be called by git_config_*() family without putting\na forward declaration for it. \n\nHelped-by: Matthieu Moy <Matthieu.Moy@imag.fr>\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n config.c | 38 +++++++++++++++++++-------------------\n 1 file changed, 19 insertions(+), 19 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex a677eb6..cb474b2 100644\n--- a/config.c\n+++ b/config.c\n@@ -1150,6 +1150,25 @@ int git_config_system(void)\n \treturn !git_env_bool(\"GIT_CONFIG_NOSYSTEM\", 0);\n }\n \n+static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)\n+{\n+\tint i, value_index;\n+\tstruct string_list *values;\n+\tstruct config_set_element *entry;\n+\tstruct configset_list *list = &cs->list;\n+\tstruct key_value_info *kv_info;\n+\n+\tfor (i = 0; i < list->nr; i++) {\n+\t\tentry = list->items[i].e;\n+\t\tvalue_index = list->items[i].value_index;\n+\t\tvalues = &entry->value_list;\n+\t\tif (fn(entry->key, values->items[value_index].string, data) < 0) {\n+\t\t\tkv_info = values->items[value_index].util;\n+\t\t\tgit_die_config_linenr(entry->key, kv_info->filename, kv_info->linenr);\n+\t\t}\n+\t}\n+}\n+\n int git_config_early(config_fn_t fn, void *data, const char *repo_config)\n {\n \tint ret = 0, found = 0;\n@@ -1245,25 +1264,6 @@ static void git_config_raw(config_fn_t fn, void *data)\n \t\tdie(_(\"unknown error occured while reading the configuration files\"));\n }\n \n-static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)\n-{\n-\tint i, value_index;\n-\tstruct string_list *values;\n-\tstruct config_set_element *entry;\n-\tstruct configset_list *list = &cs->list;\n-\tstruct key_value_info *kv_info;\n-\n-\tfor (i = 0; i < list->nr; i++) {\n-\t\tentry = list->items[i].e;\n-\t\tvalue_index = list->items[i].value_index;\n-\t\tvalues = &entry->value_list;\n-\t\tif (fn(entry->key, values->items[value_index].string, data) < 0) {\n-\t\t\tkv_info = values->items[value_index].util;\n-\t\t\tgit_die_config_linenr(entry->key, kv_info->filename, kv_info->linenr);\n-\t\t}\n-\t}\n-}\n-\n static void git_config_check_init(void);\n \n void git_config(config_fn_t fn, void *data)\n-- \n1.9.0.GIT\n"},{"id":"250163","messageId":"1412256292-4286-3-git-send-email-tanayabh@gmail.com","threadId":"37664","inReplyTo":"1412256292-4286-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH/RFC 2/5] make git_config_with_options() to use a configset","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-10-02T13:24:49Z","receivedAt":"2014-10-02T13:24:49Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Make git_config_with_options() to use a configset to feed values\nin the callback function. This change gives us the power to filter\nvariables we feed to the callback using custom constraints.\n\nA slight behaviour change, git_config_int() loses the ability to\nprint the file name of the invalid variable while dying.\n\nHelped-by: Matthieu Moy <Matthieu.Moy@imag.fr>\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n config.c               | 21 +++++++++++++++++++--\n t/t1300-repo-config.sh |  2 +-\n 2 files changed, 20 insertions(+), 3 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex cb474b2..09cf009 100644\n--- a/config.c\n+++ b/config.c\n@@ -1214,7 +1214,7 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)\n \treturn ret == 0 ? found : ret;\n }\n \n-int git_config_with_options(config_fn_t fn, void *data,\n+static int git_config_with_options_raw(config_fn_t fn, void *data,\n \t\t\t    struct git_config_source *config_source,\n \t\t\t    int respect_includes)\n {\n@@ -1247,9 +1247,26 @@ int git_config_with_options(config_fn_t fn, void *data,\n \treturn ret;\n }\n \n+static int config_set_callback(const char *key, const char *value, void *cb);\n+\n+int git_config_with_options(config_fn_t fn, void *data,\n+\t\t\t    struct git_config_source *config_source,\n+\t\t\t    int respect_includes)\n+{\n+\tint ret;\n+\tstruct config_set options_config;\n+\tgit_configset_init(&options_config);\n+\tret = git_config_with_options_raw(config_set_callback, &options_config,\n+\t\t\t\t\t  config_source, respect_includes);\n+\tif (ret >= 0)\n+\t\tconfigset_iter(&options_config, fn, data);\n+\tgit_configset_clear(&options_config);\n+\treturn ret;\n+}\n+\n static void git_config_raw(config_fn_t fn, void *data)\n {\n-\tif (git_config_with_options(fn, data, NULL, 1) < 0)\n+\tif (git_config_with_options_raw(fn, data, NULL, 1) < 0)\n \t\t/*\n \t\t * git_config_with_options() normally returns only\n \t\t * positive values, as most errors are fatal, and\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 938fc8b..ce5ea01 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -678,7 +678,7 @@ test_expect_success 'invalid unit' '\n \tgit config aninvalid.unit >actual &&\n \ttest_cmp expect actual &&\n \tcat >expect <<-\\EOF\n-\tfatal: bad numeric config value '\\''1auto'\\'' for '\\''aninvalid.unit'\\'' in .git/config: invalid unit\n+\tfatal: bad numeric config value '\\''1auto'\\'' for '\\''aninvalid.unit'\\'': invalid unit\n \tEOF\n \ttest_must_fail git config --int --get aninvalid.unit 2>actual &&\n \ttest_i18ncmp expect actual\n-- \n1.9.0.GIT\n"},{"id":"250165","messageId":"1412256292-4286-4-git-send-email-tanayabh@gmail.com","threadId":"37664","inReplyTo":"1412256292-4286-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH/RFC 3/5] add \"unset.variable\" for unsetting previously set variables","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-10-02T13:24:50Z","receivedAt":"2014-10-02T13:24:50Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Add a new config variable \"unset.variable\" which unsets previously set\nvariables. It affects `git_config()` and `git_config_get_*()` family\nof functions. It removes the matching variables from the `configset`\nwhich were added previously. Those matching variables which come after\nthe \"unset.variable\" in parsing order will not be deleted and will\nbe left untouched.\n\nIt affects the result of \"git config -l\" and similar calls.\nIt may be used in cases where the user can not access the config files,\nfor example, the system wide config files may be only accessible to\nthe system administrator. We can unset an unwanted variable declared in\nthe system config file by using \"unset.variable\" in a local config file.\n\nfor example, /etc/gitconfig may look like this,\n\t[foo]\n\t\tbar = baz\n\nin the repo config file, we will write,\n\t[unset]\n\t\tvariable  = foo.bar\nto unset foo.bar previously declared in system wide config file.\n\nHelped-by: Matthieu Moy <Matthieu.Moy@imag.fr>\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n config.c | 34 ++++++++++++++++++++++++++++++++++\n 1 file changed, 34 insertions(+)\n\ndiff --git a/config.c b/config.c\nindex 09cf009..a80832d 100644\n--- a/config.c\n+++ b/config.c\n@@ -1311,6 +1311,38 @@ static struct config_set_element *configset_find_element(struct config_set *cs,\n \treturn found_entry;\n }\n \n+static void delete_config_variable(struct config_set *cs, const char *key, const char *value)\n+{\n+\tchar *normalized_value;\n+\tstruct config_set_element *e = NULL;\n+\tint ret, current = 0, updated = 0;\n+\tstruct configset_list *list = &cs->list;\n+\t/*\n+\t * if we find a key value pair with key as \"unset.variable\", unset all variables\n+\t * in the configset with keys equivalent to the value in \"unset.variable\".\n+\t * unsetting a variable means that the variable is permanently deleted from the\n+\t * configset.\n+\t */\n+\tret = git_config_parse_key(value, &normalized_value, NULL);\n+\tif (!ret) {\n+\t\t/* first remove matching variables from the configset_list */\n+\t\twhile (current < list->nr) {\n+\t\t\tif (!strcmp(list->items[current].e->key, normalized_value))\n+\t\t\t\tcurrent++;\n+\t\t\telse\n+\t\t\t\tlist->items[updated++] = list->items[current++];\n+\t\t}\n+\t\tlist->nr = updated;\n+\t\t/* then delete the matching entry from the configset hashmap */\n+\t\te = configset_find_element(cs, normalized_value);\n+\t\tif (e) {\n+\t\t\tfree(e->key);\n+\t\t\tstring_list_clear(&e->value_list, 1);\n+\t\t\thashmap_remove(&cs->config_hash, e, NULL);\n+\t\t}\n+\t}\n+}\n+\n static int configset_add_value(struct config_set *cs, const char *key, const char *value)\n {\n \tstruct config_set_element *e;\n@@ -1331,6 +1363,8 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha\n \t\thashmap_add(&cs->config_hash, e);\n \t}\n \tsi = string_list_append_nodup(&e->value_list, value ? xstrdup(value) : NULL);\n+\tif (!strcmp(key, \"unset.variable\"))\n+\t\tdelete_config_variable(cs, key, value);\n \n \tALLOC_GROW(cs->list.items, cs->list.nr + 1, cs->list.alloc);\n \tl_item = &cs->list.items[cs->list.nr++];\n-- \n1.9.0.GIT\n"},{"id":"250166","messageId":"1412256292-4286-5-git-send-email-tanayabh@gmail.com","threadId":"37664","inReplyTo":"1412256292-4286-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH/RFC 4/5] document the new \"unset.variable\" variable","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-10-02T13:24:51Z","receivedAt":"2014-10-02T13:24:51Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Helped-by: Matthieu Moy <Matthieu.Moy@imag.fr>\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n Documentation/config.txt | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 3b5b24a..7f36d35 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2382,6 +2382,18 @@ transfer.unpackLimit::\n \tnot set, the value of this variable is used instead.\n \tThe default value is 100.\n \n+unset.variable::\n+\tThis variable can be used to unset previously set variables\n+\twhich had been already declared in files of lower priority\n+\tor declared before in the same file. It does not unset\n+\tmatching variables declared after its position in the file\n+\tor in files of higher priority. It can be used to unset\n+\tpesky variables declared in files which the user might not\n+\tbe able to open due to not having the required security\n+\tprivileges, for example, system wide configuration file\n+\t`/etc/gitconfig` which may be accessible to the system\n+\tadministrator only.\n+\n uploadarchive.allowUnreachable::\n \tIf true, allow clients to use `git archive --remote` to request\n \tany tree, whether reachable from the ref tips or not. See the\n-- \n1.9.0.GIT\n"},{"id":"250167","messageId":"1412256292-4286-6-git-send-email-tanayabh@gmail.com","threadId":"37664","inReplyTo":"1412256292-4286-1-git-send-email-tanayabh@gmail.com","subject":"[PATCH/RFC 5/5] add tests for checking the behaviour of \"unset.variable\"","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-10-02T13:24:52Z","receivedAt":"2014-10-02T13:24:52Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Helped-by: Matthieu Moy <Matthieu.Moy@imag.fr>\nSigned-off-by: Tanay Abhra <tanayabh@gmail.com>\n---\n t/t1300-repo-config.sh | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 54 insertions(+)\n\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex ce5ea01..f75c001 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -1179,4 +1179,58 @@ test_expect_success POSIXPERM,PERL 'preserves existing permissions' '\n \t  \"die q(badrename) if ((stat(q(.git/config)))[2] & 07777) != 0600\"\n '\n \n+test_expect_success 'unset.variable unsets all previous matching keys' '\n+\tcat >.git/config <<-\\EOF &&\n+\t[alias]\n+\t\tcheckconfig = -c foo.check=baz config foo.check\n+\t\tcheckconfig = -c foo.check=bar config foo.check\n+\t[unset]\n+\t\tvariable = alias.checkconfig\n+\tEOF\n+\n+\ttest_expect_code 1 git checkconfig\n+'\n+\n+test_expect_success 'unset.variable does not touch all matching keys after it' '\n+\tcat >.git/config <<-\\EOF &&\n+\t[alias]\n+\t\tcheckconfig = -c foo.check=foo config foo.check\n+\t[unset]\n+\t\tvariable = alias.checkconfig\n+\t[alias]\n+\t\tcheckconfig = -c foo.check=baz config foo.check\n+\t\tcheckconfig = -c foo.check=bar config foo.check\n+\tEOF\n+\n+\tcat >expect <<-\\EOF &&\n+\tbar\n+\tEOF\n+\n+\ttest_expect_code 0 git checkconfig >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'document how unset.variable will behave in shell scripts' '\n+\trm -f .git/config &&\n+\tcat >expect <<-\\EOF &&\n+\tEOF\n+\tgit config foo.bar boz1 &&\n+\tgit config --add foo.bar boz2 &&\n+\tgit config unset.variable foo.bar &&\n+\tgit config --add foo.bar boz3 &&\n+\ttest_must_fail git config --get-all foo.bar >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'unset.variable declared after in shell scripts' '\n+\trm -f .git/config &&\n+\tcat >expect <<-\\EOF &&\n+\tEOF\n+\tgit config foo.bar boz1 &&\n+\tgit config --add foo.bar boz2 &&\n+\tgit config unset.variable foo.bar &&\n+\ttest_must_fail git config --get-all foo.bar >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.9.0.GIT\n"},{"id":"250180","messageId":"xmqq1tqqnud1.fsf@gitster.dls.corp.google.com","threadId":"37664","inReplyTo":"1412256292-4286-1-git-send-email-tanayabh@gmail.com","subject":"Re: [PATCH/RFC 0/5] add \"unset.variable\" for unsetting previously set variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-02T19:29:14Z","receivedAt":"2014-10-02T19:29:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tanay Abhra <tanayabh@gmail.com> writes:\n\n(just this point quick)\n\n> 1> The name of the variable, I could not decide between \"unset.variable\"\n> and \"config.unset\", or may be some other name would be more appropriate.\n\nI'd prefer to see this as [config] something.\n\nI wish we did the include as \"[config] include = path/to/filename\",\nnot as \"[include] path = path/to/filename\".  Perhaps we can deprecate\nand move it over time?\n"},{"id":"250181","messageId":"xmqqvbo2meg5.fsf@gitster.dls.corp.google.com","threadId":"37664","inReplyTo":"1412256292-4286-1-git-send-email-tanayabh@gmail.com","subject":"Re: [PATCH/RFC 0/5] add \"unset.variable\" for unsetting previously set variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-02T19:58:18Z","receivedAt":"2014-10-02T19:58:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tanay Abhra <tanayabh@gmail.com> writes:\n\n> 2> It affects both the C git_config() calls and, git config shell\n> invocations. Due to this some variables may be absent from the git config -l\n> result which might confuse the user.\n\nI am not sure what you mean by this.  If you process variable\ndefinitions as they come, and you read\n\n\t[some]\n\t\tvariable = set to some value initially\n                ...\n\t[unset]\n\t\tvariable = some.variable\n\t\t...\n\t[some]\n\t\tvariable = set to some other value\n\nthen a user might be able to see\n\n\t$ git config -l\n        some.variable=set to some value initially\n        !some.variable\n\tsome.variable=set to some other value\n\n(here, I am using an imaginary \"!variable.name\" to denote \"this\nvariable is unset at this point in the reading sequence\").\n\nI would imagine if the result comes from a caching layer, the user\nwould see\n\n\t$ git config -l\n\tsome.variable=set to some other value\n\nand nothing else.  Is that what you are referring to?\n\nI would think that the latter is probably desirable; otherwise we\nwould need to come up with a way to say \"forget about everything we\nsaid about this variable so far\" (i.e. my \"!some.variable\" above)\nand also the scripts that parse \"git config -l\" output need to code\nthe logic to forget and start afresh.\n\n> 3> I also have an another implementation for this series which just marks the config\n> variables instead of deleting them from the configset. This can be used to\n> provide two versions of git_config(), one with filtered variables other without\n> it.\n\nI do not see a value in the filtered version.\n\nWhat worries me _more_ is why people may want to put things in\nsystem-wide global, and if it is a wise thing to do to allow users\nto override.\n\nWe may later want to add ways to mark variables in various ways,\ne.g. (thinking aloud)\n\n - \"[config] sealed = section.variable\" will prevent the variable\n   from being reset, modified or appended.  If an administrator\n   wants to enforce a certain setting in /etc/gitconfig, she may\n   mark sensitive variables as cofnig.sealed at the end of the file:\n\n\t[security]\n        \tenforced = true\n                ...\n\t[config]\n        \tsealed = security.enforced\n\n   and we would ignore \n\n\t[config]\n        \tunset = security.enforce\n        [security]\n\t\tenforce = false\n\n   written in .git/config or ~/.gitconfig, perhaps?\n\n - \"[config] safeInclude = path\" will allow a configuration file to\n   be included safely from the project's working tree.  The path\n   given as the value must be a relative path and it is relative to\n   the top level of the project's working tree.\n\n - \"[config] safe = section.variable\" will list variables that can\n   be included with the config.safeInclude mechanism.  Any variable\n   that is not marked as config.safe that appears in the file\n   included by the config.safeInclude mechanism will be ignored.\n\n   The 'frotz' project might have a .frotz-gitconfig file at the\n   root level of its working tree that stores this:\n\n\t[diff]\n        \trenames = true\n\n   and your .git/config may have\n\n\t[config]\n        \tsafe = diff.renames\n                safeInclude = .frotz-gitconfig\n\n   You will not have to worry about a malicious participant\n   committing a\n\n\t[diff]\n        \texternal = rm -fr .\n\n   to .frotz-gitconfig and pushing it to the project because you do\n   not mark diff.external as config.safe in your .git/config\n"},{"id":"250182","messageId":"xmqqr3yqmdxa.fsf@gitster.dls.corp.google.com","threadId":"37664","inReplyTo":"1412256292-4286-6-git-send-email-tanayabh@gmail.com","subject":"Re: [PATCH/RFC 5/5] add tests for checking the behaviour of \"unset.variable\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-02T20:09:37Z","receivedAt":"2014-10-02T20:09:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tanay Abhra <tanayabh@gmail.com> writes:\n\n> +test_expect_success 'document how unset.variable will behave in shell scripts' '\n> +\trm -f .git/config &&\n> +\tcat >expect <<-\\EOF &&\n> +\tEOF\n> +\tgit config foo.bar boz1 &&\n> +\tgit config --add foo.bar boz2 &&\n> +\tgit config unset.variable foo.bar &&\n> +\tgit config --add foo.bar boz3 &&\n> +\ttest_must_fail git config --get-all foo.bar >actual &&\n\nYou make foo.bar a multi-valued one, then you unset it, so I would\nimagine that the value given after that, 'boz3', would be the only\nvalue foo.bar has.  Why should --get-all fail?\n\nI am having a hard time imagining how this behaviour can make any\nsense.\n"},{"id":"250183","messageId":"542DB2FE.609@gmail.com","threadId":"37664","inReplyTo":"xmqqr3yqmdxa.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 5/5] add tests for checking the behaviour of \"unset.variable\"","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-10-02T20:18:06Z","receivedAt":"2014-10-02T20:18:06Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"On 10/3/2014 1:39 AM, Junio C Hamano wrote:\n> Tanay Abhra <tanayabh@gmail.com> writes:\n> \n>> +test_expect_success 'document how unset.variable will behave in shell scripts' '\n>> +\trm -f .git/config &&\n>> +\tcat >expect <<-\\EOF &&\n>> +\tEOF\n>> +\tgit config foo.bar boz1 &&\n>> +\tgit config --add foo.bar boz2 &&\n>> +\tgit config unset.variable foo.bar &&\n>> +\tgit config --add foo.bar boz3 &&\n>> +\ttest_must_fail git config --get-all foo.bar >actual &&\n> \n> You make foo.bar a multi-valued one, then you unset it, so I would\n> imagine that the value given after that, 'boz3', would be the only\n> value foo.bar has.  Why should --get-all fail?\n>\n> I am having a hard time imagining how this behaviour can make any\n> sense.\n> \n\ngit config -add appends the value to a existing header, after these\ntwo commands have executed the config file would look like,\n\ngit config foo.bar boz1 &&\ngit config --add foo.bar boz2 &&\n\n[foo]\n\tbar = boz1\n\tbar = boz2\n\nAfter git config unset.variable foo.bar,\n\n[foo]\n\tbar = boz1\n\tbar = boz2\n[unset]\n\tvariable = foo.bar\n\nNow the tricky part, git config --add foo.bar boz3 append to the\nexisting header,\n\n[foo]\n\tbar = boz1\n\tbar = boz2\n\tbar = boz3\n[unset]\n\tvariable = foo.bar\n\nSince unset.variable unsets all previous set values in parsing order,\ngit config --get-all foo.bar gives us nothing in result.\n"},{"id":"250184","messageId":"xmqqmw9emdax.fsf@gitster.dls.corp.google.com","threadId":"37664","inReplyTo":"542DB2FE.609@gmail.com","subject":"Re: [PATCH/RFC 5/5] add tests for checking the behaviour of \"unset.variable\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-02T20:23:02Z","receivedAt":"2014-10-02T20:23:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tanay Abhra <tanayabh@gmail.com> writes:\n\n> On 10/3/2014 1:39 AM, Junio C Hamano wrote:\n>> Tanay Abhra <tanayabh@gmail.com> writes:\n>> \n>>> +test_expect_success 'document how unset.variable will behave in shell scripts' '\n>>> +\trm -f .git/config &&\n>>> +\tcat >expect <<-\\EOF &&\n>>> +\tEOF\n>>> +\tgit config foo.bar boz1 &&\n>>> +\tgit config --add foo.bar boz2 &&\n>>> +\tgit config unset.variable foo.bar &&\n>>> +\tgit config --add foo.bar boz3 &&\n>>> +\ttest_must_fail git config --get-all foo.bar >actual &&\n>> \n>> You make foo.bar a multi-valued one, then you unset it, so I would\n>> imagine that the value given after that, 'boz3', would be the only\n>> value foo.bar has.  Why should --get-all fail?\n>>\n>> I am having a hard time imagining how this behaviour can make any\n>> sense.\n>> \n>\n> git config -add appends the value to a existing header, after these\n> two commands have executed the config file would look like,\n> ...\n\nI *know* how it is implemented before the changes of this series.\nAnd if the original implementation of \"add\" is left as-is, I can\nimagine how such a behaviour that is unintuitive to end-users can\narise.\n\nI was and am having a hard time how this behaviour can make any\nsense from an end-user's point of view.\n"},{"id":"250185","messageId":"542DB711.9040503@gmail.com","threadId":"37664","inReplyTo":"xmqqmw9emdax.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 5/5] add tests for checking the behaviour of \"unset.variable\"","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-10-02T20:35:29Z","receivedAt":"2014-10-02T20:35:29Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"\n\nOn 10/3/2014 1:53 AM, Junio C Hamano wrote:\n> Tanay Abhra <tanayabh@gmail.com> writes:\n> \n>> On 10/3/2014 1:39 AM, Junio C Hamano wrote:\n>>> Tanay Abhra <tanayabh@gmail.com> writes:\n>>>\n>>>> +test_expect_success 'document how unset.variable will behave in shell scripts' '\n>>>> +\trm -f .git/config &&\n>>>> +\tcat >expect <<-\\EOF &&\n>>>> +\tEOF\n>>>> +\tgit config foo.bar boz1 &&\n>>>> +\tgit config --add foo.bar boz2 &&\n>>>> +\tgit config unset.variable foo.bar &&\n>>>> +\tgit config --add foo.bar boz3 &&\n>>>> +\ttest_must_fail git config --get-all foo.bar >actual &&\n>>>\n>>> You make foo.bar a multi-valued one, then you unset it, so I would\n>>> imagine that the value given after that, 'boz3', would be the only\n>>> value foo.bar has.  Why should --get-all fail?\n>>>\n>>> I am having a hard time imagining how this behaviour can make any\n>>> sense.\n>>>\n>>\n>> git config -add appends the value to a existing header, after these\n>> two commands have executed the config file would look like,\n>> ...\n> \n> I *know* how it is implemented before the changes of this series.\n> And if the original implementation of \"add\" is left as-is, I can\n> imagine how such a behaviour that is unintuitive to end-users can\n> arise.\n> \n> I was and am having a hard time how this behaviour can make any\n> sense from an end-user's point of view.\n>\n\nThat's what I was trying to document. I think that it makes no sense\nto use the feature as I had shown above. I had envisioned \"unset.variable\" to\nbe explicitly typed in on the appropriate place as can be seen in the first\ntwo tests but Matthieu had suggested to add tests using git config too. This\nis when I had discovered these inconsistencies.\n\nI can think of two solutions, one leave it as it is and advertise it to be\nexplicitly typed in the config files at the appropriate position or to change\nthe behavior of unset.variable to unset all matching variables in that file,\nbefore and after. We could also change git config --add to append at the end\nof the file regardless the variable exists or not. Which course of action\ndo you think would be best?\n"},{"id":"250186","messageId":"20141002204106.GA4556@peff.net","threadId":"37664","inReplyTo":"xmqq1tqqnud1.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 0/5] add \"unset.variable\" for unsetting previously set variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-10-02T20:41:07Z","receivedAt":"2014-10-02T20:41:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 02, 2014 at 12:29:14PM -0700, Junio C Hamano wrote:\n\n> Tanay Abhra <tanayabh@gmail.com> writes:\n> \n> (just this point quick)\n> \n> > 1> The name of the variable, I could not decide between \"unset.variable\"\n> > and \"config.unset\", or may be some other name would be more appropriate.\n> \n> I'd prefer to see this as [config] something.\n> \n> I wish we did the include as \"[config] include = path/to/filename\",\n> not as \"[include] path = path/to/filename\".  Perhaps we can deprecate\n> and move it over time?\n\nI chose [include] because I had intended there to be multiple include\nvariables (include.path, include.ref, etc). The others were shot down\nfor now. If we put it under [config], I'd still prefer to leave room by\ncalling it:\n\n  [config]\n  includePath = path/to/filename\n\nI also wanted [include] as a section name to leave room for\nconditional includes. We've sometimes discussed things like:\n\n  [include \"has-some-git-feature\"]\n  path = ...\n\nto allow conditional inclusion only when git supports a certain\nfeature-set (so that your config doesn't cause git to blow up when you\nuse an old version of git). It's possible that I'm the only person in\nthe world who really wants that, because I run old git versions all the\ntime for testing and debugging. And it is kind of gross as a syntax. But\nit would still be nice to leave room for it.\n\nI don't think there's a reason we couldn't allow:\n\n  [config \"condition...\"]\n  includePath = ...\n\nin the same way if we wanted to (though aside from includes, I do not\nknow of any other feature that would want the condition).\n\nSo I'm not _opposed_ to adding [config], deprecating [include], and\nwaiting a bit before dropping [include]. But I also don't really see the\ncurrent name as a particularly bad thing.\n\n-Peff\n"},{"id":"250187","messageId":"xmqqiok2m494.fsf@gitster.dls.corp.google.com","threadId":"37664","inReplyTo":"542DB711.9040503@gmail.com","subject":"Re: [PATCH/RFC 5/5] add tests for checking the behaviour of \"unset.variable\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-02T23:38:31Z","receivedAt":"2014-10-02T23:38:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tanay Abhra <tanayabh@gmail.com> writes:\n\n> I can think of two solutions, one leave it as it is and advertise it to be\n> explicitly typed in the config files at the appropriate position or to change\n> the behavior of unset.variable to unset all matching variables in that file,\n> before and after. We could also change git config --add to append at the end\n> of the file regardless the variable exists or not. Which course of action\n> do you think would be best?\n\nOff the top of my head, from an end-user's point of view, something\nlike this would give a behaviour that is at least understandable:\n\n (1) forbid \"git config\" command line from touching \"unset.var\", as\n     there is no way for a user to control where a new unset.var\n     goes.  And\n\n (2) When adding or appending section.var (it may also apply to\n     removing one--you need to think about it deeper), ignore\n     everything that comes before the last appearance of \"unset.var\"\n     that unsets the \"section.var\" variable.\n\nThat way, if you do not have \"[section]\" after \"[unset] variable =\nsection.var\", you would end up adding a new \"[section] var = value\",\nand if you already have \"[section]\", you would add a \"var = value\"\nin that existing \"[section]\" that appears after the last unset of\nthe variable, so eerything will be kept neat.\n\nAlternatively, if the syntax to unset a \"section.var\" were not\n\n\t[unset]\n        \tvariable = section.var\n\nbut rather\n\n\t[section]\n\t\t! variable\n\nor soemthing, then the current \"find the section and append at the\nend\" code may work as-is.\n"},{"id":"250194","messageId":"vpqeguptz5k.fsf@anie.imag.fr","threadId":"37664","inReplyTo":"xmqqiok2m494.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 5/5] add tests for checking the behaviour of \"unset.variable\"","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-10-03T07:01:27Z","receivedAt":"2014-10-03T07:01:27Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Tanay Abhra <tanayabh@gmail.com> writes:\n>\n>> I can think of two solutions, one leave it as it is and advertise it to be\n>> explicitly typed in the config files at the appropriate position or to change\n>> the behavior of unset.variable to unset all matching variables in that file,\n>> before and after. We could also change git config --add to append at the end\n>> of the file regardless the variable exists or not. Which course of action\n>> do you think would be best?\n>\n> Off the top of my head, from an end-user's point of view, something\n> like this would give a behaviour that is at least understandable:\n>\n>  (1) forbid \"git config\" command line from touching \"unset.var\", as\n>      there is no way for a user to control where a new unset.var\n>      goes.  And\n\nWell, the normal use-case for unset.variable is to put it in a local\nconfig file, to unset a variable set in another, lower-priority file.\n\nThis common use-case works with the command-line \"git config\", and it\nwould be a pity to forbid the common use-case because of a particular,\nunusual case.\n\n>  (2) When adding or appending section.var (it may also apply to\n>      removing one--you need to think about it deeper), ignore\n>      everything that comes before the last appearance of \"unset.var\"\n>      that unsets the \"section.var\" variable.\n\nThat would probably be the best option from a user's point of view, but\nI'd say the implementation complexity is not worth the trouble.\n\n> Alternatively, if the syntax to unset a \"section.var\" were not\n>\n> \t[unset]\n>         \tvariable = section.var\n>\n> but rather\n>\n> \t[section]\n> \t\t! variable\n>\n> or soemthing, then the current \"find the section and append at the\n> end\" code may work as-is.\n\nBut that would break backward compatibility rather badly: old git's\nwould stop working completely in repositories using this syntax.\n\nWell, perhaps we can also consider that this is acceptable: just don't\nuse the feature for a few years if you care about backward\ncompatibility.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"250202","messageId":"xmqq1tqpm2na.fsf@gitster.dls.corp.google.com","threadId":"37664","inReplyTo":"vpqeguptz5k.fsf@anie.imag.fr","subject":"Re: [PATCH/RFC 5/5] add tests for checking the behaviour of \"unset.variable\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-03T18:25:29Z","receivedAt":"2014-10-03T18:25:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n> ...\n>> Off the top of my head, from an end-user's point of view, something\n>> like this would give a behaviour that is at least understandable:\n\nLet's make sure we have the same starting point (i.e. understanding\nof the limitation of the current code) in mind.\n\nThe \"git config [--add] section.var value\" UI, which does not give\nthe user any control over where the new definition goes in the\nresulting file, and the current implementation, which finds the last\nexisting \"section\" and adds the \"var = value\" definition at the end\n(or adds a \"section\" at the end and then adds the definition there),\nare adequate for ordinary variables.\n\nIt is fine for single-valued ones that follow \"the last one wins\"\nsemantics; \"git config\" would add the new definition at the end and\nthat definition will win.\n\nIt is manageable for multi-valued variables, too.  For uses of a\nmulti-valued variable to track unordered set of values, by\ndefinition the order does not matter.\n\n\tSide note.  Otherwise, the scripter needs to read the\n\texisting ones, --unset-all them, and then add the elements\n\tof the final list in desired order.  It is cumbersome, but\n\tfor a single multi-valued variable it is manageable.\n\nBut the \"unset.variable\" by nature wants a lot finer-grained control\nover the order in which other ordinary variables are defined and it\nitself is placed.  No matter what improvements you attempt to make\nto the implementation, because the UI is not getting enough\ninformation from the user to learn where exactly the user wants to\nadd a new variable or \"unset.variable\", it would not give you enough\nflexibility to do the job.\n\n>>  (1) forbid \"git config\" command line from touching \"unset.var\", as\n>>      there is no way for a user to control where a new unset.var\n>>      goes.  And\n>\n> Well, the normal use-case for unset.variable is to put it in a local\n> config file, to unset a variable set in another, lower-priority file.\n\nI agree that is one major use case.\n\n> This common use-case works with the command-line \"git config\", and it\n> would be a pity to forbid the common use-case because of a particular,\n> unusual case.\n\nEither you are being incoherent or I am not reading you right.  If\nyou said \"If this common use-case worked with the command-line 'git\nconfig', it would be nice, but it would be a pity because it does\nnot\", I would understand.\n\nIf you use the command line 'git config', i.e.\n\n\tgit config unset.variable xyzzy.frotz\n\nin a repository whose .git/config does not have any unset.variable,\nyou will add that _at the end_, which would undo what you did in\nyour configuration file, not just what came before yours.  Even if\nyou ignore more exotic cases, the command line is *not* working.\n\nThat is why I said \"unset.variable\" is unworkable with existing \"git\nconfig\" command line.  Always appending at the end is usable for\nordinary variables, but for unset.variable, it is most likely the\nleast useful thing to do.  You can explain \"among 47 different\nthings it could do, we chose to do the most useless thing, because\nthat is _consistent_ with how the ordinary variables are placed in\nthe cofiguration file\" in the documentation but it forgets to\nquestion if unset.variable should be treated the same way as\nordinary variables in the first place.\n\nAnother use case would be to override what includes gave us.  I.e.\n\n\t[unset]\n        \tvariable = ... nullify some /etc/gitconfig values ...\n\t[include]\n        \tpath = ~/.gitcommon\n\t[unset]\n\t\tvariable = ... nullify some ~/.gitcommon values ...\n\t[xyzzy]\n\t\tfrotz = nitfol\n\nSpecial-casing unset.variable and always adding them at the\nbeginning of the output *might* turn it from totally useless to\nslightly usable.  At least it supports the \"nullify previous ones\",\neven though it does not help \"nullify after include\".\n\nI doubt if such a change to add unset.variable always at the top is\nworth it, though.\n\n>>  (2) When adding or appending section.var (it may also apply to\n>>      removing one--you need to think about it deeper), ignore\n>>      everything that comes before the last appearance of \"unset.var\"\n>>      that unsets the \"section.var\" variable.\n>\n> That would probably be the best option from a user's point of view, but\n> I'd say the implementation complexity is not worth the trouble.\n\ntest_expect_success declares a particular behaviour as \"the right\nthing to happen\" in order to protect that right behaviour from\nfuture changes.  It is OK for you to illustrate what illogical thing\nthe implementation does as a caveat, and admit that the behaviour\ncomes because we consider the change is not worth the trouble and we\npunted.  But pretending as if that is the right behaviour and the\nsystem has to promise that the broken behaviour will be kept forever\nby casting it in stone is the worst thing you can do, I am afraid.\n\nUnfortunately, as I already said, there is no sensible behaviour to\nadd \"unset.variable\" from the command line with the current \"git\nconfig\" UI, so it is not even feasible to define \"the right thing to\nhappen\" and to document it as something other people may want to fix\nlater, e.g.\n\n\ttest_expect_failure 'natural but not working yet' '\n\t\tgit config xyzzy.frotz 1\n                git config --add xyzzy.frotz 2\n                git config --add xyzzy.frotz 3\n                : a magic command to add\n                :  [unset] variable = xyzzy.frotz \n                : between 2 and 3\n\t\tgit config xyzzy.frotz >actual\n                echo 3 >expect\n                test_expect_success expect actual\n\t'\n\nHowever, you should be able to arrange in the test to do the\nfollowing sequence:\n\n    - Define \"[xyzzy] frotz 1\" in $HOME/.gitconfig (I think $HOME\n      defaults to your trash directory).\n\n    - Verify that \"git config xyzzy.frotz\" gives \"1\".\n\n    - Define \"[unset] variable = xyzzy.frotz\" in .git/config (it is\n      OK to use \"git config unset.variable xyzzy.frotz\" here).\n\n    - Verify that \"git config xyzzy.frotz\" does not find anything.\n\n    - Define \"[xyzzy] frotz 2\" in .git/config (again, it is OK to\n      use \"git config xyzzy.frotz 2\" here).\n\n    - Verify that \"git config xyzzy.frotz\" gives \"2\".\n\nI think we can agree that the above sequence is something we would\nwant to support, regardless of how we will change/fix the underlying\nconfig-writer implementation.  Which means that something like the\nabove can safely be cast in stone with test_expect_success.\n"},{"id":"250203","messageId":"xmqqwq8hkmzw.fsf@gitster.dls.corp.google.com","threadId":"37664","inReplyTo":"xmqq1tqpm2na.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 5/5] add tests for checking the behaviour of \"unset.variable\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-03T18:48:51Z","receivedAt":"2014-10-03T18:48:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> That is why I said \"unset.variable\" is unworkable with existing \"git\n> config\" command line.  Always appending at the end is usable for\n> ordinary variables, but for unset.variable, it is most likely the\n> least useful thing to do.  You can explain \"among 47 different\n> things it could do, we chose to do the most useless thing, because\n> that is _consistent_ with how the ordinary variables are placed in\n> the cofiguration file\" in the documentation but it forgets to\n> question if unset.variable should be treated the same way as\n> ordinary variables in the first place.\n\nThis is a tangent, but the above also applies to the \"include.path\".\n"},{"id":"250208","messageId":"vpq4mvlgchj.fsf@anie.imag.fr","threadId":"37664","inReplyTo":"xmqq1tqpm2na.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 5/5] add tests for checking the behaviour of \"unset.variable\"","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-10-03T19:49:28Z","receivedAt":"2014-10-03T19:49:28Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> The \"git config [--add] section.var value\" UI, [...] finds the \"var = value\"\n> definition at the end (or adds a \"section\" at the end and then adds\n> [...]\n>\n> It is fine for single-valued ones that follow \"the last one wins\"\n> semantics; \"git config\" would add the new definition at the end and\n> that definition will win.\n\nNot always.\n\ngit config foo.bar old-value\ngit config unset.variable foo.bar\ngit config foo.bar new-value\n\nOne could expect the new value to be taken into account, but it is not.\n\n>> Well, the normal use-case for unset.variable is to put it in a local\n>> config file, to unset a variable set in another, lower-priority file.\n>\n> I agree that is one major use case.\n>\n>> This common use-case works with the command-line \"git config\", and it\n>> would be a pity to forbid the common use-case because of a particular,\n>> unusual case.\n>\n> Either you are being incoherent or I am not reading you right.  If\n> you said \"If this common use-case worked with the command-line 'git\n> config', it would be nice, but it would be a pity because it does\n> not\", I would understand.\n\nI think you missed the \"another\" in my sentence above. The normal\nuse-case is to have foo.bar and unset.variable=foo.bar in different\nfiles. In this case, you do not care about the position in file.\n\n> in a repository whose .git/config does not have any unset.variable,\n> you will add that _at the end_, which would undo what you did in\n> your configuration file, not just what came before yours.  Even if\n> you ignore more exotic cases, the command line is *not* working.\n\nIf my sysadmin has set foo.bar=boz in /etc/gitconfig, I can use\n\n  git config [--global] unset.variable foo.bar\n\nand it does work. Always.\n\nPlaying with the order of variables in-file is essentially useless OTOH\nexcept for the include case you mentionned (if I want to unset a\nvariable in a file, I'll just delete or comment out the variable and I\ndon't need unset.variable).\n\nReally, I don't see the point in making any complex plans to support the\nuseless part of the unset.variable feature. The reason it was designed\nfor already works, and $EDITOR does the job for other cases.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"250209","messageId":"xmqqeguolxow.fsf@gitster.dls.corp.google.com","threadId":"37664","inReplyTo":"vpq4mvlgchj.fsf@anie.imag.fr","subject":"Re: [PATCH/RFC 5/5] add tests for checking the behaviour of \"unset.variable\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-03T20:12:31Z","receivedAt":"2014-10-03T20:12:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> The \"git config [--add] section.var value\" UI, [...] finds the \"var = value\"\n>> definition at the end (or adds a \"section\" at the end and then adds\n>> [...]\n>>\n>> It is fine for single-valued ones that follow \"the last one wins\"\n>> semantics; \"git config\" would add the new definition at the end and\n>> that definition will win.\n>\n> Not always.\n>\n> git config foo.bar old-value\n> git config unset.variable foo.bar\n> git config foo.bar new-value\n>\n> One could expect the new value to be taken into account, but it is not.\n\nI think you misunderstood what I said.  With ordinary variables,\neverything works fine, that is, without unset.variable, which this\nseries is trying to pretend as if it were just another ordinary\nvariable, but it is not.  You are just showing how broken it is to\ntreat unset.variable as just another ordinary variable, which is\nwhat I've been telling you.\n\nSo we are in agreement.\n\n>>> Well, the normal use-case for unset.variable is to put it in a local\n>>> config file, to unset a variable set in another, lower-priority file.\n>>\n>> I agree that is one major use case.\n>>\n>>> This common use-case works with the command-line \"git config\", and it\n>>> would be a pity to forbid the common use-case because of a particular,\n>>> unusual case.\n>>\n>> Either you are being incoherent or I am not reading you right.  If\n>> you said \"If this common use-case worked with the command-line 'git\n>> config', it would be nice, but it would be a pity because it does\n>> not\", I would understand.\n>\n> I think you missed the \"another\" in my sentence above. The normal\n> use-case is to have foo.bar and unset.variable=foo.bar in different\n> files. In this case, you do not care about the position in file.\n\nI didn't miss anything.  The reason you want to have \"unset foo.bar\"\nin your .git/config is to override a \"foo.bar = z\" you have in\nanother place, e.g. ~/.gitconfig; which would let you sanely do\nanother \"foo.bar = x\" in .git/config for multi-value foo.bar, *if*\nthe unset comes before foo.bar.\n\nBut what if you have this in your .git/config file\n\n\t[core]\n\t\tbare = false\n            ... standard stuff left by git init ...\n\t[foo]\n        \tbar = x\n\nbefore you add \"unset foo.bar\" there?\n\nAnd that is not a contribed example.\n\nThe likely reason why you want to resort to \"unset foo.bar\" is that\nyou found that you get an unwanted foo.bar=z in addition to desired\nfoo.bar=z in a repository that has the above in .git/config, and at\nthat point you would want to say \"I want to unset the outside\ninfluence\".\n\nAnd there is no \"[unset] variable = foo.bar\" in there yet; without\nsome special casing, if you treated unset.variable as if it were\njust another ordinary variable, it would go after you define your\nown foo.bar=x, which is not what you want.\n"},{"id":"250272","messageId":"5432E67B.8070604@gmail.com","threadId":"37664","inReplyTo":"xmqqeguolxow.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 5/5] add tests for checking the behaviour of \"unset.variable\"","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-10-06T18:59:07Z","receivedAt":"2014-10-06T18:59:07Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"On 10/4/2014 1:42 AM, Junio C Hamano wrote:\n> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n> \n>> Junio C Hamano <gitster@pobox.com> writes:\n>>>> Well, the normal use-case for unset.variable is to put it in a local\n>>>> config file, to unset a variable set in another, lower-priority file.\n>>>\n>>> I agree that is one major use case.\n>>>\n>>>> This common use-case works with the command-line \"git config\", and it\n>>>> would be a pity to forbid the common use-case because of a particular,\n>>>> unusual case.\n>>>\n>>> Either you are being incoherent or I am not reading you right.  If\n>>> you said \"If this common use-case worked with the command-line 'git\n>>> config', it would be nice, but it would be a pity because it does\n>>> not\", I would understand.\n>>\n>> I think you missed the \"another\" in my sentence above. The normal\n>> use-case is to have foo.bar and unset.variable=foo.bar in different\n>> files. In this case, you do not care about the position in file.\n> \n> I didn't miss anything.  The reason you want to have \"unset foo.bar\"\n> in your .git/config is to override a \"foo.bar = z\" you have in\n> another place, e.g. ~/.gitconfig; which would let you sanely do\n> another \"foo.bar = x\" in .git/config for multi-value foo.bar, *if*\n> the unset comes before foo.bar.\n> \n> But what if you have this in your .git/config file\n> \n> \t[core]\n> \t\tbare = false\n>             ... standard stuff left by git init ...\n> \t[foo]\n>         \tbar = x\n> \n> before you add \"unset foo.bar\" there?\n> \n> And that is not a contribed example.\n> \n> The likely reason why you want to resort to \"unset foo.bar\" is that\n> you found that you get an unwanted foo.bar=z in addition to desired\n> foo.bar=z in a repository that has the above in .git/config, and at\n> that point you would want to say \"I want to unset the outside\n> influence\".\n> \n> And there is no \"[unset] variable = foo.bar\" in there yet; without\n> some special casing, if you treated unset.variable as if it were\n> just another ordinary variable, it would go after you define your\n> own foo.bar=x, which is not what you want.\n\nI have made some conclusions after reading the whole thread,\n\n1> Add some tests for this use case which seems the most appropriate\nuse case for this feature,\n\n(Copied from Junio's mail)\n\n    - Define \"[xyzzy] frotz 1\" in $HOME/.gitconfig (I think $HOME\n      defaults to your trash directory).\n\n    - Verify that \"git config xyzzy.frotz\" gives \"1\".\n\n    - Define \"[unset] variable = xyzzy.frotz\" in .git/config (it is\n      OK to use \"git config unset.variable xyzzy.frotz\" here).\n\n    - Verify that \"git config xyzzy.frotz\" does not find anything.\n\n    - Define \"[xyzzy] frotz 2\" in .git/config (again, it is OK to\n      use \"git config xyzzy.frotz 2\" here).\n\n    - Verify that \"git config xyzzy.frotz\" gives \"2\".\n\n2> Leave the internal implementation as it is, as it helps in manually\nwriting unset.variable in its appropriate place by using an editor,\ni.e this use case,\n\n\t[unset]\n        \tvariable = ... nullify some /etc/gitconfig values ...\n\t[include]\n        \tpath = ~/.gitcommon\n\t[unset]\n\t\tvariable = ... nullify some ~/.gitcommon values ...\n\t[xyzzy]\n\t\tfrotz = nitfol\n\n3> Special case \"unset.variable\", so that git config unset.variable foo.baz\npastes it on the top of the file. The implementation should be trivial, but,\nJunio had said in an earlier mail that he doesn't think this approach would\ndo much good.\n\nOther than this approach, Junio had suggested to append it after the last mention\nof \"foo.baz\", but I don't think if it would be worth it, but still I will cook up\nthe code for it.\n\n4> Change the name to config.unset or something.\n\nI will make the above changes in the next revision.\n\nThanks.\n"},{"id":"250273","messageId":"CAPc5daXa_Maz3nTr4s=fxxL9FYs8VbndqX-2R_iwo4KRHCKQhA@mail.gmail.com","threadId":"37664","inReplyTo":"5432E67B.8070604@gmail.com","subject":"Re: [PATCH/RFC 5/5] add tests for checking the behaviour of \"unset.variable\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-06T19:28:55Z","receivedAt":"2014-10-06T19:28:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Mon, Oct 6, 2014 at 11:59 AM, Tanay Abhra <tanayabh@gmail.com> wrote:\n> 3> Special case \"unset.variable\", so that git config unset.variable foo.baz\n> pastes it on the top of the file. The implementation should be trivial, but,\n> Junio had said in an earlier mail that he doesn't think this approach would\n> do much good.\n>\n> Other than this approach, Junio had suggested to append it after the last mention\n> of \"foo.baz\",...\n\nJust to make sure there is no misunderstanding.\n\n\"it\" in \"append it\" above does not refer to unset.variable, and\n\"the last mention of \"foo.baz\"\" is not \"[foo]baz=value\"\n\nThe point is to prevent\"git config --add foo.baz anothervalue\" starting from\n\n--- --- ---\n[foo]\n  bar = some\n[unset] variable = foo.baz\n--- --- ---\n\nfrom adding foo.baz next to existing foo.bar. We would want to end up with\n\n--- --- ---\n[foo]\n  bar = some\n[unset] variable = foo.baz\n[foo]\n  baz = anothervalue\n--- --- ---\n\nSo \"When dealing with foo.baz, ignore everything above the last unset.variable\nthat unsets foo.baz\" is what I meant (and I think that is how I wrote).\n"},{"id":"250274","messageId":"5432F0E8.7020705@gmail.com","threadId":"37664","inReplyTo":"CAPc5daXa_Maz3nTr4s=fxxL9FYs8VbndqX-2R_iwo4KRHCKQhA@mail.gmail.com","subject":"Re: [PATCH/RFC 5/5] add tests for checking the behaviour of \"unset.variable\"","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-10-06T19:43:36Z","receivedAt":"2014-10-06T19:43:36Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"On 10/7/2014 12:58 AM, Junio C Hamano wrote:\n> \n> The point is to prevent\"git config --add foo.baz anothervalue\" starting from\n> \n> --- --- ---\n> [foo]\n>   bar = some\n> [unset] variable = foo.baz\n> --- --- ---\n> \n> from adding foo.baz next to existing foo.bar. We would want to end up with\n> \n> --- --- ---\n> [foo]\n>   bar = some\n> [unset] variable = foo.baz\n> [foo]\n>   baz = anothervalue\n> --- --- ---\n> \n> So \"When dealing with foo.baz, ignore everything above the last unset.variable\n> that unsets foo.baz\" is what I meant (and I think that is how I wrote).\n>\n\n\nYes, that was what I inferred too, I should have worded it more carefully, thanks.\n"},{"id":"250277","messageId":"5433CBC3.5010202@gmail.com","threadId":"37664","inReplyTo":"xmqqvbo2meg5.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 0/5] add \"unset.variable\" for unsetting previously set variables","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2014-10-07T11:17:23Z","receivedAt":"2014-10-07T11:17:23Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano wrote:\n\n>   - \"[config] safe = section.variable\" will list variables that can\n>     be included with the config.safeInclude mechanism.  Any variable\n>     that is not marked as config.safe that appears in the file\n>     included by the config.safeInclude mechanism will be ignored.\n\nWhy user must know which variables are safe, why it cannot be left to \nGit to know which configuration variables can call external scripts?\n\n-- \nJakub Narębski\n"},{"id":"250287","messageId":"xmqq1tqjkexe.fsf@gitster.dls.corp.google.com","threadId":"37664","inReplyTo":"5433CBC3.5010202@gmail.com","subject":"Re: [PATCH/RFC 0/5] add \"unset.variable\" for unsetting previously set variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-07T16:44:29Z","receivedAt":"2014-10-07T16:44:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narębski <jnareb@gmail.com> writes:\n\n> Junio C Hamano wrote:\n>\n>>   - \"[config] safe = section.variable\" will list variables that can\n>>     be included with the config.safeInclude mechanism.  Any variable\n>>     that is not marked as config.safe that appears in the file\n>>     included by the config.safeInclude mechanism will be ignored.\n>\n> Why user must know which variables are safe, why it cannot be left to\n> Git to know which configuration variables can call external scripts?\n\nThat's a fallback to let them take responsibility for variables we\ndo not mark as \"safe\"; and having that fallback mechanism lets us\nkeep the set of variables we by default mark as safe to the absolute\nminimum.\n"},{"id":"250330","messageId":"vpqzjd7kta6.fsf@anie.imag.fr","threadId":"37664","inReplyTo":"xmqq1tqjkexe.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 0/5] add \"unset.variable\" for unsetting previously set variables","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-10-08T05:46:41Z","receivedAt":"2014-10-08T05:46:41Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jakub Narębski <jnareb@gmail.com> writes:\n>\n>> Junio C Hamano wrote:\n>>\n>>>   - \"[config] safe = section.variable\" will list variables that can\n>>>     be included with the config.safeInclude mechanism.  Any variable\n>>>     that is not marked as config.safe that appears in the file\n>>>     included by the config.safeInclude mechanism will be ignored.\n>>\n>> Why user must know which variables are safe, why it cannot be left to\n>> Git to know which configuration variables can call external scripts?\n>\n> That's a fallback to let them take responsibility for variables we\n> do not mark as \"safe\"; and having that fallback mechanism lets us\n> keep the set of variables we by default mark as safe to the absolute\n> minimum.\n\nPerhaps this would need a way to say \"this value is safe for this\nvariable\" too. I don't have a real use-case, but one could say something\nlike \"I'm OK with the file overriding core.editor, but the only values I\naccept are nano, vim and emacs\".\n\nIt doesn't seem to be a prerequisite to implement the safeInclude\nfeature, but we should live room in the namespace for the day we want to\nadd it.\n\nI don't have really good idea for it. The first I could think of was\n\n[config \"safe\"]\n    core.editor = nano\n    core.editor = vim\n    core.editor = emacs\n\nbut it's not accepted by the current parser, hence not backward\ncompatible.\n\nEmacs has such mechanism for -*- ... -*- local variables in files for\nexample.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"250362","messageId":"xmqqy4sqbi12.fsf@gitster.dls.corp.google.com","threadId":"37664","inReplyTo":"vpqzjd7kta6.fsf@anie.imag.fr","subject":"Re: [PATCH/RFC 0/5] add \"unset.variable\" for unsetting previously set variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-08T17:14:33Z","receivedAt":"2014-10-08T17:14:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> Jakub Narębski <jnareb@gmail.com> writes:\n>>\n>>> Junio C Hamano wrote:\n>>>\n>>>>   - \"[config] safe = section.variable\" will list variables that can\n>>>>     be included with the config.safeInclude mechanism.  Any variable\n>>>>     that is not marked as config.safe that appears in the file\n>>>>     included by the config.safeInclude mechanism will be ignored.\n>>>\n>>> Why user must know which variables are safe, why it cannot be left to\n>>> Git to know which configuration variables can call external scripts?\n>>\n>> That's a fallback to let them take responsibility for variables we\n>> do not mark as \"safe\"; and having that fallback mechanism lets us\n>> keep the set of variables we by default mark as safe to the absolute\n>> minimum.\n>\n> Perhaps this would need a way to say \"this value is safe for this\n> variable\" too. I don't have a real use-case, but one could say something\n> like \"I'm OK with the file overriding core.editor, but the only values I\n> accept are nano, vim and emacs\".\n>\n> It doesn't seem to be a prerequisite to implement the safeInclude\n> feature, but we should live room in the namespace for the day we want to\n> add it.\n>\n> I don't have really good idea for it. The first I could think of was\n>\n> [config \"safe\"]\n>     core.editor = nano\n>     core.editor = vim\n>     core.editor = emacs\n>\n> but it's not accepted by the current parser, hence not backward\n> compatible.\n\nInteresting thought (I've cc'ed Rasmus who did an RFC patchset on\nthe safe include feature).  I do not offhand think of a good example\nof an variable that we may want to allow overriding but still want\nto limit its values myself.  Almost all variables I would rather not\nto see in-tree .gitconfig to touch at all, and the ones that I may\nwant to allow to be futzed with I can think of offhand are booleans.\nWith more people and time we might find a better example to illustrate\nwhy we may want to have such a feature added.\n\nThanks.\n"},{"id":"250383","messageId":"vpq61fujtlk.fsf@anie.imag.fr","threadId":"37664","inReplyTo":"xmqqy4sqbi12.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 0/5] add \"unset.variable\" for unsetting previously set variables","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-10-08T18:37:27Z","receivedAt":"2014-10-08T18:37:27Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I do not offhand think of a good example of an variable that we may\n> want to allow overriding but still want to limit its values myself.\n\nI just thought of a semi-realistic use-case : diff.*.{command,textconv}.\n\nOne may want to allow per-project sets of diff drivers, but these\nvariables contain actual commands, so clearly we can't allow any\nvalue for these variables.\n\n\"semi-realistic\" only because I never needed a per-project diff driver,\nI have my per-user preference and I'm happy with it.\n\nAnyway, the feature does not seem vital to me, but if someone comes up\nwith a clever way to keep room for it in the namespace, that would be\ncool.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"250392","messageId":"xmqqk34a8hl3.fsf@gitster.dls.corp.google.com","threadId":"37664","inReplyTo":"vpq61fujtlk.fsf@anie.imag.fr","subject":"Re: [PATCH/RFC 0/5] add \"unset.variable\" for unsetting previously set variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-08T19:52:24Z","receivedAt":"2014-10-08T19:52:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> I do not offhand think of a good example of an variable that we may\n>> want to allow overriding but still want to limit its values myself.\n>\n> I just thought of a semi-realistic use-case : diff.*.{command,textconv}.\n>\n> One may want to allow per-project sets of diff drivers, but these\n> variables contain actual commands, so clearly we can't allow any\n> value for these variables.\n>\n> \"semi-realistic\" only because I never needed a per-project diff driver,\n> I have my per-user preference and I'm happy with it.\n\nThis may open another aspect of the discussion, actually.\n\nThe whole reason why the actualy diff.*.command and textconv\ncommands are defined in .git/config while the filetype label is\nassigned by in-tree .gitattributes is because these commands are\nplatform dependant.  So textconv on Linux, BSD and Windows may want\nto be different commands, and the project that ships an in-tree\n.gitconfig to be safe-included may want to not \"set\" the variable to\none specific value, but stop at offering a suggestion, i.e. \"there\nare these possibilities, perhaps you may want to pick one of them?\"\nwithout actually making the choice for the user.\n\nAnd on the receiving side (i.e. [config \"safe\"] in .git/config), it\nis unlikely that you would list textconv choices that are plausible\non different platforms.  Rather, you would say \"I would want this\nvalue to be set on textconv and not others\".\n\nBut at that point, if you have to be that informed to set up the\n[config \"safe\"] to list allowed values, I wonder why a user chooses\nto do so and safe-include in-tree .gitconfig, instead of explicitly\nsetting her preferred textconv in .git/config herself, without\nbothering to include anything.\n\n> Anyway, the feature does not seem vital to me, but if someone comes up\n> with a clever way to keep room for it in the namespace, that would be\n> cool.\n\nYes.\n"},{"id":"250474","messageId":"20141010081107.GA8355@peff.net","threadId":"37664","inReplyTo":"xmqqk34a8hl3.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC 0/5] add \"unset.variable\" for unsetting previously set variables","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-10-10T08:11:07Z","receivedAt":"2014-10-10T08:11:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 08, 2014 at 12:52:24PM -0700, Junio C Hamano wrote:\n\n> The whole reason why the actualy diff.*.command and textconv\n> commands are defined in .git/config while the filetype label is\n> assigned by in-tree .gitattributes is because these commands are\n> platform dependant.  So textconv on Linux, BSD and Windows may want\n> to be different commands, and the project that ships an in-tree\n> .gitconfig to be safe-included may want to not \"set\" the variable to\n> one specific value, but stop at offering a suggestion, i.e. \"there\n> are these possibilities, perhaps you may want to pick one of them?\"\n> without actually making the choice for the user.\n\nOr it could even auto-detect a sensible version based on the user's\nfilesystem. Which makes me wonder if safe-include is really helping that\nmuch versus a project shipping a shell script that munges the repository\nconfig. The latter is less safe (you are, after all, running code, but\nyou would at least have the chance to examine it), but is way more\nflexible. And the safety is comparable to running \"make\" on a cloned\nproject.\n\nI dunno. I do not have anything against the safe-include idea, but each\ntime it comes up, I think we are often left guessing about exactly which\nconfig options projects would want to set, and to what values.\n\n-Peff\n"},{"id":"250559","messageId":"xmqqh9z74ypt.fsf@gitster.dls.corp.google.com","threadId":"37664","inReplyTo":"20141010081107.GA8355@peff.net","subject":"Re: [PATCH/RFC 0/5] add \"unset.variable\" for unsetting previously set variables","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-13T18:21:50Z","receivedAt":"2014-10-13T18:21:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> ... Which makes me wonder if safe-include is really helping that\n> much versus a project shipping a shell script that munges the repository\n> config. The latter is less safe (you are, after all, running code, but\n> you would at least have the chance to examine it), but is way more\n> flexible. And the safety is comparable to running \"make\" on a cloned\n> project.\n>\n> I dunno. I do not have anything against the safe-include idea, but each\n> time it comes up, I think we are often left guessing about exactly which\n> config options projects would want to set, and to what values.\n\nI tend to agree.  Every time somebody says \"a project wants to give\nits participants suggested settings\", we seem to tell them to ship\nan instruction to their participants, either in BUILDING or setup.sh\nor whatever.  It certainly is simpler and more flexible.\n\nThe only real difference it might make is an attempt to push to an\nunattended place and automatically making the changes to take effect,\naka \"push to deploy\", which is not what we encourage anyway, so...\n"}]}