Re: [PATCH/RFC] mingw: rename WIN32 cpp macro to NATIVE_WINDOWS
On 26.02.13 05:08, Mark Levedahl wrote: > On 02/25/2013 01:44 AM, Junio C Hamano wrote: >> I was in "find leftover bits" mode today and found this thread hanging. Has >> anything come out of this thread, or there is nothing to improve in this >> area? > > The patch passed my simple tests (build, run a few commands), but I didn't > get around to a full test. And of course, I am testing on current Cygwin > where git compiles and runs correctly anyway. > > Mark I run the test suite, and there was 1301 failing (and t0070 ?) which have to do with POSIX permisions. They are on my TODO stack. /Torsten -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] mingw: rename WIN32 cpp macro to NATIVE_WINDOWS
On 02/25/2013 01:44 AM, Junio C Hamano wrote: I was in "find leftover bits" mode today and found this thread hanging. Has anything come out of this thread, or there is nothing to improve in this area? The patch passed my simple tests (build, run a few commands), but I didn't get around to a full test. And of course, I am testing on current Cygwin where git compiles and runs correctly anyway. Mark -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] mingw: rename WIN32 cpp macro to NATIVE_WINDOWS
Ramsay Jones writes: > Jonathan Nieder wrote: > >> Throughout git, it is assumed that the WIN32 preprocessor symbol is >> defined on native Windows setups (mingw and msvc) and not on Cygwin. >> On Cygwin, most of the time git can pretend this is just another Unix >> machine, and Windows-specific magic is generally counterproductive. >> >> Unfortunately Cygwin *does* define the WIN32 symbol in some headers. >> Best to rely on a new git-specific symbol NATIVE_WINDOWS instead, >> defined as follows: >> >> #if defined(WIN32) && !defined(__CYGWIN__) >> # define NATIVE_WINDOWS >> #endif >> >> After this change, it should be possible to drop the >> CYGWIN_V15_WIN32API setting without any negative effect. >> >> Signed-off-by: Jonathan Nieder > > If we go with this approach, could we prefix the symbol name with GIT_ > in order to reduce the global namespace pollution? > > eg GIT_NATIVE_WINDOWS, or GIT_NATIVE_WIN32 or just GIT_WIN32. > (Yeah, I'm not good at choosing names!) I was in "find leftover bits" mode today and found this thread hanging. Has anything come out of this thread, or there is nothing to improve in this area? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] mingw: rename WIN32 cpp macro to NATIVE_WINDOWS
Jonathan Nieder wrote: > Throughout git, it is assumed that the WIN32 preprocessor symbol is > defined on native Windows setups (mingw and msvc) and not on Cygwin. > On Cygwin, most of the time git can pretend this is just another Unix > machine, and Windows-specific magic is generally counterproductive. > > Unfortunately Cygwin *does* define the WIN32 symbol in some headers. > Best to rely on a new git-specific symbol NATIVE_WINDOWS instead, > defined as follows: > > #if defined(WIN32) && !defined(__CYGWIN__) > # define NATIVE_WINDOWS > #endif > > After this change, it should be possible to drop the > CYGWIN_V15_WIN32API setting without any negative effect. > > Signed-off-by: Jonathan Nieder If we go with this approach, could we prefix the symbol name with GIT_ in order to reduce the global namespace pollution? eg GIT_NATIVE_WINDOWS, or GIT_NATIVE_WIN32 or just GIT_WIN32. (Yeah, I'm not good at choosing names!) ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] mingw: rename WIN32 cpp macro to NATIVE_WINDOWS
On 26.01.13 02:03, Jonathan Nieder wrote:
> Throughout git, it is assumed that the WIN32 preprocessor symbol is
> defined on native Windows setups (mingw and msvc) and not on Cygwin.
> On Cygwin, most of the time git can pretend this is just another Unix
> machine, and Windows-specific magic is generally counterproductive.
>
> Unfortunately Cygwin *does* define the WIN32 symbol in some headers.
> Best to rely on a new git-specific symbol NATIVE_WINDOWS instead,
> defined as follows:
>
> #if defined(WIN32) && !defined(__CYGWIN__)
> # define NATIVE_WINDOWS
> #endif
>
> After this change, it should be possible to drop the
> CYGWIN_V15_WIN32API setting without any negative effect.
>
> Signed-off-by: Jonathan Nieder
> ---
> Eric Blake wrote:
>
>> Which is why other projects, like gnulib, have
>>
>> # if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
>>
>> all over the place.
> So, how about this?
>
> Completely untested.
>
> abspath.c | 2 +-
> compat/terminal.c | 4 ++--
> compat/win32.h| 2 +-
> diff-no-index.c | 2 +-
> git-compat-util.h | 3 ++-
> help.c| 2 +-
> run-command.c | 10 +-
> test-chmtime.c| 2 +-
> thread-utils.c| 2 +-
> 9 files changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/abspath.c b/abspath.c
> index 40cdc462..c7d5458e 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -216,7 +216,7 @@ const char *absolute_path(const char *path)
> const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
> {
> static char path[PATH_MAX];
> -#ifndef WIN32
> +#ifndef WINDOWS_NATIVE
> if (!pfx_len || is_absolute_path(arg))
> return arg;
> memcpy(path, pfx, pfx_len);
> diff --git a/compat/terminal.c b/compat/terminal.c
> index 9b5e3d1b..136e4a74 100644
> --- a/compat/terminal.c
> +++ b/compat/terminal.c
> @@ -3,7 +3,7 @@
> #include "sigchain.h"
> #include "strbuf.h"
>
> -#if defined(HAVE_DEV_TTY) || defined(WIN32)
> +#if defined(HAVE_DEV_TTY) || defined(WINDOWS_NATIVE)
>
> static void restore_term(void);
>
> @@ -53,7 +53,7 @@ error:
> return -1;
> }
>
> -#elif defined(WIN32)
> +#elif defined(WINDOWS_NATIVE)
>
> #define INPUT_PATH "CONIN$"
> #define OUTPUT_PATH "CONOUT$"
> diff --git a/compat/win32.h b/compat/win32.h
> index 8ce91048..31dd30f7 100644
> --- a/compat/win32.h
> +++ b/compat/win32.h
> @@ -2,7 +2,7 @@
> #define WIN32_H
>
> /* common Win32 functions for MinGW and Cygwin */
> -#ifndef WIN32 /* Not defined by Cygwin */
> +#ifndef WINDOWS_NATIVE /* Not defined for Cygwin */
> #include
> #endif
>
> diff --git a/diff-no-index.c b/diff-no-index.c
> index 74da6593..a0bc9c50 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -45,7 +45,7 @@ static int get_mode(const char *path, int *mode)
>
> if (!path || !strcmp(path, "/dev/null"))
> *mode = 0;
> -#ifdef _WIN32
> +#ifdef WINDOWS_NATIVE
> else if (!strcasecmp(path, "nul"))
> *mode = 0;
> #endif
> diff --git a/git-compat-util.h b/git-compat-util.h
> index e5a4b745..ebbdff53 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -85,10 +85,11 @@
> #define _NETBSD_SOURCE 1
> #define _SGI_SOURCE 1
>
> -#ifdef WIN32 /* Both MinGW and MSVC */
> +#if defined(WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */
> #define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */
> #include
> #include
> +#define WINDOWS_NATIVE
> #endif
>
> #include
> diff --git a/help.c b/help.c
> index 2a42ec6d..cc1e63f7 100644
> --- a/help.c
> +++ b/help.c
> @@ -106,7 +106,7 @@ static int is_executable(const char *name)
> !S_ISREG(st.st_mode))
> return 0;
>
> -#if defined(WIN32) || defined(__CYGWIN__)
> +#if defined(WINDOWS_NATIVE) || defined(__CYGWIN__)
> #if defined(__CYGWIN__)
> if ((st.st_mode & S_IXUSR) == 0)
> #endif
> diff --git a/run-command.c b/run-command.c
> index 04712191..04ac6181 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -72,7 +72,7 @@ static inline void close_pair(int fd[2])
> close(fd[1]);
> }
>
> -#ifndef WIN32
> +#ifndef WINDOWS_NATIVE
> static inline void dup_devnull(int to)
> {
> int fd = open("/dev/null", O_RDWR);
> @@ -159,7 +159,7 @@ static const char **prepare_shell_cmd(const char **argv)
> die("BUG: shell command is empty");
>
> if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) {
> -#ifndef WIN32
> +#ifndef WINDOWS_NATIVE
> nargv[nargc++] = SHELL_PATH;
> #else
> nargv[nargc++] = "sh";
> @@ -182,7 +182,7 @@ static const char **prepare_shell_cmd(const char **argv)
> return nargv;
> }
>
> -#ifndef WIN32
> +#ifndef WINDOWS_NATIVE
> static int execv_shell_cmd(const char **argv)
> {
> const char **nargv = prepare_shell_cmd(argv);
> @@ -193,7 +193,7 @@ static int execv_shell_cmd(const char **argv)
> }
> #endif
>
> -#ifndef WIN32
> +#ifndef WI
Re: [PATCH/RFC] mingw: rename WIN32 cpp macro to NATIVE_WINDOWS
On 01/25/2013 08:03 PM, Jonathan Nieder wrote:
diff --git a/abspath.c b/abspath.c
index 40cdc462..c7d5458e 100644
--- a/abspath.c
+++ b/abspath.c
@@ -216,7 +216,7 @@ const char *absolute_path(const char *path)
const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
{
static char path[PATH_MAX];
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
if (!pfx_len || is_absolute_path(arg))
return arg;
memcpy(path, pfx, pfx_len);
diff --git a/compat/terminal.c b/compat/terminal.c
index 9b5e3d1b..136e4a74 100644
Maybe WINDOWS_NATIVE should be defined in config.mak.uname?
Otherwise, I tested the patch and it does build / run on Cygwin, but I
cannot run a test suite until next week. I am concerned about subtle
changes due to the various WIN32 tests that were not guarded by
__CYGWIN__ before, haven't stared at the code enough to see if there
could be an issue.
Mark
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC] mingw: rename WIN32 cpp macro to NATIVE_WINDOWS
Throughout git, it is assumed that the WIN32 preprocessor symbol is
defined on native Windows setups (mingw and msvc) and not on Cygwin.
On Cygwin, most of the time git can pretend this is just another Unix
machine, and Windows-specific magic is generally counterproductive.
Unfortunately Cygwin *does* define the WIN32 symbol in some headers.
Best to rely on a new git-specific symbol NATIVE_WINDOWS instead,
defined as follows:
#if defined(WIN32) && !defined(__CYGWIN__)
# define NATIVE_WINDOWS
#endif
After this change, it should be possible to drop the
CYGWIN_V15_WIN32API setting without any negative effect.
Signed-off-by: Jonathan Nieder
---
Eric Blake wrote:
> Which is why other projects, like gnulib, have
>
> # if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
>
> all over the place.
So, how about this?
Completely untested.
abspath.c | 2 +-
compat/terminal.c | 4 ++--
compat/win32.h| 2 +-
diff-no-index.c | 2 +-
git-compat-util.h | 3 ++-
help.c| 2 +-
run-command.c | 10 +-
test-chmtime.c| 2 +-
thread-utils.c| 2 +-
9 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/abspath.c b/abspath.c
index 40cdc462..c7d5458e 100644
--- a/abspath.c
+++ b/abspath.c
@@ -216,7 +216,7 @@ const char *absolute_path(const char *path)
const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
{
static char path[PATH_MAX];
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
if (!pfx_len || is_absolute_path(arg))
return arg;
memcpy(path, pfx, pfx_len);
diff --git a/compat/terminal.c b/compat/terminal.c
index 9b5e3d1b..136e4a74 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -3,7 +3,7 @@
#include "sigchain.h"
#include "strbuf.h"
-#if defined(HAVE_DEV_TTY) || defined(WIN32)
+#if defined(HAVE_DEV_TTY) || defined(WINDOWS_NATIVE)
static void restore_term(void);
@@ -53,7 +53,7 @@ error:
return -1;
}
-#elif defined(WIN32)
+#elif defined(WINDOWS_NATIVE)
#define INPUT_PATH "CONIN$"
#define OUTPUT_PATH "CONOUT$"
diff --git a/compat/win32.h b/compat/win32.h
index 8ce91048..31dd30f7 100644
--- a/compat/win32.h
+++ b/compat/win32.h
@@ -2,7 +2,7 @@
#define WIN32_H
/* common Win32 functions for MinGW and Cygwin */
-#ifndef WIN32 /* Not defined by Cygwin */
+#ifndef WINDOWS_NATIVE /* Not defined for Cygwin */
#include
#endif
diff --git a/diff-no-index.c b/diff-no-index.c
index 74da6593..a0bc9c50 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -45,7 +45,7 @@ static int get_mode(const char *path, int *mode)
if (!path || !strcmp(path, "/dev/null"))
*mode = 0;
-#ifdef _WIN32
+#ifdef WINDOWS_NATIVE
else if (!strcasecmp(path, "nul"))
*mode = 0;
#endif
diff --git a/git-compat-util.h b/git-compat-util.h
index e5a4b745..ebbdff53 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -85,10 +85,11 @@
#define _NETBSD_SOURCE 1
#define _SGI_SOURCE 1
-#ifdef WIN32 /* Both MinGW and MSVC */
+#if defined(WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */
#define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */
#include
#include
+#define WINDOWS_NATIVE
#endif
#include
diff --git a/help.c b/help.c
index 2a42ec6d..cc1e63f7 100644
--- a/help.c
+++ b/help.c
@@ -106,7 +106,7 @@ static int is_executable(const char *name)
!S_ISREG(st.st_mode))
return 0;
-#if defined(WIN32) || defined(__CYGWIN__)
+#if defined(WINDOWS_NATIVE) || defined(__CYGWIN__)
#if defined(__CYGWIN__)
if ((st.st_mode & S_IXUSR) == 0)
#endif
diff --git a/run-command.c b/run-command.c
index 04712191..04ac6181 100644
--- a/run-command.c
+++ b/run-command.c
@@ -72,7 +72,7 @@ static inline void close_pair(int fd[2])
close(fd[1]);
}
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
static inline void dup_devnull(int to)
{
int fd = open("/dev/null", O_RDWR);
@@ -159,7 +159,7 @@ static const char **prepare_shell_cmd(const char **argv)
die("BUG: shell command is empty");
if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) {
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
nargv[nargc++] = SHELL_PATH;
#else
nargv[nargc++] = "sh";
@@ -182,7 +182,7 @@ static const char **prepare_shell_cmd(const char **argv)
return nargv;
}
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
static int execv_shell_cmd(const char **argv)
{
const char **nargv = prepare_shell_cmd(argv);
@@ -193,7 +193,7 @@ static int execv_shell_cmd(const char **argv)
}
#endif
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
static int child_err = 2;
static int child_notifier = -1;
@@ -330,7 +330,7 @@ fail_pipe:
trace_argv_printf(cmd->argv, "trace: run_command:");
fflush(NULL);
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
{
int notify_pipe[2];
if (pipe(notify_pipe))
diff --git a/test-chmtime.c b/test
