* t1503 broken ?
@ 2017-03-25 10:46 Torsten Bögershausen
2017-03-25 11:46 ` Duy Nguyen
0 siblings, 1 reply; 11+ messages in thread
From: Torsten Bögershausen @ 2017-03-25 10:46 UTC (permalink / raw)
To: Git Mailing List, Nguyen Thai Ngoc Duy
./t1305-config-include.sh
seems to be broken:
not ok 19 - conditional include, $HOME expansion
not ok 21 - conditional include, relative path
Both Mac and Linux.
The problem seems to be the line
git config test.two >actual &&
and
git config test.four >actual &&
In both cases the config "test.two or test.four" is not found,
at least that is what my debugging shows.
After adding an
exit 0
before the test_done gives this:
find trash\ directory.t1305-config-include/ -name "bar*"
trash directory.t1305-config-include/foo/.git/bar
trash directory.t1305-config-include/foo/.git/bar3
trash directory.t1305-config-include/foo/.git/bar5
trash directory.t1305-config-include/foo/.git/bar2
trash directory.t1305-config-include/bar4
Where should bar2 and bar4 be ?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: t1503 broken ?
2017-03-25 10:46 t1503 broken ? Torsten Bögershausen
@ 2017-03-25 11:46 ` Duy Nguyen
2017-03-25 12:26 ` Duy Nguyen
0 siblings, 1 reply; 11+ messages in thread
From: Duy Nguyen @ 2017-03-25 11:46 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Git Mailing List
On Sat, Mar 25, 2017 at 5:46 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> ./t1305-config-include.sh
> seems to be broken:
> not ok 19 - conditional include, $HOME expansion
> not ok 21 - conditional include, relative path
let me guess, your "git" directory is in a symlink path?
--
Duy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: t1503 broken ?
2017-03-25 11:46 ` Duy Nguyen
@ 2017-03-25 12:26 ` Duy Nguyen
2017-03-25 13:05 ` Duy Nguyen
0 siblings, 1 reply; 11+ messages in thread
From: Duy Nguyen @ 2017-03-25 12:26 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Git Mailing List
On Sat, Mar 25, 2017 at 6:46 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Sat, Mar 25, 2017 at 5:46 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>> ./t1305-config-include.sh
>> seems to be broken:
>> not ok 19 - conditional include, $HOME expansion
>> not ok 21 - conditional include, relative path
>
> let me guess, your "git" directory is in a symlink path?
Yes I could reproduce it when I put my "git" in a symlink. There's a
note in document about "Symlinks in `$GIT_DIR` are not resolved before
matching" but failing tests is not acceptable. I'll fix it.
--
Duy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: t1503 broken ?
2017-03-25 12:26 ` Duy Nguyen
@ 2017-03-25 13:05 ` Duy Nguyen
2017-03-25 19:41 ` Torsten Bögershausen
2017-03-30 11:37 ` [PATCH 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy
0 siblings, 2 replies; 11+ messages in thread
From: Duy Nguyen @ 2017-03-25 13:05 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Git Mailing List
On Sat, Mar 25, 2017 at 07:26:14PM +0700, Duy Nguyen wrote:
> On Sat, Mar 25, 2017 at 6:46 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> > On Sat, Mar 25, 2017 at 5:46 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> >> ./t1305-config-include.sh
> >> seems to be broken:
> >> not ok 19 - conditional include, $HOME expansion
> >> not ok 21 - conditional include, relative path
> >
> > let me guess, your "git" directory is in a symlink path?
>
> Yes I could reproduce it when I put my "git" in a symlink. There's a
> note in document about "Symlinks in `$GIT_DIR` are not resolved before
> matching" but failing tests is not acceptable. I'll fix it.
The fix may be something like this. The problem is $GIT_DIR has symlinks
resolved, but we don't do the same for other paths in this code. As a
result, matching paths fails.
I'm a bit concerned about the change in expand_user_path() because I'm
not quite sure if it's a completely safe change. But at least could
you try the patch and see if it passe the tests on your machine too?
-- 8< --
diff --git a/config.c b/config.c
index 1a4d855..fc4eae9 100644
--- a/config.c
+++ b/config.c
@@ -191,7 +191,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat)
return error(_("relative config include "
"conditionals must come from files"));
- strbuf_add_absolute_path(&path, cf->path);
+ strbuf_realpath(&path, cf->path, 1);
slash = find_last_dir_sep(path.buf);
if (!slash)
die("BUG: how is this possible?");
@@ -213,7 +213,7 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase)
struct strbuf pattern = STRBUF_INIT;
int ret = 0, prefix;
- strbuf_add_absolute_path(&text, get_git_dir());
+ strbuf_realpath(&text, get_git_dir(), 1);
strbuf_add(&pattern, cond, cond_len);
prefix = prepare_include_condition_pattern(&pattern);
diff --git a/path.c b/path.c
index 2224843..18eaac3 100644
--- a/path.c
+++ b/path.c
@@ -654,7 +654,7 @@ char *expand_user_path(const char *path)
const char *home = getenv("HOME");
if (!home)
goto return_null;
- strbuf_addstr(&user_path, home);
+ strbuf_addstr(&user_path, real_path(home));
#ifdef GIT_WINDOWS_NATIVE
convert_slashes(user_path.buf);
#endif
-- 8< --
--
Duy
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: t1503 broken ?
2017-03-25 13:05 ` Duy Nguyen
@ 2017-03-25 19:41 ` Torsten Bögershausen
2017-03-30 11:37 ` [PATCH 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy
1 sibling, 0 replies; 11+ messages in thread
From: Torsten Bögershausen @ 2017-03-25 19:41 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
On 03/25/2017 02:05 PM, Duy Nguyen wrote:
> On Sat, Mar 25, 2017 at 07:26:14PM +0700, Duy Nguyen wrote:
>> On Sat, Mar 25, 2017 at 6:46 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>>> On Sat, Mar 25, 2017 at 5:46 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>>>> ./t1305-config-include.sh
>>>> seems to be broken:
>>>> not ok 19 - conditional include, $HOME expansion
>>>> not ok 21 - conditional include, relative path
>>> let me guess, your "git" directory is in a symlink path?
>> Yes I could reproduce it when I put my "git" in a symlink. There's a
>> note in document about "Symlinks in `$GIT_DIR` are not resolved before
>> matching" but failing tests is not acceptable. I'll fix it.
> The fix may be something like this. The problem is $GIT_DIR has symlinks
> resolved, but we don't do the same for other paths in this code. As a
> result, matching paths fails.
>
> I'm a bit concerned about the change in expand_user_path() because I'm
> not quite sure if it's a completely safe change. But at least could
> you try the patch and see if it passe the tests on your machine too?
>
> -- 8< --
> diff --git a/config.c b/config.c
> index 1a4d855..fc4eae9 100644
> --- a/config.c
> +++ b/config.c
> @@ -191,7 +191,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat)
> return error(_("relative config include "
> "conditionals must come from files"));
>
> - strbuf_add_absolute_path(&path, cf->path);
> + strbuf_realpath(&path, cf->path, 1);
> slash = find_last_dir_sep(path.buf);
> if (!slash)
> die("BUG: how is this possible?");
> @@ -213,7 +213,7 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase)
> struct strbuf pattern = STRBUF_INIT;
> int ret = 0, prefix;
>
> - strbuf_add_absolute_path(&text, get_git_dir());
> + strbuf_realpath(&text, get_git_dir(), 1);
> strbuf_add(&pattern, cond, cond_len);
> prefix = prepare_include_condition_pattern(&pattern);
>
> diff --git a/path.c b/path.c
> index 2224843..18eaac3 100644
> --- a/path.c
> +++ b/path.c
> @@ -654,7 +654,7 @@ char *expand_user_path(const char *path)
> const char *home = getenv("HOME");
> if (!home)
> goto return_null;
> - strbuf_addstr(&user_path, home);
> + strbuf_addstr(&user_path, real_path(home));
> #ifdef GIT_WINDOWS_NATIVE
> convert_slashes(user_path.buf);
> #endif
> -- 8< --
Thanks for the fast reply.
Yes, my path is a softlink - into a directory under NoBackup/ - to make
the backup shorter.
And I had forgotten about this :-(
And yes, your patch fixes it- tested under Linux.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] path.c: and an option to call real_path() in expand_user_path()
2017-03-25 13:05 ` Duy Nguyen
2017-03-25 19:41 ` Torsten Bögershausen
@ 2017-03-30 11:37 ` Nguyễn Thái Ngọc Duy
2017-03-30 11:37 ` [PATCH 2/2] config: resolve symlinks in conditional include's patterns Nguyễn Thái Ngọc Duy
2017-04-05 10:24 ` [PATCH v2 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy
1 sibling, 2 replies; 11+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2017-03-30 11:37 UTC (permalink / raw)
To: git; +Cc: tboegi, Junio C Hamano, Nguyễn Thái Ngọc Duy
In the next patch we need the ability to expand '~' to
real_path($HOME). But we can't do that from outside because '~' is part
of a pattern, not a true path. Add an option to expand_user_path() to do
so.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/commit.c | 2 +-
builtin/config.c | 2 +-
cache.h | 2 +-
config.c | 8 ++++----
credential-cache.c | 2 +-
credential-store.c | 2 +-
path.c | 11 ++++++++---
7 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 4e288bc513..ad188fea9e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1404,7 +1404,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
static const char *implicit_ident_advice(void)
{
- char *user_config = expand_user_path("~/.gitconfig");
+ char *user_config = expand_user_path("~/.gitconfig", 0);
char *xdg_config = xdg_config_home("config");
int config_exists = file_exists(user_config) || file_exists(xdg_config);
diff --git a/builtin/config.c b/builtin/config.c
index 05843a0f96..70bfaaaa1d 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -502,7 +502,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
}
if (use_global_config) {
- char *user_config = expand_user_path("~/.gitconfig");
+ char *user_config = expand_user_path("~/.gitconfig", 0);
char *xdg_config = xdg_config_home("config");
if (!user_config)
diff --git a/cache.h b/cache.h
index 2214d52f61..62e44bfa2f 100644
--- a/cache.h
+++ b/cache.h
@@ -1146,7 +1146,7 @@ typedef int create_file_fn(const char *path, void *cb);
int raceproof_create_file(const char *path, create_file_fn fn, void *cb);
int mkdir_in_gitdir(const char *path);
-extern char *expand_user_path(const char *path);
+extern char *expand_user_path(const char *path, int real_home);
const char *enter_repo(const char *path, int strict);
static inline int is_absolute_path(const char *path)
{
diff --git a/config.c b/config.c
index 1a4d85537b..f036c721e6 100644
--- a/config.c
+++ b/config.c
@@ -135,7 +135,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc
if (!path)
return config_error_nonbool("include.path");
- expanded = expand_user_path(path);
+ expanded = expand_user_path(path, 0);
if (!expanded)
return error("could not expand include path '%s'", path);
path = expanded;
@@ -177,7 +177,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat)
char *expanded;
int prefix = 0;
- expanded = expand_user_path(pat->buf);
+ expanded = expand_user_path(pat->buf, 0);
if (expanded) {
strbuf_reset(pat);
strbuf_addstr(pat, expanded);
@@ -948,7 +948,7 @@ int git_config_pathname(const char **dest, const char *var, const char *value)
{
if (!value)
return config_error_nonbool(var);
- *dest = expand_user_path(value);
+ *dest = expand_user_path(value, 0);
if (!*dest)
die(_("failed to expand user dir in: '%s'"), value);
return 0;
@@ -1498,7 +1498,7 @@ static int do_git_config_sequence(config_fn_t fn, void *data)
{
int ret = 0;
char *xdg_config = xdg_config_home("config");
- char *user_config = expand_user_path("~/.gitconfig");
+ char *user_config = expand_user_path("~/.gitconfig", 0);
char *repo_config = have_git_dir() ? git_pathdup("config") : NULL;
current_parsing_scope = CONFIG_SCOPE_SYSTEM;
diff --git a/credential-cache.c b/credential-cache.c
index 3cbd420019..91550bfb0b 100644
--- a/credential-cache.c
+++ b/credential-cache.c
@@ -87,7 +87,7 @@ static char *get_socket_path(void)
{
struct stat sb;
char *old_dir, *socket;
- old_dir = expand_user_path("~/.git-credential-cache");
+ old_dir = expand_user_path("~/.git-credential-cache", 0);
if (old_dir && !stat(old_dir, &sb) && S_ISDIR(sb.st_mode))
socket = xstrfmt("%s/socket", old_dir);
else
diff --git a/credential-store.c b/credential-store.c
index 55ca1b1334..ac295420dd 100644
--- a/credential-store.c
+++ b/credential-store.c
@@ -168,7 +168,7 @@ int cmd_main(int argc, const char **argv)
if (file) {
string_list_append(&fns, file);
} else {
- if ((file = expand_user_path("~/.git-credentials")))
+ if ((file = expand_user_path("~/.git-credentials", 0)))
string_list_append_nodup(&fns, file);
file = xdg_config_home("credentials");
if (file)
diff --git a/path.c b/path.c
index 22248436bf..010c565512 100644
--- a/path.c
+++ b/path.c
@@ -638,8 +638,10 @@ static struct passwd *getpw_str(const char *username, size_t len)
* Return a string with ~ and ~user expanded via getpw*. If buf != NULL,
* then it is a newly allocated string. Returns NULL on getpw failure or
* if path is NULL.
+ *
+ * If real_home is true, real_path($HOME) is used in the expansion.
*/
-char *expand_user_path(const char *path)
+char *expand_user_path(const char *path, int real_home)
{
struct strbuf user_path = STRBUF_INIT;
const char *to_copy = path;
@@ -654,7 +656,10 @@ char *expand_user_path(const char *path)
const char *home = getenv("HOME");
if (!home)
goto return_null;
- strbuf_addstr(&user_path, home);
+ if (real_home)
+ strbuf_addstr(&user_path, real_path(home));
+ else
+ strbuf_addstr(&user_path, home);
#ifdef GIT_WINDOWS_NATIVE
convert_slashes(user_path.buf);
#endif
@@ -723,7 +728,7 @@ const char *enter_repo(const char *path, int strict)
strbuf_add(&validated_path, path, len);
if (used_path.buf[0] == '~') {
- char *newpath = expand_user_path(used_path.buf);
+ char *newpath = expand_user_path(used_path.buf, 0);
if (!newpath)
return NULL;
strbuf_attach(&used_path, newpath, strlen(newpath),
--
2.11.0.157.gd943d85
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] config: resolve symlinks in conditional include's patterns
2017-03-30 11:37 ` [PATCH 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy
@ 2017-03-30 11:37 ` Nguyễn Thái Ngọc Duy
2017-03-30 18:38 ` Junio C Hamano
2017-04-05 10:24 ` [PATCH v2 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy
1 sibling, 1 reply; 11+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2017-03-30 11:37 UTC (permalink / raw)
To: git; +Cc: tboegi, Junio C Hamano, Nguyễn Thái Ngọc Duy
$GIT_DIR returned by get_git_dir() is normalized, with all symlinks
resolved (see setup_work_tree function). In order to match paths (or
patterns) against $GIT_DIR char-by-char, they have to be normalized
too. There is a note in config.txt about this, that the user need to
resolve symlinks by themselves if needed.
The problem is, we allow certain path expansion, '~/' and './', for
convenience and can't ask the user to resolve symlinks in these
expansions. Make sure the expanded paths have all symlinks resolved.
PS. The strbuf_realpath(&text, get_git_dir(), 1) is still needed because
get_git_dir() may return relative path.
Noticed-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
config.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/config.c b/config.c
index f036c721e6..d5ba848b65 100644
--- a/config.c
+++ b/config.c
@@ -177,7 +177,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat)
char *expanded;
int prefix = 0;
- expanded = expand_user_path(pat->buf, 0);
+ expanded = expand_user_path(pat->buf, 1);
if (expanded) {
strbuf_reset(pat);
strbuf_addstr(pat, expanded);
@@ -191,7 +191,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat)
return error(_("relative config include "
"conditionals must come from files"));
- strbuf_add_absolute_path(&path, cf->path);
+ strbuf_realpath(&path, cf->path, 1);
slash = find_last_dir_sep(path.buf);
if (!slash)
die("BUG: how is this possible?");
@@ -213,7 +213,7 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase)
struct strbuf pattern = STRBUF_INIT;
int ret = 0, prefix;
- strbuf_add_absolute_path(&text, get_git_dir());
+ strbuf_realpath(&text, get_git_dir(), 1);
strbuf_add(&pattern, cond, cond_len);
prefix = prepare_include_condition_pattern(&pattern);
--
2.11.0.157.gd943d85
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] config: resolve symlinks in conditional include's patterns
2017-03-30 11:37 ` [PATCH 2/2] config: resolve symlinks in conditional include's patterns Nguyễn Thái Ngọc Duy
@ 2017-03-30 18:38 ` Junio C Hamano
2017-04-04 10:12 ` Duy Nguyen
0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2017-03-30 18:38 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, tboegi
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> $GIT_DIR returned by get_git_dir() is normalized, with all symlinks
> resolved (see setup_work_tree function). In order to match paths (or
> patterns) against $GIT_DIR char-by-char, they have to be normalized
> too. There is a note in config.txt about this, that the user need to
> resolve symlinks by themselves if needed.
>
> The problem is, we allow certain path expansion, '~/' and './', for
> convenience and can't ask the user to resolve symlinks in these
> expansions. Make sure the expanded paths have all symlinks resolved.
That sounds sensible but I fail to see why 1/2 is the right approach
to do this, and I must be missing something. Wouldn't you get the
same result if you run realpath() yourself on expanded, after
receiving the return value of expand_user_path() in it?
Can you add a test to demonstrate the issue (which would need to be
protected with SYMLINKS prereq)?
Thanks.
> PS. The strbuf_realpath(&text, get_git_dir(), 1) is still needed because
> get_git_dir() may return relative path.
>
> Noticed-by: Torsten Bögershausen <tboegi@web.de>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> config.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/config.c b/config.c
> index f036c721e6..d5ba848b65 100644
> --- a/config.c
> +++ b/config.c
> @@ -177,7 +177,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat)
> char *expanded;
> int prefix = 0;
>
> - expanded = expand_user_path(pat->buf, 0);
> + expanded = expand_user_path(pat->buf, 1);
> if (expanded) {
> strbuf_reset(pat);
> strbuf_addstr(pat, expanded);
> @@ -191,7 +191,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat)
> return error(_("relative config include "
> "conditionals must come from files"));
>
> - strbuf_add_absolute_path(&path, cf->path);
> + strbuf_realpath(&path, cf->path, 1);
> slash = find_last_dir_sep(path.buf);
> if (!slash)
> die("BUG: how is this possible?");
> @@ -213,7 +213,7 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase)
> struct strbuf pattern = STRBUF_INIT;
> int ret = 0, prefix;
>
> - strbuf_add_absolute_path(&text, get_git_dir());
> + strbuf_realpath(&text, get_git_dir(), 1);
> strbuf_add(&pattern, cond, cond_len);
> prefix = prepare_include_condition_pattern(&pattern);
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] config: resolve symlinks in conditional include's patterns
2017-03-30 18:38 ` Junio C Hamano
@ 2017-04-04 10:12 ` Duy Nguyen
0 siblings, 0 replies; 11+ messages in thread
From: Duy Nguyen @ 2017-04-04 10:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Torsten Bögershausen
On Fri, Mar 31, 2017 at 1:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> $GIT_DIR returned by get_git_dir() is normalized, with all symlinks
>> resolved (see setup_work_tree function). In order to match paths (or
>> patterns) against $GIT_DIR char-by-char, they have to be normalized
>> too. There is a note in config.txt about this, that the user need to
>> resolve symlinks by themselves if needed.
>>
>> The problem is, we allow certain path expansion, '~/' and './', for
>> convenience and can't ask the user to resolve symlinks in these
>> expansions. Make sure the expanded paths have all symlinks resolved.
>
> That sounds sensible but I fail to see why 1/2 is the right approach
> to do this, and I must be missing something. Wouldn't you get the
> same result if you run realpath() yourself on expanded, after
> receiving the return value of expand_user_path() in it?
Because at that point I don't know what part is $HOME (i.e. valid path
that real_path can walk through), what part is random wildcards from
the pattern. Note that in this case we pass a wildmatch pattern to
expand_user_path(), like ~/[ab]foo/*bar*/**. After expansion it
becomes /home/pclouds/[ab]foo/*bar*/**. It does not feel right to let
real_path() walk the "[ab]foo..." part. In the tests, I hit
die("Invalid path") in strbuf_realpath(). Even if I set die_on_error()
to avoid that, strbuf_realpath() will not return the resolved path
> Can you add a test to demonstrate the issue (which would need to be
> protected with SYMLINKS prereq)?
Will do. It may look a bit ugly though because I have to force
setup_git_directory() to call real_path() because it doesn't always do
that.
--
Duy
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] path.c: and an option to call real_path() in expand_user_path()
2017-03-30 11:37 ` [PATCH 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy
2017-03-30 11:37 ` [PATCH 2/2] config: resolve symlinks in conditional include's patterns Nguyễn Thái Ngọc Duy
@ 2017-04-05 10:24 ` Nguyễn Thái Ngọc Duy
2017-04-05 10:24 ` [PATCH v2 2/2] config: resolve symlinks in conditional include's patterns Nguyễn Thái Ngọc Duy
1 sibling, 1 reply; 11+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2017-04-05 10:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, tboegi, Nguyễn Thái Ngọc Duy
In the next patch we need the ability to expand '~' to
real_path($HOME). But we can't do that from outside because '~' is part
of a pattern, not a true path. Add an option to expand_user_path() to do
so.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
No changes in v2.
builtin/commit.c | 2 +-
builtin/config.c | 2 +-
cache.h | 2 +-
config.c | 8 ++++----
credential-cache.c | 2 +-
credential-store.c | 2 +-
path.c | 11 ++++++++---
7 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 4e288bc513..ad188fea9e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1404,7 +1404,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
static const char *implicit_ident_advice(void)
{
- char *user_config = expand_user_path("~/.gitconfig");
+ char *user_config = expand_user_path("~/.gitconfig", 0);
char *xdg_config = xdg_config_home("config");
int config_exists = file_exists(user_config) || file_exists(xdg_config);
diff --git a/builtin/config.c b/builtin/config.c
index 05843a0f96..70bfaaaa1d 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -502,7 +502,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
}
if (use_global_config) {
- char *user_config = expand_user_path("~/.gitconfig");
+ char *user_config = expand_user_path("~/.gitconfig", 0);
char *xdg_config = xdg_config_home("config");
if (!user_config)
diff --git a/cache.h b/cache.h
index 2214d52f61..62e44bfa2f 100644
--- a/cache.h
+++ b/cache.h
@@ -1146,7 +1146,7 @@ typedef int create_file_fn(const char *path, void *cb);
int raceproof_create_file(const char *path, create_file_fn fn, void *cb);
int mkdir_in_gitdir(const char *path);
-extern char *expand_user_path(const char *path);
+extern char *expand_user_path(const char *path, int real_home);
const char *enter_repo(const char *path, int strict);
static inline int is_absolute_path(const char *path)
{
diff --git a/config.c b/config.c
index 1a4d85537b..f036c721e6 100644
--- a/config.c
+++ b/config.c
@@ -135,7 +135,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc
if (!path)
return config_error_nonbool("include.path");
- expanded = expand_user_path(path);
+ expanded = expand_user_path(path, 0);
if (!expanded)
return error("could not expand include path '%s'", path);
path = expanded;
@@ -177,7 +177,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat)
char *expanded;
int prefix = 0;
- expanded = expand_user_path(pat->buf);
+ expanded = expand_user_path(pat->buf, 0);
if (expanded) {
strbuf_reset(pat);
strbuf_addstr(pat, expanded);
@@ -948,7 +948,7 @@ int git_config_pathname(const char **dest, const char *var, const char *value)
{
if (!value)
return config_error_nonbool(var);
- *dest = expand_user_path(value);
+ *dest = expand_user_path(value, 0);
if (!*dest)
die(_("failed to expand user dir in: '%s'"), value);
return 0;
@@ -1498,7 +1498,7 @@ static int do_git_config_sequence(config_fn_t fn, void *data)
{
int ret = 0;
char *xdg_config = xdg_config_home("config");
- char *user_config = expand_user_path("~/.gitconfig");
+ char *user_config = expand_user_path("~/.gitconfig", 0);
char *repo_config = have_git_dir() ? git_pathdup("config") : NULL;
current_parsing_scope = CONFIG_SCOPE_SYSTEM;
diff --git a/credential-cache.c b/credential-cache.c
index 3cbd420019..91550bfb0b 100644
--- a/credential-cache.c
+++ b/credential-cache.c
@@ -87,7 +87,7 @@ static char *get_socket_path(void)
{
struct stat sb;
char *old_dir, *socket;
- old_dir = expand_user_path("~/.git-credential-cache");
+ old_dir = expand_user_path("~/.git-credential-cache", 0);
if (old_dir && !stat(old_dir, &sb) && S_ISDIR(sb.st_mode))
socket = xstrfmt("%s/socket", old_dir);
else
diff --git a/credential-store.c b/credential-store.c
index 55ca1b1334..ac295420dd 100644
--- a/credential-store.c
+++ b/credential-store.c
@@ -168,7 +168,7 @@ int cmd_main(int argc, const char **argv)
if (file) {
string_list_append(&fns, file);
} else {
- if ((file = expand_user_path("~/.git-credentials")))
+ if ((file = expand_user_path("~/.git-credentials", 0)))
string_list_append_nodup(&fns, file);
file = xdg_config_home("credentials");
if (file)
diff --git a/path.c b/path.c
index 22248436bf..010c565512 100644
--- a/path.c
+++ b/path.c
@@ -638,8 +638,10 @@ static struct passwd *getpw_str(const char *username, size_t len)
* Return a string with ~ and ~user expanded via getpw*. If buf != NULL,
* then it is a newly allocated string. Returns NULL on getpw failure or
* if path is NULL.
+ *
+ * If real_home is true, real_path($HOME) is used in the expansion.
*/
-char *expand_user_path(const char *path)
+char *expand_user_path(const char *path, int real_home)
{
struct strbuf user_path = STRBUF_INIT;
const char *to_copy = path;
@@ -654,7 +656,10 @@ char *expand_user_path(const char *path)
const char *home = getenv("HOME");
if (!home)
goto return_null;
- strbuf_addstr(&user_path, home);
+ if (real_home)
+ strbuf_addstr(&user_path, real_path(home));
+ else
+ strbuf_addstr(&user_path, home);
#ifdef GIT_WINDOWS_NATIVE
convert_slashes(user_path.buf);
#endif
@@ -723,7 +728,7 @@ const char *enter_repo(const char *path, int strict)
strbuf_add(&validated_path, path, len);
if (used_path.buf[0] == '~') {
- char *newpath = expand_user_path(used_path.buf);
+ char *newpath = expand_user_path(used_path.buf, 0);
if (!newpath)
return NULL;
strbuf_attach(&used_path, newpath, strlen(newpath),
--
2.11.0.157.gd943d85
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] config: resolve symlinks in conditional include's patterns
2017-04-05 10:24 ` [PATCH v2 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy
@ 2017-04-05 10:24 ` Nguyễn Thái Ngọc Duy
0 siblings, 0 replies; 11+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2017-04-05 10:24 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, tboegi, Nguyễn Thái Ngọc Duy
$GIT_DIR returned by get_git_dir() is normalized, with all symlinks
resolved (see setup_work_tree function). In order to match paths (or
patterns) against $GIT_DIR char-by-char, they have to be normalized
too. There is a note in config.txt about this, that the user need to
resolve symlinks by themselves if needed.
The problem is, we allow certain path expansion, '~/' and './', for
convenience and can't ask the user to resolve symlinks in these
expansions. Make sure the expanded paths have all symlinks resolved.
PS. The strbuf_realpath(&text, get_git_dir(), 1) is still needed because
get_git_dir() may return relative path.
Noticed-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Tests are added in v2.
config.c | 6 +++---
t/t1305-config-include.sh | 54 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/config.c b/config.c
index f036c721e6..d5ba848b65 100644
--- a/config.c
+++ b/config.c
@@ -177,7 +177,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat)
char *expanded;
int prefix = 0;
- expanded = expand_user_path(pat->buf, 0);
+ expanded = expand_user_path(pat->buf, 1);
if (expanded) {
strbuf_reset(pat);
strbuf_addstr(pat, expanded);
@@ -191,7 +191,7 @@ static int prepare_include_condition_pattern(struct strbuf *pat)
return error(_("relative config include "
"conditionals must come from files"));
- strbuf_add_absolute_path(&path, cf->path);
+ strbuf_realpath(&path, cf->path, 1);
slash = find_last_dir_sep(path.buf);
if (!slash)
die("BUG: how is this possible?");
@@ -213,7 +213,7 @@ static int include_by_gitdir(const char *cond, size_t cond_len, int icase)
struct strbuf pattern = STRBUF_INIT;
int ret = 0, prefix;
- strbuf_add_absolute_path(&text, get_git_dir());
+ strbuf_realpath(&text, get_git_dir(), 1);
strbuf_add(&pattern, cond, cond_len);
prefix = prepare_include_condition_pattern(&pattern);
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
index e833939320..8fbc7a029f 100755
--- a/t/t1305-config-include.sh
+++ b/t/t1305-config-include.sh
@@ -3,6 +3,16 @@
test_description='test config file include directives'
. ./test-lib.sh
+# Force setup_explicit_git_dir() to run until the end. This is needed
+# by some tests to make sure real_path() is called on $GIT_DIR. The
+# caller needs to make sure git commands are run from a subdirectory
+# though or real_path() will not be called.
+force_setup_explicit_git_dir() {
+ GIT_DIR="$(pwd)/.git"
+ GIT_WORK_TREE="$(pwd)"
+ export GIT_DIR GIT_WORK_TREE
+}
+
test_expect_success 'include file by absolute path' '
echo "[test]one = 1" >one &&
echo "[include]path = \"$(pwd)/one\"" >.gitconfig &&
@@ -208,6 +218,50 @@ test_expect_success 'conditional include, both unanchored, icase' '
)
'
+test_expect_success SYMLINKS 'conditional include, set up symlinked $HOME' '
+ mkdir real-home &&
+ ln -s real-home home &&
+ (
+ HOME="$TRASH_DIRECTORY/home" &&
+ export HOME &&
+ cd "$HOME" &&
+
+ git init foo &&
+ cd foo &&
+ mkdir sub
+ )
+'
+
+test_expect_success SYMLINKS 'conditional include, $HOME expansion with symlinks' '
+ (
+ HOME="$TRASH_DIRECTORY/home" &&
+ export HOME &&
+ cd "$HOME"/foo &&
+
+ echo "[includeIf \"gitdir:~/foo/\"]path=bar2" >>.git/config &&
+ echo "[test]two=2" >.git/bar2 &&
+ echo 2 >expect &&
+ force_setup_explicit_git_dir &&
+ git -C sub config test.two >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success SYMLINKS 'conditional include, relative path with symlinks' '
+ echo "[includeIf \"gitdir:./foo/.git\"]path=bar4" >home/.gitconfig &&
+ echo "[test]four=4" >home/bar4 &&
+ (
+ HOME="$TRASH_DIRECTORY/home" &&
+ export HOME &&
+ cd "$HOME"/foo &&
+
+ echo 4 >expect &&
+ force_setup_explicit_git_dir &&
+ git -C sub config test.four >actual &&
+ test_cmp expect actual
+ )
+'
+
test_expect_success 'include cycles are detected' '
cat >.gitconfig <<-\EOF &&
[test]value = gitconfig
--
2.11.0.157.gd943d85
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-04-05 10:26 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-25 10:46 t1503 broken ? Torsten Bögershausen
2017-03-25 11:46 ` Duy Nguyen
2017-03-25 12:26 ` Duy Nguyen
2017-03-25 13:05 ` Duy Nguyen
2017-03-25 19:41 ` Torsten Bögershausen
2017-03-30 11:37 ` [PATCH 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy
2017-03-30 11:37 ` [PATCH 2/2] config: resolve symlinks in conditional include's patterns Nguyễn Thái Ngọc Duy
2017-03-30 18:38 ` Junio C Hamano
2017-04-04 10:12 ` Duy Nguyen
2017-04-05 10:24 ` [PATCH v2 1/2] path.c: and an option to call real_path() in expand_user_path() Nguyễn Thái Ngọc Duy
2017-04-05 10:24 ` [PATCH v2 2/2] config: resolve symlinks in conditional include's patterns Nguyễn Thái Ngọc Duy
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).