Hi Junio,
On Mon, Aug 13, 2012 at 10:48:14AM -0700, Junio C Hamano wrote:
> Heiko Voigt <hvoigt@xxxxxxxxxx> writes:
> > What do you think?
>
> Does having the "stat()" help on Windows in any way? Does it ever
> return an executable bit by itself?
No, AFAIK it does not return anything about executability. But I think
the stat is still necessary to verify that the file exists and is a
regular file.
Here is a draft of a patch I would follow up with:
Subject: [PATCH RFC] help: extract platform dependent part of is_executable in
extra module
To remove various ifdefs required for the special handling of
executables on windows we create a new module 'executable' in compat
which allows a user to correctly fill the S_IXUSR flag of struct stat on
Windows.
Since this is valid for all variants of windows (mingw, msvc, cygwin) we
create a new module to avoid code duplication. compat/msvc.h includes
mingw.h so we do not add an extra include there. By default we define
correct_executeable_stat() to a no op on all other platforms. This is
guarded by a NO_EXECUTABLE_STAT which when defined by a compat header
requires and implementation of this function.
NOTE: I did not test this. I first would like to discuss whether the
overall structure this approach is taking is ok.
Signed-off-by: Heiko Voigt <hvoigt@xxxxxxxxxx>
---
Makefile | 8 +++++---
compat/cygwin.h | 2 ++
compat/mingw.h | 2 ++
compat/win32/executable.c | 33 +++++++++++++++++++++++++++++++++
compat/win32/executable.h | 9 +++++++++
git-compat-util.h | 4 ++++
help.c | 30 ++----------------------------
7 files changed, 57 insertions(+), 31 deletions(-)
create mode 100644 compat/win32/executable.c
create mode 100644 compat/win32/executable.h
diff --git a/Makefile b/Makefile
index 6b0c961..cea3559 100644
--- a/Makefile
+++ b/Makefile
@@ -1073,7 +1073,7 @@ ifeq ($(uname_O),Cygwin)
# Try commenting this out if you suspect MMAP is more efficient
NO_MMAP = YesPlease
X = .exe
- COMPAT_OBJS += compat/cygwin.o
+ COMPAT_OBJS += compat/cygwin.o compat/win32/executable.o
UNRELIABLE_FSTAT = UnfortunatelyYes
SPARSE_FLAGS = -isystem /usr/include/w32api -Wno-one-bit-signed-bitfield
endif
@@ -1257,7 +1257,8 @@ ifeq ($(uname_S),Windows)
BASIC_CFLAGS = -nologo -I. -I../zlib -Icompat/vcbuild -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/pthread.o compat/win32/syslog.o \
- compat/win32/poll.o compat/win32/dirent.o
+ compat/win32/poll.o compat/win32/dirent.o \
+ compat/win32/executable.o
COMPAT_CFLAGS = -D__USE_MINGW_ACCESS -DNOGDI -DHAVE_STRING_H -DHAVE_ALLOCA_H -Icompat -Icompat/regex -Icompat/win32 -DSTRIP_EXTENSION=\".exe\"
BASIC_LDFLAGS = -IGNORE:4217 -IGNORE:4049 -NOLOGO -SUBSYSTEM:CONSOLE -NODEFAULTLIB:MSVCRT.lib
EXTLIBS = user32.lib advapi32.lib shell32.lib wininet.lib ws2_32.lib
@@ -1347,7 +1348,8 @@ ifneq (,$(findstring MINGW,$(uname_S)))
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
COMPAT_OBJS += compat/mingw.o compat/winansi.o \
compat/win32/pthread.o compat/win32/syslog.o \
- compat/win32/poll.o compat/win32/dirent.o
+ compat/win32/poll.o compat/win32/dirent.o \
+ compat/win32/executable.o
EXTLIBS += -lws2_32
PTHREAD_LIBS =
X = .exe
diff --git a/compat/cygwin.h b/compat/cygwin.h
index a3229f5..e3bbd4d 100644
--- a/compat/cygwin.h
+++ b/compat/cygwin.h
@@ -1,6 +1,8 @@
#include <sys/types.h>
#include <sys/stat.h>
+#include "win32/executable.h"
+
typedef int (*stat_fn_t)(const char*, struct stat*);
extern stat_fn_t cygwin_stat_fn;
extern stat_fn_t cygwin_lstat_fn;
diff --git a/compat/mingw.h b/compat/mingw.h
index 61a6521..73c4f3d 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -1,6 +1,8 @@
#include <winsock2.h>
#include <ws2tcpip.h>
+#include "win32/executable.h"
+
/*
* things that are not available in header files
*/
diff --git a/compat/win32/executable.c b/compat/win32/executable.c
new file mode 100644
index 0000000..326ddb1
--- /dev/null
+++ b/compat/win32/executable.c
@@ -0,0 +1,33 @@
+#include "executeable.h"
+
+void correct_executable_stat(const char *filename, struct stat *st)
+{
+ char buf[3] = { 0 };
+ int n, fd;
+
+ /* On Windows we cannot use the executable bit. The executable
+ * state is determined by extension only. We do this first
+ * because with virus scanners opening an executeable for
+ * reading is potentially expensive.
+ */
+ if (has_extension(name, ".exe"))
+ st->st_mode |= S_IXUSR;
+
+ if (st.st_mode & S_IXUSR)
+ return;
+
+ /* now that we know it does not have an executable extension,
+ peek into the file instead */
+ fd = open(name, O_RDONLY);
+ if (fd < 0)
+ return;
+
+ n = read(fd, buf, 2);
+ if (n == 2) {
+ /* look for a she-bang */
+ if (!strcmp(buf, "#!"))
+ st.st_mode |= S_IXUSR;
+ }
+
+ close(fd);
+}
diff --git a/compat/win32/executable.h b/compat/win32/executable.h
new file mode 100644
index 0000000..e4f0c5c
--- /dev/null
+++ b/compat/win32/executable.h
@@ -0,0 +1,9 @@
+#ifndef COMPAT_WIN32_EXECUTEABLE
+#define COMPAT_WIN32_EXECUTEABLE
+
+#include "../../git-compat-util.h"
+
+#define NO_EXECUTABLE_STAT
+extern void correct_executable_stat(const char *filename, struct stat *st);
+
+#endif /* COMPAT_WIN32_EXECUTEABLE */
diff --git a/git-compat-util.h b/git-compat-util.h
index 35b095e..1b723af 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -607,4 +607,8 @@ int remove_or_warn(unsigned int mode, const char *path);
/* Get the passwd entry for the UID of the current process. */
struct passwd *xgetpwuid_self(void);
+#ifndef NO_EXECUTABLE_STAT
+#define correct_executable_stat(name, stat) ;
+#endif
+
#endif
diff --git a/help.c b/help.c
index ebc2c42..d9fae3c 100644
--- a/help.c
+++ b/help.c
@@ -106,34 +106,8 @@ static int is_executable(const char *name)
!S_ISREG(st.st_mode))
return 0;
-#if defined(WIN32) || defined(__CYGWIN__)
- /* On Windows we cannot use the executable bit. The executable
- * state is determined by extension only. We do this first
- * because with virus scanners opening an executeable for
- * reading is potentially expensive.
- */
- if (has_extension(name, ".exe"))
- return S_IXUSR;
-
-#if defined(__CYGWIN__)
-if ((st.st_mode & S_IXUSR) == 0)
-#endif
-{ /* now that we know it does not have an executable extension,
- peek into the file instead */
- char buf[3] = { 0 };
- int n;
- int fd = open(name, O_RDONLY);
- st.st_mode &= ~S_IXUSR;
- if (fd >= 0) {
- n = read(fd, buf, 2);
- if (n == 2)
- /* look for a she-bang */
- if (!strcmp(buf, "#!"))
- st.st_mode |= S_IXUSR;
- close(fd);
- }
-}
-#endif
+ correct_executable_stat(name, &st);
+
return st.st_mode & S_IXUSR;
}
--
1.7.12.rc2.11.g5d52328
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html