{"thread":{"id":"30323","subject":"[PATCH] config: allow ~/ and ~user/ in include.path value","startedAt":"2012-04-24T11:08:09Z","lastAt":"2012-04-26T06:19:23Z","messageCount":6,"participants":["Matthieu Moy","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"189964","messageId":"1335265689-2270-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"30323","inReplyTo":null,"subject":"[PATCH] config: allow ~/ and ~user/ in include.path value","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2012-04-24T11:08:09Z","receivedAt":"2012-04-24T11:08:09Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\n config.c |   10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex bfe0c79..dd8b9bf 100644\n--- a/config.c\n+++ b/config.c\n@@ -33,11 +33,14 @@ static const char include_depth_advice[] =\n \"from\\n\"\n \"\t%s\\n\"\n \"Do you have circular includes?\";\n-static int handle_path_include(const char *path, struct config_include_data *inc)\n+static int handle_path_include(const char *raw_path, struct config_include_data *inc)\n {\n \tint ret = 0;\n \tstruct strbuf buf = STRBUF_INIT;\n-\n+\tint must_free_path = 1;\n+\tchar *path = expand_user_path(raw_path);\n+\tif (!path)\n+\t\treturn error(\"Could not expand include path '%s'.\", raw_path);\n \t/*\n \t * Use an absolute path as-is, but interpret relative paths\n \t * based on the including config file.\n@@ -52,6 +55,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n \t\tif (slash)\n \t\t\tstrbuf_add(&buf, cf->name, slash - cf->name + 1);\n \t\tstrbuf_addstr(&buf, path);\n+\t\tmust_free_path = 0;\n \t\tpath = buf.buf;\n \t}\n \n@@ -63,6 +67,8 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n \t\tinc->depth--;\n \t}\n \tstrbuf_release(&buf);\n+\tif (must_free_path)\n+\t\tfree(path);\n \treturn ret;\n }\n \n-- \n1.7.10.235.gb2edec\n"},{"id":"189979","messageId":"1335292396-8502-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"30323","inReplyTo":"1335265689-2270-1-git-send-email-Matthieu.Moy@imag.fr","subject":"[PATCH v2] config: allow ~/ and ~user/ in include.path value","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2012-04-24T18:33:16Z","receivedAt":"2012-04-24T18:33:16Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nJust added a free(path) before must_free_path = 0. Not that the memory\nleak is really problematic, but let's be clean.\n\n config.c |   11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex bfe0c79..f054a35 100644\n--- a/config.c\n+++ b/config.c\n@@ -33,11 +33,14 @@ static const char include_depth_advice[] =\n \"from\\n\"\n \"\t%s\\n\"\n \"Do you have circular includes?\";\n-static int handle_path_include(const char *path, struct config_include_data *inc)\n+static int handle_path_include(const char *raw_path, struct config_include_data *inc)\n {\n \tint ret = 0;\n \tstruct strbuf buf = STRBUF_INIT;\n-\n+\tint must_free_path = 1;\n+\tchar *path = expand_user_path(raw_path);\n+\tif (!path)\n+\t\treturn error(\"Could not expand include path '%s'.\", raw_path);\n \t/*\n \t * Use an absolute path as-is, but interpret relative paths\n \t * based on the including config file.\n@@ -52,6 +55,8 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n \t\tif (slash)\n \t\t\tstrbuf_add(&buf, cf->name, slash - cf->name + 1);\n \t\tstrbuf_addstr(&buf, path);\n+\t\tfree(path);\n+\t\tmust_free_path = 0;\n \t\tpath = buf.buf;\n \t}\n \n@@ -63,6 +68,8 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n \t\tinc->depth--;\n \t}\n \tstrbuf_release(&buf);\n+\tif (must_free_path)\n+\t\tfree(path);\n \treturn ret;\n }\n \n-- \n1.7.10.235.gb2edec.dirty\n"},{"id":"190058","messageId":"20120425120036.GB21579@sigill.intra.peff.net","threadId":"30323","inReplyTo":"1335292396-8502-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH v2] config: allow ~/ and ~user/ in include.path value","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-25T12:00:36Z","receivedAt":"2012-04-25T12:00:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 24, 2012 at 08:33:16PM +0200, Matthieu Moy wrote:\n\n> Subject: Re: [PATCH v2] config: allow ~/ and ~user/ in include.path value\n\nIt seems like such an obvious choice, I'm not sure why I didn't put it\nin the initial implementation.\n\n>  config.c |   11 +++++++++--\n>  1 file changed, 9 insertions(+), 2 deletions(-)\n\nNo docs or tests. :P\n\n> -static int handle_path_include(const char *path, struct config_include_data *inc)\n> +static int handle_path_include(const char *raw_path, struct config_include_data *inc)\n>  {\n>  \tint ret = 0;\n>  \tstruct strbuf buf = STRBUF_INIT;\n> -\n> +\tint must_free_path = 1;\n> +\tchar *path = expand_user_path(raw_path);\n> +\tif (!path)\n> +\t\treturn error(\"Could not expand include path '%s'.\", raw_path);\n>  \t/*\n>  \t * Use an absolute path as-is, but interpret relative paths\n>  \t * based on the including config file.\n> @@ -52,6 +55,8 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n>  \t\tif (slash)\n>  \t\t\tstrbuf_add(&buf, cf->name, slash - cf->name + 1);\n>  \t\tstrbuf_addstr(&buf, path);\n> +\t\tfree(path);\n> +\t\tmust_free_path = 0;\n>  \t\tpath = buf.buf;\n>  \t}\n>  \n> @@ -63,6 +68,8 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n>  \t\tinc->depth--;\n>  \t}\n>  \tstrbuf_release(&buf);\n> +\tif (must_free_path)\n> +\t\tfree(path);\n\nUgh. This must_free_path business in the middle was quite confusing for\nme to read (and I think maybe for you to write, since you ended up with\na leak in the initial version).\n\nIf you just detach the strbuf buffer, then must_free_path can go away\n(you must always free it then). But I think it is even simpler to just\nkeep the allocated bits in a separate variable. Patch is below, with\ndocumentation updates and tests.\n\n-Peff\n\n-- >8 --\nSubject: [PATCH] config: expand tildes in include.path variable\n\nYou can already use relative paths in include.path, which\nmeans that including \"foo\" from your global \"~/.gitconfig\"\nwill look in your home directory. However, you might want to\ndo something clever like putting \"~/.gitconfig-foo\" in a\nspecific repository's config file.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/config.txt  |    5 ++++-\n config.c                  |    6 ++++++\n t/t1305-config-include.sh |    8 ++++++++\n 3 files changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex fb386ab..0b3f291 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -95,7 +95,9 @@ included file is expanded immediately, as if its contents had been\n found at the location of the include directive. If the value of the\n `include.path` variable is a relative path, the path is considered to be\n relative to the configuration file in which the include directive was\n-found. See below for examples.\n+found. The value of `include.path` is subject to tilde expansion: `~/`\n+is expanded to the value of `$HOME`, and `~user/` to the specified\n+user's home directory. See below for examples.\n \n Example\n ~~~~~~~\n@@ -122,6 +124,7 @@ Example\n \t[include]\n \t\tpath = /path/to/foo.inc ; include by absolute path\n \t\tpath = foo ; expand \"foo\" relative to the current file\n+\t\tpath = ~/foo ; expand \"foo\" in your $HOME directory\n \n Variables\n ~~~~~~~~~\ndiff --git a/config.c b/config.c\nindex 68d3294..2bbf02d 100644\n--- a/config.c\n+++ b/config.c\n@@ -37,6 +37,11 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n {\n \tint ret = 0;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tchar *expanded = expand_user_path(path);\n+\n+\tif (!expanded)\n+\t\treturn error(\"Could not expand include path '%s'\", path);\n+\tpath = expanded;\n \n \t/*\n \t * Use an absolute path as-is, but interpret relative paths\n@@ -63,6 +68,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n \t\tinc->depth--;\n \t}\n \tstrbuf_release(&buf);\n+\tfree(expanded);\n \treturn ret;\n }\n \ndiff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh\nindex 4b1cbaa..a707076 100755\n--- a/t/t1305-config-include.sh\n+++ b/t/t1305-config-include.sh\n@@ -29,6 +29,14 @@ test_expect_success 'chained relative paths' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'include paths get tilde-expansion' '\n+\techo \"[test]one = 1\" >one &&\n+\techo \"[include]path = ~/one\" >.gitconfig &&\n+\techo 1 >expect &&\n+\tgit config test.one >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_expect_success 'include options can still be examined' '\n \techo \"[test]one = 1\" >one &&\n \techo \"[include]path = one\" >.gitconfig &&\n-- \n1.7.9.6.11.gd9951\n"},{"id":"190063","messageId":"vpqehrbolis.fsf@bauges.imag.fr","threadId":"30323","inReplyTo":"20120425120036.GB21579@sigill.intra.peff.net","subject":"Re: [PATCH v2] config: allow ~/ and ~user/ in include.path value","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-04-25T15:53:47Z","receivedAt":"2012-04-25T15:53:47Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Jeff King <peff@peff.net> writes:\n\n>  Documentation/config.txt  |    5 ++++-\n>  t/t1305-config-include.sh |    8 ++++++++\n\nNice, thanks.\n\n> --- a/config.c\n> +++ b/config.c\n> @@ -37,6 +37,11 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n>  {\n>  \tint ret = 0;\n>  \tstruct strbuf buf = STRBUF_INIT;\n> +\tchar *expanded = expand_user_path(path);\n> +\n> +\tif (!expanded)\n> +\t\treturn error(\"Could not expand include path '%s'\", path);\n> +\tpath = expanded;\n>  \n>  \t/*\n>  \t * Use an absolute path as-is, but interpret relative paths\n> @@ -63,6 +68,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc\n>  \t\tinc->depth--;\n>  \t}\n>  \tstrbuf_release(&buf);\n> +\tfree(expanded);\n\nClearly better than my version.\n\nAcked-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"190076","messageId":"xmqqwr53bmc7.fsf@junio.mtv.corp.google.com","threadId":"30323","inReplyTo":"20120425120036.GB21579@sigill.intra.peff.net","subject":"Re: [PATCH v2] config: allow ~/ and ~user/ in include.path value","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-25T20:14:32Z","receivedAt":"2012-04-25T20:14:32Z","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> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index fb386ab..0b3f291 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -95,7 +95,9 @@ included file is expanded immediately, as if its contents had been\n>  found at the location of the include directive. If the value of the\n>  `include.path` variable is a relative path, the path is considered to be\n>  relative to the configuration file in which the include directive was\n> -found. See below for examples.\n> +found. The value of `include.path` is subject to tilde expansion: `~/`\n> +is expanded to the value of `$HOME`, and `~user/` to the specified\n> +user's home directory. See below for examples.\n>  \n>  Example\n>  ~~~~~~~\n> @@ -122,6 +124,7 @@ Example\n>  \t[include]\n>  \t\tpath = /path/to/foo.inc ; include by absolute path\n>  \t\tpath = foo ; expand \"foo\" relative to the current file\n> +\t\tpath = ~/foo ; expand \"foo\" in your $HOME directory\n\nModulo s/~/{tilde}/ in the body text (but not in the displayed example),\nlooked good to me, so I queued with two amends.\n\nThanks.\n"},{"id":"190097","messageId":"20120426061923.GA22819@sigill.intra.peff.net","threadId":"30323","inReplyTo":"xmqqwr53bmc7.fsf@junio.mtv.corp.google.com","subject":"Re: [PATCH v2] config: allow ~/ and ~user/ in include.path value","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-04-26T06:19:23Z","receivedAt":"2012-04-26T06:19:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 25, 2012 at 01:14:32PM -0700, Junio C Hamano wrote:\n\n> > +found. The value of `include.path` is subject to tilde expansion: `~/`\n> > +is expanded to the value of `$HOME`, and `~user/` to the specified\n> > +user's home directory. See below for examples.\n> >  \n> >  Example\n> >  ~~~~~~~\n> > @@ -122,6 +124,7 @@ Example\n> >  \t[include]\n> >  \t\tpath = /path/to/foo.inc ; include by absolute path\n> >  \t\tpath = foo ; expand \"foo\" relative to the current file\n> > +\t\tpath = ~/foo ; expand \"foo\" in your $HOME directory\n> \n> Modulo s/~/{tilde}/ in the body text (but not in the displayed example),\n> looked good to me, so I queued with two amends.\n\nThanks, I forgot about that.\n\nI'd like to eventually stop building the documentation with\nno-inline-literal. It was an asciidoc7 compatibility thing, but we can\nprobably drop that now. However, I suspect that would require us to\nsimultaneously convert all of the `{tilde}` uses back into `~`.\n\n-Peff\n"}]}