All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Matheus Tavares <matheus.bernardino@usp.br>
Cc: git@vger.kernel.org, ajrhunt@google.com, peff@peff.net
Subject: Re: [PATCH] init: fix bug regarding ~/ expansion in init.templateDir
Date: Tue, 25 May 2021 13:21:04 +0900	[thread overview]
Message-ID: <xmqqlf83h2a7.fsf@gitster.g> (raw)
In-Reply-To: <b079bc0288429919aca482a689ee87e70b719303.1621914058.git.matheus.bernardino@usp.br> (Matheus Tavares's message of "Tue, 25 May 2021 00:41:01 -0300")

Matheus Tavares <matheus.bernardino@usp.br> writes:

> We used to read the init.templateDir setting at builtin/init-db.c using
> a git_config() callback that, in turn, called git_config_pathname(). To
> simplify the config reading logic at this file and plug a memory leak,
> this was replaced by a direct call to git_config_get_value() at
> e4de4502e6 ("init: remove git_init_db_config() while fixing leaks",
> 2021-03-14). However, this function doesn't provide path expanding
> semantics, like git_config_pathname() does, so paths with '~/' and
> '~user/' are treated literally. This makes 'git init' fail to handle
> init.templateDir paths using these constructs:
>
> 	$ git config init.templateDir '~/templates_dir'
> 	$ git init
> 	'warning: templates not found in ~/templates_dir'
>
> Replace the git_config_get_value() call by git_config_get_pathname(),
> which does the '~/' and '~user/' expansions. Also add a regression test.
> Note that unlike git_config_get_value(), the config cache does not own
> the memory for the path returned by git_config_get_pathname(), so we
> must free() it.
>
> Reported on IRC by rkta.
>
> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
> ---

The patch looks like a clean regression fix.

> +init_no_templatedir_env () {
> +	(
> +		sane_unset GIT_TEMPLATE_DIR &&
> +		NO_SET_GIT_TEMPLATE_DIR=t &&
> +		export NO_SET_GIT_TEMPLATE_DIR &&
> +		git init "$1"

	(
		sane_unset GIT_TEMPLATE_DIR &&
		NO_SET_GIT_TEMPLATE_DIR=t git init "$1"
	)

would be a shorter way to write it, but this is inheriting from the
original that used longhand, so it is OK, I guess.  We cannot lose
the subprocess because we do not want to lose GIT_TEMPLATE_DIR in
tests that run after a test that uses this function.

OK.  We could lose the outermost {} but let's take the patch as-is.

Thanks.

> +	)
> +}
> +
>  test_expect_success 'init with init.templatedir set' '
>  	mkdir templatedir-source &&
>  	echo Content >templatedir-source/file &&
>  	test_config_global init.templatedir "${HOME}/templatedir-source" &&
> -	(
> -		mkdir templatedir-set &&
> -		cd templatedir-set &&
> -		sane_unset GIT_TEMPLATE_DIR &&
> -		NO_SET_GIT_TEMPLATE_DIR=t &&
> -		export NO_SET_GIT_TEMPLATE_DIR &&
> -		git init
> -	) &&
> +
> +	init_no_templatedir_env templatedir-set &&
>  	test_cmp templatedir-source/file templatedir-set/.git/file
>  '
>  
> +test_expect_success 'init with init.templatedir using ~ expansion' '
> +	mkdir -p templatedir-source &&
> +	echo Content >templatedir-source/file &&
> +	test_config_global init.templatedir "~/templatedir-source" &&
> +
> +	init_no_templatedir_env templatedir-expansion &&
> +	test_cmp templatedir-source/file templatedir-expansion/.git/file
> +'
> +
>  test_expect_success 'init --bare/--shared overrides system/global config' '
>  	test_config_global core.bare false &&
>  	test_config_global core.sharedRepository 0640 &&

      reply	other threads:[~2021-05-25  4:21 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-25  3:41 [PATCH] init: fix bug regarding ~/ expansion in init.templateDir Matheus Tavares
2021-05-25  4:21 ` Junio C Hamano [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqlf83h2a7.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=ajrhunt@google.com \
    --cc=git@vger.kernel.org \
    --cc=matheus.bernardino@usp.br \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.