Re: t1503 broken ?

2017-03-25 Thread Torsten Bögershausen



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  wrote:

On Sat, Mar 25, 2017 at 5:46 PM, Torsten Bögershausen  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(, cf->path);

+   strbuf_realpath(, 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(, get_git_dir());

+   strbuf_realpath(, get_git_dir(), 1);
strbuf_add(, cond, cond_len);
prefix = prepare_include_condition_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(_path, home);
+   strbuf_addstr(_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.





Re: t1503 broken ?

2017-03-25 Thread Duy Nguyen
On Sat, Mar 25, 2017 at 07:26:14PM +0700, Duy Nguyen wrote:
> On Sat, Mar 25, 2017 at 6:46 PM, Duy Nguyen  wrote:
> > On Sat, Mar 25, 2017 at 5:46 PM, Torsten Bögershausen  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(, cf->path);
+   strbuf_realpath(, 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(, get_git_dir());
+   strbuf_realpath(, get_git_dir(), 1);
strbuf_add(, cond, cond_len);
prefix = prepare_include_condition_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(_path, home);
+   strbuf_addstr(_path, real_path(home));
 #ifdef GIT_WINDOWS_NATIVE
convert_slashes(user_path.buf);
 #endif
-- 8< --
-- 
Duy


Re: t1503 broken ?

2017-03-25 Thread Duy Nguyen
On Sat, Mar 25, 2017 at 6:46 PM, Duy Nguyen  wrote:
> On Sat, Mar 25, 2017 at 5:46 PM, Torsten Bögershausen  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


Re: t1503 broken ?

2017-03-25 Thread Duy Nguyen
On Sat, Mar 25, 2017 at 5:46 PM, Torsten Bögershausen  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