Re: [PATCH] config: match both symlink & realpath versions in IncludeIf.gitdir:*
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 16, 2017, 01:15 UTC
- Message-ID
- <xmqqlgpx4n5v.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170515191552.5193-1-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> Change the conditional inclusion mechanism to support > e.g. gitdir:~/git_tree/repo where ~/git_tree is a symlink to to
s/to to/to/;
Show 47 quoted lines
> /mnt/stuff/repo.
>
> This worked in the initial version of this facility[1], but regressed
> later in the series while solving a related bug[2].
>
> Now gitdir: will match against the symlinked
> path (e.g. gitdir:~/git_tree/repo) in addition to the current
> /mnt/stuff/repo path.
>
> Since this is already in a release version note in the documentation
> that this behavior changed, so users who expect their configuration to
> work on both v2.13.0 and some future version of git with this fix
> aren't utterly confused.
>
> 1. commit 3efd0bedc6 ("config: add conditional include", 2017-03-01)
> 2. commit 86f9515708 ("config: resolve symlinks in conditional
> include's patterns", 2017-04-05)
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>
> Here's a non-RFC patch to fix this bug.
>
> Documentation/config.txt | 11 +++++++++++
> config.c | 16 ++++++++++++++++
> t/t1305-config-include.sh | 23 +++++++++++++++++++++++
> 3 files changed, 50 insertions(+)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 475e874d51..137502a289 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -140,6 +140,17 @@ A few more notes on matching via `gitdir` and `gitdir/i`:
>
> * Symlinks in `$GIT_DIR` are not resolved before matching.
>
> + * Both the symlink & realpath versions of paths will be matched
> + outside of `$GIT_DIR`. E.g. if ~/git is a symlink to
> + /mnt/storage/git, both `gitdir:~/git` and `gitdir:/mnt/storage/git`
> + will match.
> +
> + This was not the case in the initial release of this feature in
> + v2.13.0, which only matched the realpath version. Configuration
> + that wants to be compatible with the initial release of this
> + feature needs to either specify only the realpath version, or both
> + versions.
> +Does the second paragraph format correctly, or do we need the usual (and ugly) "+ by itself on a line and then dedent the next paragraph" trick?
Show 9 quoted lines
> diff --git a/config.c b/config.c > index b4a3205da3..0498746112 100644 > --- a/config.c > +++ b/config.c > @@ -214,6 +214,7 @@ static int include_by_gitdir(const struct config_options *opts, > struct strbuf pattern = STRBUF_INIT; > int ret = 0, prefix; > const char *git_dir; > + int retry = 1;
I think your earlier RFC used a variable with a better name than this; it is not like we are preparing ourselves for adding a mechanism here to retry for many random reasons.
The logic would be clearer With an "already_tried_absolute" variable that is initially 0, retrying unless it is already set, and set it when we retry, like you did in your RFC.
Thanks.