From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
"Torsten Bögershausen" <tboegi@web.de>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH] config: match both symlink & realpath versions in IncludeIf.gitdir:*
Date: Mon, 15 May 2017 19:15:52 +0000 [thread overview]
Message-ID: <20170515191552.5193-1-avarab@gmail.com> (raw)
In-Reply-To: <CACBZZX5nchNUb-V3U8FL9fuhk=3t42Qhz4=Wh9Qk_0C=x85UdQ@mail.gmail.com>
Change the conditional inclusion mechanism to support
e.g. gitdir:~/git_tree/repo where ~/git_tree is a symlink to to
/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.
+
* Note that "../" is not special and will match literally, which is
unlikely what you want.
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;
if (opts->git_dir)
git_dir = opts->git_dir;
@@ -226,6 +227,7 @@ static int include_by_gitdir(const struct config_options *opts,
strbuf_add(&pattern, cond, cond_len);
prefix = prepare_include_condition_pattern(&pattern);
+again:
if (prefix < 0)
goto done;
@@ -245,6 +247,20 @@ static int include_by_gitdir(const struct config_options *opts,
ret = !wildmatch(pattern.buf + prefix, text.buf + prefix,
icase ? WM_CASEFOLD : 0, NULL);
+ if (!ret && retry) {
+ /*
+ * We've tried e.g. matching gitdir:~/work, but if
+ * ~/work is a symlink to /mnt/storage/work
+ * strbuf_realpath() will expand it, so the rule won't
+ * match. Let's match against a
+ * strbuf_add_absolute_path() version of the path,
+ * which'll do the right thing
+ */
+ strbuf_reset(&text);
+ strbuf_add_absolute_path(&text, git_dir);
+ retry = 0;
+ goto again;
+ }
done:
strbuf_release(&pattern);
strbuf_release(&text);
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
index 933915ec06..d9d2f545a4 100755
--- a/t/t1305-config-include.sh
+++ b/t/t1305-config-include.sh
@@ -273,6 +273,29 @@ test_expect_success SYMLINKS 'conditional include, relative path with symlinks'
)
'
+test_expect_success SYMLINKS 'conditional include, gitdir matching symlink' '
+ ln -s foo bar &&
+ (
+ cd bar &&
+ echo "[includeIf \"gitdir:bar/\"]path=bar7" >>.git/config &&
+ echo "[test]seven=7" >.git/bar7 &&
+ echo 7 >expect &&
+ git config test.seven >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success SYMLINKS 'conditional include, gitdir matching symlink, icase' '
+ (
+ cd bar &&
+ echo "[includeIf \"gitdir/i:BAR/\"]path=bar8" >>.git/config &&
+ echo "[test]eight=8" >.git/bar8 &&
+ echo 8 >expect &&
+ git config test.eight >actual &&
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'include cycles are detected' '
cat >.gitconfig <<-\EOF &&
[test]value = gitconfig
--
2.13.0.rc2.291.g57267f2277
next prev parent reply other threads:[~2017-05-15 19:16 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-15 15:20 [PATCH/RFC] The new IncludeIf facility doesn't DWIM when the repo is symlinked Ævar Arnfjörð Bjarmason
2017-05-15 18:30 ` Ævar Arnfjörð Bjarmason
2017-05-15 19:15 ` Ævar Arnfjörð Bjarmason [this message]
2017-05-16 1:15 ` [PATCH] config: match both symlink & realpath versions in IncludeIf.gitdir:* Junio C Hamano
2017-05-16 8:28 ` [PATCH v2] " Ævar Arnfjörð Bjarmason
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=20170515191552.5193-1-avarab@gmail.com \
--to=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=pclouds@gmail.com \
--cc=tboegi@web.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).