t1503 broken ?

PATCH Superseded Torsten Bögershausen 3 participants 11 messages Mar 25, 2017 – Apr 5, 2017

Share & tools

Torsten Bögershausen

author

2017-03-25 10:46 UTC #0 lore raw

./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 ?

Duy Nguyen

2017-03-25 11:55 UTC #1 lore raw

Quoted 4 lines · Torsten Bögershausen

./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?

Signature · 1 line

Duy

Duy Nguyen

2017-03-25 12:26 UTC #2 lore raw

Quoted 7 lines · Duy Nguyen

Quoted 4 lines · Torsten Bögershausen
./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.

Signature · 1 line

Duy

Duy Nguyen

2017-03-25 13:06 UTC #3 lore raw

Quoted 7 lines · Duy Nguyen

Quoted 6 lines · Duy Nguyen
Quoted 4 lines · Torsten Bögershausen
./t1305-config-include.sh
seems to be broken:
not ok 19 - conditional include, $HOME expansion
not ok 21 - conditional include, relative path

Quoted 5 lines

Quoted 1 line
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< --

config.c +2 -2

index 1a4d855..fc4eae9 100644
@@ -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);

path.c +1 -1

index 2224843..18eaac3 100644
@@ -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< --

Signature · 1 line

Duy

Torsten Bögershausen

author

2017-03-25 19:42 UTC #4 lore raw

Quoted 47 lines · Duy Nguyen

Quoted 10 lines · Duy Nguyen
Quoted 6 lines · Duy Nguyen
Quoted 4 lines · Torsten Bögershausen
./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< --
config.c +2 -2
index 1a4d855..fc4eae9 100644
@@ -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);
 
path.c +0 -0
index 2224843..18eaac3 100644

path.c char *expand_user_path(const char *path)

Quoted 9 lines

path.c +1 -2

@@ -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.

Nguyễn Thái Ngọc Duy

2017-03-30 11:37 UTC #5 lore raw

PATCH 1/2 path.c: and an option to call real_path() in expand_user_path()

In reply to Duy Nguyen

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 <[email protected]>

---
 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(-)

builtin/commit.c +1 -1

index 4e288bc513..ad188fea9e 100644
@@ -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);

builtin/config.c +1 -1

index 05843a0f96..70bfaaaa1d 100644
@@ -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)

cache.h +1 -1

index 2214d52f61..62e44bfa2f 100644
@@ -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)
 {

config.c +4 -4

index 1a4d85537b..f036c721e6 100644
@@ -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;

credential-cache.c +1 -1

index 3cbd420019..91550bfb0b 100644
@@ -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

credential-store.c +1 -1

index 55ca1b1334..ac295420dd 100644
@@ -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)

path.c +8 -3

index 22248436bf..010c565512 100644
@@ -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),

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

2017-03-30 11:37 UTC #6 lore raw

PATCH 2/2 config: resolve symlinks in conditional include's patterns

$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 <[email protected]>

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 config.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

config.c +3 -3

index f036c721e6..d5ba848b65 100644
@@ -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);

Signature · 1 line

2.11.0.157.gd943d85

Junio C Hamano

2017-03-30 18:38 UTC #7 lore raw

Re: [PATCH 2/2] config: resolve symlinks in conditional include's patterns

Quoted 9 lines · 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.

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.

Quoted 39 lines

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 <[email protected]>

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 config.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

config.c +3 -3

index f036c721e6..d5ba848b65 100644
@@ -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);

Duy Nguyen

2017-04-04 10:13 UTC #8 lore raw

Re: [PATCH 2/2] config: resolve symlinks in conditional include's patterns

Quoted 8 lines · Junio C Hamano

Quoted 6 lines · 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.

Quoted 8 lines

Quoted 3 lines
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

Quoted 2 lines

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.

Signature · 1 line

Duy

Nguyễn Thái Ngọc Duy

2017-04-05 10:26 UTC #9 lore raw

PATCH v2 1/2 path.c: and an option to call real_path() in expand_user_path()

In reply to Nguyễn Thái Ngọc Duy

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
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(-)

builtin/commit.c +1 -1

index 4e288bc513..ad188fea9e 100644
@@ -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);

builtin/config.c +1 -1

index 05843a0f96..70bfaaaa1d 100644
@@ -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)

cache.h +1 -1

index 2214d52f61..62e44bfa2f 100644
@@ -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)
 {

config.c +4 -4

index 1a4d85537b..f036c721e6 100644
@@ -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;

credential-cache.c +1 -1

index 3cbd420019..91550bfb0b 100644
@@ -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

credential-store.c +1 -1

index 55ca1b1334..ac295420dd 100644
@@ -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)

path.c +8 -3

index 22248436bf..010c565512 100644
@@ -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),

Signature · 1 line

2.11.0.157.gd943d85

Nguyễn Thái Ngọc Duy

2017-04-05 10:26 UTC #10 lore raw

PATCH v2 2/2 config: resolve symlinks in conditional include's patterns

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 <[email protected]>

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
Tests are added in v2.

 config.c                  |  6 +++---
 t/t1305-config-include.sh | 54 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 3 deletions(-)

config.c +3 -3

index f036c721e6..d5ba848b65 100644
@@ -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);

t/t1305-config-include.sh +54 -0

66 lines not on this page · raw message

Signature · 1 line

2.11.0.157.gd943d85