* [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio)
@ 2019-04-08 11:16 Sven Strickroth
2019-04-09 2:36 ` Taylor Blau
2019-04-09 5:53 ` Torsten Bögershausen
0 siblings, 2 replies; 10+ messages in thread
From: Sven Strickroth @ 2019-04-08 11:16 UTC (permalink / raw)
To: git; +Cc: gitster, peff, johannes.schindelin
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 <email@cs-ware.de>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2] MSVC: Unbreak real_path for Windows paths
2019-04-09 7:34 ` Sven Strickroth
@ 2019-04-08 11:26 ` Sven Strickroth
2019-04-09 11:45 ` Junio C Hamano
2019-04-09 16:19 ` [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) Torsten Bögershausen
1 sibling, 1 reply; 10+ messages in thread
From: Sven Strickroth @ 2019-04-08 11:26 UTC (permalink / raw)
To: Torsten Bögershausen , git; +Cc: gitster, peff, johannes.schindelin
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 <email@cs-ware.de>
---
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio)
2019-04-08 11:16 [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) Sven Strickroth
@ 2019-04-09 2:36 ` Taylor Blau
2019-04-09 5:53 ` Torsten Bögershausen
1 sibling, 0 replies; 10+ messages in thread
From: Taylor Blau @ 2019-04-09 2:36 UTC (permalink / raw)
To: Sven Strickroth; +Cc: git, gitster, peff, johannes.schindelin
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 <email@cs-ware.de>
> ---
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio)
2019-04-08 11:16 [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) Sven Strickroth
2019-04-09 2:36 ` Taylor Blau
@ 2019-04-09 5:53 ` Torsten Bögershausen
2019-04-09 7:34 ` Sven Strickroth
1 sibling, 1 reply; 10+ messages in thread
From: Torsten Bögershausen @ 2019-04-09 5:53 UTC (permalink / raw)
To: Sven Strickroth, git; +Cc: gitster, peff, johannes.schindelin
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 <email@cs-ware.de>
> ---
> 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
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio)
2019-04-09 5:53 ` Torsten Bögershausen
@ 2019-04-09 7:34 ` Sven Strickroth
2019-04-08 11:26 ` [PATCH v2] MSVC: Unbreak real_path for Windows paths Sven Strickroth
2019-04-09 16:19 ` [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) Torsten Bögershausen
0 siblings, 2 replies; 10+ messages in thread
From: Sven Strickroth @ 2019-04-09 7:34 UTC (permalink / raw)
To: Torsten Bögershausen, git; +Cc: gitster, peff, johannes.schindelin
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] MSVC: Unbreak real_path for Windows paths
2019-04-08 11:26 ` [PATCH v2] MSVC: Unbreak real_path for Windows paths Sven Strickroth
@ 2019-04-09 11:45 ` Junio C Hamano
0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2019-04-09 11:45 UTC (permalink / raw)
To: Sven Strickroth; +Cc: Torsten Bögershausen, git, peff, johannes.schindelin
Sven Strickroth <email@cs-ware.de> 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 <email@cs-ware.de>
> ---
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio)
2019-04-09 7:34 ` Sven Strickroth
2019-04-08 11:26 ` [PATCH v2] MSVC: Unbreak real_path for Windows paths Sven Strickroth
@ 2019-04-09 16:19 ` Torsten Bögershausen
2019-04-09 16:46 ` Junio C Hamano
1 sibling, 1 reply; 10+ messages in thread
From: Torsten Bögershausen @ 2019-04-09 16:19 UTC (permalink / raw)
To: Sven Strickroth, git; +Cc: gitster, peff, johannes.schindelin
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio)
2019-04-09 16:19 ` [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) Torsten Bögershausen
@ 2019-04-09 16:46 ` Junio C Hamano
2019-04-10 5:32 ` Torsten Bögershausen
0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2019-04-09 16:46 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Sven Strickroth, git, peff, johannes.schindelin
Torsten Bögershausen <tboegi@web.de> 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?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio)
2019-04-09 16:46 ` Junio C Hamano
@ 2019-04-10 5:32 ` Torsten Bögershausen
2019-04-12 1:15 ` Junio C Hamano
0 siblings, 1 reply; 10+ messages in thread
From: Torsten Bögershausen @ 2019-04-10 5:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sven Strickroth, git, peff, johannes.schindelin
On 2019-04-09 18:46, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> 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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio)
2019-04-10 5:32 ` Torsten Bögershausen
@ 2019-04-12 1:15 ` Junio C Hamano
0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2019-04-12 1:15 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Sven Strickroth, git, peff, johannes.schindelin
Torsten Bögershausen <tboegi@web.de> 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.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-04-12 1:15 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-08 11:16 [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) Sven Strickroth
2019-04-09 2:36 ` Taylor Blau
2019-04-09 5:53 ` Torsten Bögershausen
2019-04-09 7:34 ` Sven Strickroth
2019-04-08 11:26 ` [PATCH v2] MSVC: Unbreak real_path for Windows paths Sven Strickroth
2019-04-09 11:45 ` Junio C Hamano
2019-04-09 16:19 ` [PATCH] Unbreak real_path on Windows for already absolute paths (with Visual Studio) Torsten Bögershausen
2019-04-09 16:46 ` Junio C Hamano
2019-04-10 5:32 ` Torsten Bögershausen
2019-04-12 1:15 ` Junio C Hamano
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).