{"thread":{"id":"55767","subject":"[PATCH] init: fix bug regarding ~/ expansion in init.templateDir","startedAt":"2021-05-25T03:41:13Z","lastAt":"2021-05-25T04:21:12Z","messageCount":2,"participants":["Matheus Tavares","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"425481","messageId":"b079bc0288429919aca482a689ee87e70b719303.1621914058.git.matheus.bernardino@usp.br","threadId":"55767","inReplyTo":null,"subject":"[PATCH] init: fix bug regarding ~/ expansion in init.templateDir","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2021-05-25T03:41:01Z","receivedAt":"2021-05-25T03:41:13Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"We used to read the init.templateDir setting at builtin/init-db.c using\na git_config() callback that, in turn, called git_config_pathname(). To\nsimplify the config reading logic at this file and plug a memory leak,\nthis was replaced by a direct call to git_config_get_value() at\ne4de4502e6 (\"init: remove git_init_db_config() while fixing leaks\",\n2021-03-14). However, this function doesn't provide path expanding\nsemantics, like git_config_pathname() does, so paths with '~/' and\n'~user/' are treated literally. This makes 'git init' fail to handle\ninit.templateDir paths using these constructs:\n\n\t$ git config init.templateDir '~/templates_dir'\n\t$ git init\n\t'warning: templates not found in ~/templates_dir'\n\nReplace the git_config_get_value() call by git_config_get_pathname(),\nwhich does the '~/' and '~user/' expansions. Also add a regression test.\nNote that unlike git_config_get_value(), the config cache does not own\nthe memory for the path returned by git_config_get_pathname(), so we\nmust free() it.\n\nReported on IRC by rkta.\n\nSigned-off-by: Matheus Tavares <matheus.bernardino@usp.br>\n---\n builtin/init-db.c |  3 ++-\n t/t0001-init.sh   | 28 ++++++++++++++++++++--------\n 2 files changed, 22 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex c19b35f1e6..2167796ff2 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -212,8 +212,9 @@ static int create_default_files(const char *template_path,\n \t * values (since we've just potentially changed what's available on\n \t * disk).\n \t */\n-\tgit_config_get_value(\"init.templatedir\", &init_template_dir);\n+\tgit_config_get_pathname(\"init.templatedir\", &init_template_dir);\n \tcopy_templates(template_path, init_template_dir);\n+\tfree((char *)init_template_dir);\n \tgit_config_clear();\n \treset_shared_repository();\n \tgit_config(git_default_config, NULL);\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex 0803994874..acd662e403 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -186,21 +186,33 @@ test_expect_success 'init with --template (blank)' '\n \ttest_path_is_missing template-blank/.git/info/exclude\n '\n \n+init_no_templatedir_env () {\n+\t(\n+\t\tsane_unset GIT_TEMPLATE_DIR &&\n+\t\tNO_SET_GIT_TEMPLATE_DIR=t &&\n+\t\texport NO_SET_GIT_TEMPLATE_DIR &&\n+\t\tgit init \"$1\"\n+\t)\n+}\n+\n test_expect_success 'init with init.templatedir set' '\n \tmkdir templatedir-source &&\n \techo Content >templatedir-source/file &&\n \ttest_config_global init.templatedir \"${HOME}/templatedir-source\" &&\n-\t(\n-\t\tmkdir templatedir-set &&\n-\t\tcd templatedir-set &&\n-\t\tsane_unset GIT_TEMPLATE_DIR &&\n-\t\tNO_SET_GIT_TEMPLATE_DIR=t &&\n-\t\texport NO_SET_GIT_TEMPLATE_DIR &&\n-\t\tgit init\n-\t) &&\n+\n+\tinit_no_templatedir_env templatedir-set &&\n \ttest_cmp templatedir-source/file templatedir-set/.git/file\n '\n \n+test_expect_success 'init with init.templatedir using ~ expansion' '\n+\tmkdir -p templatedir-source &&\n+\techo Content >templatedir-source/file &&\n+\ttest_config_global init.templatedir \"~/templatedir-source\" &&\n+\n+\tinit_no_templatedir_env templatedir-expansion &&\n+\ttest_cmp templatedir-source/file templatedir-expansion/.git/file\n+'\n+\n test_expect_success 'init --bare/--shared overrides system/global config' '\n \ttest_config_global core.bare false &&\n \ttest_config_global core.sharedRepository 0640 &&\n-- \n2.31.1\n\n"},{"id":"425483","messageId":"xmqqlf83h2a7.fsf@gitster.g","threadId":"55767","inReplyTo":"b079bc0288429919aca482a689ee87e70b719303.1621914058.git.matheus.bernardino@usp.br","subject":"Re: [PATCH] init: fix bug regarding ~/ expansion in init.templateDir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-25T04:21:04Z","receivedAt":"2021-05-25T04:21:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matheus Tavares <matheus.bernardino@usp.br> writes:\n\n> We used to read the init.templateDir setting at builtin/init-db.c using\n> a git_config() callback that, in turn, called git_config_pathname(). To\n> simplify the config reading logic at this file and plug a memory leak,\n> this was replaced by a direct call to git_config_get_value() at\n> e4de4502e6 (\"init: remove git_init_db_config() while fixing leaks\",\n> 2021-03-14). However, this function doesn't provide path expanding\n> semantics, like git_config_pathname() does, so paths with '~/' and\n> '~user/' are treated literally. This makes 'git init' fail to handle\n> init.templateDir paths using these constructs:\n>\n> \t$ git config init.templateDir '~/templates_dir'\n> \t$ git init\n> \t'warning: templates not found in ~/templates_dir'\n>\n> Replace the git_config_get_value() call by git_config_get_pathname(),\n> which does the '~/' and '~user/' expansions. Also add a regression test.\n> Note that unlike git_config_get_value(), the config cache does not own\n> the memory for the path returned by git_config_get_pathname(), so we\n> must free() it.\n>\n> Reported on IRC by rkta.\n>\n> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>\n> ---\n\nThe patch looks like a clean regression fix.\n\n> +init_no_templatedir_env () {\n> +\t(\n> +\t\tsane_unset GIT_TEMPLATE_DIR &&\n> +\t\tNO_SET_GIT_TEMPLATE_DIR=t &&\n> +\t\texport NO_SET_GIT_TEMPLATE_DIR &&\n> +\t\tgit init \"$1\"\n\n\t(\n\t\tsane_unset GIT_TEMPLATE_DIR &&\n\t\tNO_SET_GIT_TEMPLATE_DIR=t git init \"$1\"\n\t)\n\nwould be a shorter way to write it, but this is inheriting from the\noriginal that used longhand, so it is OK, I guess.  We cannot lose\nthe subprocess because we do not want to lose GIT_TEMPLATE_DIR in\ntests that run after a test that uses this function.\n\nOK.  We could lose the outermost {} but let's take the patch as-is.\n\nThanks.\n\n> +\t)\n> +}\n> +\n>  test_expect_success 'init with init.templatedir set' '\n>  \tmkdir templatedir-source &&\n>  \techo Content >templatedir-source/file &&\n>  \ttest_config_global init.templatedir \"${HOME}/templatedir-source\" &&\n> -\t(\n> -\t\tmkdir templatedir-set &&\n> -\t\tcd templatedir-set &&\n> -\t\tsane_unset GIT_TEMPLATE_DIR &&\n> -\t\tNO_SET_GIT_TEMPLATE_DIR=t &&\n> -\t\texport NO_SET_GIT_TEMPLATE_DIR &&\n> -\t\tgit init\n> -\t) &&\n> +\n> +\tinit_no_templatedir_env templatedir-set &&\n>  \ttest_cmp templatedir-source/file templatedir-set/.git/file\n>  '\n>  \n> +test_expect_success 'init with init.templatedir using ~ expansion' '\n> +\tmkdir -p templatedir-source &&\n> +\techo Content >templatedir-source/file &&\n> +\ttest_config_global init.templatedir \"~/templatedir-source\" &&\n> +\n> +\tinit_no_templatedir_env templatedir-expansion &&\n> +\ttest_cmp templatedir-source/file templatedir-expansion/.git/file\n> +'\n> +\n>  test_expect_success 'init --bare/--shared overrides system/global config' '\n>  \ttest_config_global core.bare false &&\n>  \ttest_config_global core.sharedRepository 0640 &&\n"}]}