# [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) 10 messages from 2019-04-08 to 2019-04-12. Participants: Sven Strickroth, Taylor Blau, Torsten Bögershausen, Junio C Hamano. Thread: https://gitlist.dev/t/50893 ## Sven Strickroth, 2019-04-08 11:16 Subject: [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) Message-ID: <6c7d4155-e554-dc9a-053e-f3a8c7cd4075@cs-ware.de> URL: https://gitlist.dev/e/6c7d4155-e554-dc9a-053e-f3a8c7cd4075%40cs-ware.de ``` A path such as 'c:/somepath/submodule/../.git/modules/submodule' wasn't resolved correctly any more, because the *nix variant of offset_1st_component is used instead of the Win32 specific version. Regression was introduced in commit 25d90d1cb72ce51407324259516843406142fe89. Signed-off-by: Sven Strickroth --- git-compat-util.h | 1 + 1 file changed, 1 insertion(+) diff --git a/git-compat-util.h b/git-compat-util.h index e0275da7e0..9be177e588 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -210,6 +210,7 @@ #include "compat/mingw.h" #include "compat/win32/fscache.h" #elif defined(_MSC_VER) +#include "compat/win32/path-utils.h" #include "compat/msvc.h" #include "compat/win32/fscache.h" #else -- 2.21.0.windows.1 ``` ## Taylor Blau, 2019-04-09 02:36 Subject: Re: [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) Message-ID: <20190409023630.GG81620@Taylors-MBP.hsd1.wa.comcast.net> URL: https://gitlist.dev/e/20190409023630.GG81620%40Taylors-MBP.hsd1.wa.comcast.net In-Reply-To: <6c7d4155-e554-dc9a-053e-f3a8c7cd4075@cs-ware.de> ``` Hi Sven, On Mon, Apr 08, 2019 at 01:16:33PM +0200, Sven Strickroth wrote: > A path such as 'c:/somepath/submodule/../.git/modules/submodule' wasn't > resolved correctly any more, because the *nix variant of > offset_1st_component is used instead of the Win32 specific version. I'm not a win32 expert by any sense, but I am do have a meta-question about your patch... > Regression was introduced in commit > 25d90d1cb72ce51407324259516843406142fe89. I can't seem to find this commit anywhere upstream. Is this SHA-1 pasted correctly? > > Signed-off-by: Sven Strickroth > --- > git-compat-util.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/git-compat-util.h b/git-compat-util.h > index e0275da7e0..9be177e588 100644 > --- a/git-compat-util.h > +++ b/git-compat-util.h > @@ -210,6 +210,7 @@ > #include "compat/mingw.h" > #include "compat/win32/fscache.h" > #elif defined(_MSC_VER) > +#include "compat/win32/path-utils.h" > #include "compat/msvc.h" > #include "compat/win32/fscache.h" > #else > -- > 2.21.0.windows.1 Thanks, Taylor ``` ## Torsten Bögershausen, 2019-04-09 05:53 Subject: Re: [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) Message-ID: <0f629384-638f-bfb9-89da-ade335e364fd@web.de> URL: https://gitlist.dev/e/0f629384-638f-bfb9-89da-ade335e364fd%40web.de In-Reply-To: <6c7d4155-e554-dc9a-053e-f3a8c7cd4075@cs-ware.de> ``` On 2019-04-08 13:16, Sven Strickroth wrote: > A path such as 'c:/somepath/submodule/../.git/modules/submodule' wasn't > resolved correctly any more, because the *nix variant of > offset_1st_component is used instead of the Win32 specific version. > > Regression was introduced in commit > 25d90d1cb72ce51407324259516843406142fe89. Was it ? 25d90d1cb merged this commit: 1cadad6f6 (junio/tb/use-common-win32-pathfuncs-on-cygwin) And, if I read that correctly, 1cadad6f6 does not change anything for MSVC. And the problem with the missing/wrong path resolution was there before 1cadad6f6 and after 1cadad6f6. From that point of view, the patch looks correct, but: The other question: In config.mak.uname we need to add a line compat/win32/path-utils.o for the Windows build. In the git-for windows codebase I see COMPAT_OBJS +=compat/win32/path-utils 3 times: For Cygwin, MINGW and Windows. In git.git only for Cygwin and MINGW. (I don't have MSVC, so I can't test) > > Signed-off-by: Sven Strickroth > --- > git-compat-util.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/git-compat-util.h b/git-compat-util.h > index e0275da7e0..9be177e588 100644 > --- a/git-compat-util.h > +++ b/git-compat-util.h > @@ -210,6 +210,7 @@ > #include "compat/mingw.h" > #include "compat/win32/fscache.h" > #elif defined(_MSC_VER) > +#include "compat/win32/path-utils.h" > #include "compat/msvc.h" > #include "compat/win32/fscache.h" > #else > ``` ## Sven Strickroth, 2019-04-09 07:34 Subject: Re: [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) Message-ID: <950ee9b8-786f-28cd-3e89-ad174fd857a4@cs-ware.de> URL: https://gitlist.dev/e/950ee9b8-786f-28cd-3e89-ad174fd857a4%40cs-ware.de In-Reply-To: <0f629384-638f-bfb9-89da-ade335e364fd@web.de> ``` Am 09.04.2019 um 07:53 schrieb Torsten Bögershausen: >> Regression was introduced in commit >> 25d90d1cb72ce51407324259516843406142fe89. > > Was it ? > 25d90d1cb merged this commit: > 1cadad6f6 (junio/tb/use-common-win32-pathfuncs-on-cygwin) Yes, I copied the revision of the merge commit. > And, if I read that correctly, 1cadad6f6 does not change anything for MSVC. > And the problem with the missing/wrong path resolution was there before > 1cadad6f6 and after 1cadad6f6. That's not correct, it was correct before: 1cadad6f6 removes mingw_offset_1st_component from mingw.c which is included by msvc.c. Then the in git-compat.h the new file "compat/win32/path-utils.h" is only included for __CYGWIN__ and __MINGW32__, here _MSC_VER is missing -> that's the regression. > In config.mak.uname we need to add a line > compat/win32/path-utils.o > for the Windows build. > In the git-for windows codebase I see > COMPAT_OBJS +=compat/win32/path-utils I don't use config.mak.uname and never did, so I can't tell you about that. -- Best regards, Sven Strickroth PGP key id F5A9D4C4 @ any key-server ``` ## Sven Strickroth, 2019-04-08 11:26 Subject: [PATCH v2] MSVC: Unbreak real_path for Windows paths Message-ID: <31485f76-13a9-ec3b-16b9-78864490164d@cs-ware.de> URL: https://gitlist.dev/e/31485f76-13a9-ec3b-16b9-78864490164d%40cs-ware.de In-Reply-To: <950ee9b8-786f-28cd-3e89-ad174fd857a4@cs-ware.de> ``` A path such as 'c:/somepath/submodule/../.git/modules/submodule' wasn't resolved correctly any more, because the *nix variant of offset_1st_component is used instead of the Win32 specific version. Regression was introduced in commit 1cadad6f6 when mingw_offset_1st_component was moved from mingw.c which is included by msvc.c to a separate file. Then, the new file "compat/win32/path-utils.h" was only included for the __CYGWIN__ and __MINGW32__ cases in git-compat-util.h, the case for _MSC_VER was missing. Signed-off-by: Sven Strickroth --- config.mak.uname | 1 + git-compat-util.h | 1 + 2 files changed, 2 insertions(+) diff --git a/config.mak.uname b/config.mak.uname index 32381f5fd1..eb1428858c 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -426,6 +426,7 @@ ifeq ($(uname_S),Windows) CFLAGS = BASIC_CFLAGS = -nologo -I. -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE COMPAT_OBJS = compat/msvc.o compat/winansi.o \ + compat/win32/path-utils.o \ compat/win32/pthread.o compat/win32/syslog.o \ compat/win32/dirent.o compat/win32/fscache.o COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DDETECT_MSYS_TTY -DNOGDI -DHAVE_STRING_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\" diff --git a/git-compat-util.h b/git-compat-util.h index e0275da7e0..9be177e588 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -210,6 +210,7 @@ #include "compat/mingw.h" #include "compat/win32/fscache.h" #elif defined(_MSC_VER) +#include "compat/win32/path-utils.h" #include "compat/msvc.h" #include "compat/win32/fscache.h" #else -- 2.21.0.windows.1 ``` ## Junio C Hamano, 2019-04-09 11:45 Subject: Re: [PATCH v2] MSVC: Unbreak real_path for Windows paths Message-ID: URL: https://gitlist.dev/e/xmqqzhoz4d2f.fsf%40gitster-ct.c.googlers.com In-Reply-To: <31485f76-13a9-ec3b-16b9-78864490164d@cs-ware.de> ``` Sven Strickroth writes: > A path such as 'c:/somepath/submodule/../.git/modules/submodule' wasn't > resolved correctly any more, because the *nix variant of offset_1st_component > is used instead of the Win32 specific version. > > Regression was introduced in commit 1cadad6f6 when mingw_offset_1st_component > was moved from mingw.c which is included by msvc.c to a separate file. Then, > the new file "compat/win32/path-utils.h" was only included for the __CYGWIN__ > and __MINGW32__ cases in git-compat-util.h, the case for _MSC_VER was missing. > > Signed-off-by: Sven Strickroth > --- > config.mak.uname | 1 + > git-compat-util.h | 1 + > 2 files changed, 2 insertions(+) Some context lines in config.mak.uname did not match tips of any of the well-known branches I tried, and the blob object name recorded on the "index" line was not useful, either, so I ended up applying the patch by hand. I do not think I screwed up a simple two-liner patch like this too badly ;-), but please keep an eye on what will appear on 'pu' and holler if I did, so we can correct it before it hits 'master'. Thanks. > > diff --git a/config.mak.uname b/config.mak.uname > index 32381f5fd1..eb1428858c 100644 > --- a/config.mak.uname > +++ b/config.mak.uname > @@ -426,6 +426,7 @@ ifeq ($(uname_S),Windows) > CFLAGS = > BASIC_CFLAGS = -nologo -I. -Icompat/vcbuild/include -DWIN32 -D_CONSOLE -DHAVE_STRING_H -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE ``` ## Torsten Bögershausen, 2019-04-09 16:19 Subject: Re: [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) Message-ID: URL: https://gitlist.dev/e/af640e82-ca1a-9c96-da47-62aaea1cc18e%40web.de In-Reply-To: <950ee9b8-786f-28cd-3e89-ad174fd857a4@cs-ware.de> ``` On 2019-04-09 09:34, Sven Strickroth wrote: > Am 09.04.2019 um 07:53 schrieb Torsten Bögershausen: >>> Regression was introduced in commit >>> 25d90d1cb72ce51407324259516843406142fe89. >> >> Was it ? >> 25d90d1cb merged this commit: >> 1cadad6f6 (junio/tb/use-common-win32-pathfuncs-on-cygwin) > > Yes, I copied the revision of the merge commit. > >> And, if I read that correctly, 1cadad6f6 does not change anything for MSVC. >> And the problem with the missing/wrong path resolution was there before >> 1cadad6f6 and after 1cadad6f6. > > That's not correct, it was correct before: No, I wasn't aware that msvc.c include mingw.c - for whatever reason. > 1cadad6f6 removes mingw_offset_1st_component from mingw.c which is > included by msvc.c. Then the in git-compat.h the new file > "compat/win32/path-utils.h" is only included for __CYGWIN__ and > __MINGW32__, here _MSC_VER is missing -> that's the regression. > OK, good. If possible, I would like to see this kind of information in the commit message. Thanks for cleaning up my mess. ``` ## Junio C Hamano, 2019-04-09 16:46 Subject: Re: [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) Message-ID: URL: https://gitlist.dev/e/xmqqv9zn2kjs.fsf%40gitster-ct.c.googlers.com In-Reply-To: ``` Torsten Bögershausen writes: >> 1cadad6f6 removes mingw_offset_1st_component from mingw.c which is >> included by msvc.c. Then the in git-compat.h the new file >> "compat/win32/path-utils.h" is only included for __CYGWIN__ and >> __MINGW32__, here _MSC_VER is missing -> that's the regression. >> > > OK, good. > If possible, I would like to see this kind of information > in the commit message. > Thanks for cleaning up my mess. Thanks, both. Should I wait for an update that fixes the proposed log message? ``` ## Torsten Bögershausen, 2019-04-10 05:32 Subject: Re: [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) Message-ID: URL: https://gitlist.dev/e/d1cf10ec-ca57-09e3-cf00-3be87dec06ee%40web.de In-Reply-To: ``` On 2019-04-09 18:46, Junio C Hamano wrote: > Torsten Bögershausen writes: > >>> 1cadad6f6 removes mingw_offset_1st_component from mingw.c which is >>> included by msvc.c. Then the in git-compat.h the new file >>> "compat/win32/path-utils.h" is only included for __CYGWIN__ and >>> __MINGW32__, here _MSC_VER is missing -> that's the regression. >>> >> >> OK, good. >> If possible, I would like to see this kind of information >> in the commit message. >> Thanks for cleaning up my mess. > > Thanks, both. Should I wait for an update that fixes the proposed > log message? > It seems that I haven't read all messages in my mailbox (or messages crossed). The V2 patch describes the problem well and looks OK for me. ``` ## Junio C Hamano, 2019-04-12 01:15 Subject: Re: [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) Message-ID: URL: https://gitlist.dev/e/xmqqftqo10si.fsf%40gitster-ct.c.googlers.com In-Reply-To: ``` Torsten Bögershausen writes: > It seems that I haven't read all messages in my mailbox (or messages crossed). > > The V2 patch describes the problem well and looks OK for me. Yeah, I just re-read the log message with a fresh pair of eyes, and I think it is clear enough. Thanks, both. ```