login
Search
So here it is, the MinGW port of git. This series is also available from git://repo.or.cz/git/mingw/j6t.git upstream http://repo.or.cz/w/git/mingw/j6t.git?a=shortlog;h=upstream Junio, please do *not* pull/apply this series into your tree(s) until we have verified that forks on repo.or.cz do not lose objects anymore when branches are rewound. What this series achieves: - An almost complete set of git tools running natively on Windows. - Functional git-gui and gitk. What is missing: - perl scripts don't work. We have a hack in the port tree, but I haven't yet taken the time to clean it up enough. - The test suite does not pass because of missing features like symbolic links. Again we have adjusted the tests in the port tree so that the test suite can be completed unattended. I've arranged the patches in a few subsets. They are different from what Dscho had proposed some weeks ago when I announced the series for the first time because I felt this topical partitioning makes more sense. * Part 1: Get it going 01/40 Add compat/regex.[ch] and compat/fnmatch.[ch]. 02/40 Compile some programs only conditionally. 03/40 Add target architecture MinGW. With these patches we have a working git.exe that can successfully run those builtins that need only read-access of the repository, like git log, git diff, etc. 7 files changed, 6222 insertions(+), 5 deletions(-) * Part 2: Working locally is possible 04/40 Windows: Use the Windows style PATH separator ';'. 05/40 Windows: Strip ".exe" from the program name. 06/40 Windows: Implement a wrapper of the open() function. 07/40 Windows: A minimal implemention of getpwuid(). 08/40 Windows: always chmod(, 0666) before unlink(). 09/40 Windows: Work around misbehaved rename(). 10/40 Windows: Treat Windows style path names. 11/40 Windows: Handle absolute paths in safe_create_leading_directories(). 12/40 Windows: Implement gettimeofday(). 13/40 Windows: Fix PRIuMAX definition. 14/40 Windows: Implement ...
Windows's rename() is based on the MoveFile() API, which fails if the
destination exists. Here we work around the problem by using MoveFileEx().
Furthermore, the posixly correct error is returned if the destination is
a directory.
The implementation is still slightly incomplete, however, because of the
missing error code translation: We assume that the failure is due to
permissions.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
compat/mingw.c | 25 +++++++++++++++++++++++++
git-compat-util.h | 3 +++
2 files changed, 28 insertions(+), 0 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 0c1d0e4..733ef87 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -60,6 +60,31 @@ struct tm *localtime_r(const time_t *timep, struct tm *result)
return result;
}
+#undef rename
+int mingw_rename(const char *pold, const char *pnew)
+{
+ /*
+ * Try native rename() first to get errno right.
+ * It is based on MoveFile(), which cannot overwrite existing files.
+ */
+ if (!rename(pold, pnew))
+ return 0;
+ if (errno != EEXIST)
+ return -1;
+ if (MoveFileEx(pold, pnew, MOVEFILE_REPLACE_EXISTING))
+ return 0;
+ /* TODO: translate more errors */
+ if (GetLastError() == ERROR_ACCESS_DENIED) {
+ DWORD attrs = GetFileAttributes(pnew);
+ if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_DIRECTORY)) {
+ errno = EISDIR;
+ return -1;
+ }
+ }
+ errno = EACCES;
+ return -1;
+}
+
struct passwd *getpwuid(int uid)
{
static char user_name[100];
diff --git a/git-compat-util.h b/git-compat-util.h
index 06ac2c1..f44a287 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -592,6 +592,9 @@ int sigaction(int sig, struct sigaction *in, struct sigaction *out);
int mingw_open (const char *filename, int oflags, ...);
#define open mingw_open
+int mingw_rename(const char*, const char*);
+#define rename mingw_rename
+
#endif /* __MINGW32__ */
#endif
--
1.5.4.1.126.ge5a7d
-
GIT's guts work with a forward slash as a path separators. We do not change
that. Rather we make sure that only "normalized" paths enter the depths
of the machinery.
We have to translate backslashes to forward slashes in the prefix and in
command line arguments. Fortunately, all of them are passed through
functions in setup.c.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
cache.h | 4 +++
compat/mingw.c | 16 ++++++++++++++
git-compat-util.h | 3 ++
setup.c | 59 +++++++++++++++++++++++++++++++++++++++++++---------
4 files changed, 71 insertions(+), 11 deletions(-)
diff --git a/cache.h b/cache.h
index e1000bc..3e4e10a 100644
--- a/cache.h
+++ b/cache.h
@@ -441,7 +441,11 @@ int safe_create_leading_directories(char *path);
char *enter_repo(char *path, int strict);
static inline int is_absolute_path(const char *path)
{
+#ifndef __MINGW32__
return path[0] == '/';
+#else
+ return path[0] == '/' || (path[0] && path[1] == ':');
+#endif
}
const char *make_absolute_path(const char *path);
diff --git a/compat/mingw.c b/compat/mingw.c
index 733ef87..71bca96 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -60,6 +60,22 @@ struct tm *localtime_r(const time_t *timep, struct tm *result)
return result;
}
+#undef getcwd
+char *mingw_getcwd(char *pointer, int len)
+{
+ char *ret = getcwd(pointer, len);
+ if (!ret)
+ return ret;
+ if (pointer[0] != 0 && pointer[1] == ':') {
+ int i;
+ for (i = 2; pointer[i]; i++)
+ /* Thanks, Bill. You'll burn in hell for that. */
+ if (pointer[i] == '\\')
+ pointer[i] = '/';
+ }
+ return ret;
+}
+
#undef rename
int mingw_rename(const char *pold, const char *pnew)
{
diff --git a/git-compat-util.h b/git-compat-util.h
index f44a287..3b57464 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -592,6 +592,9 @@ int sigaction(int sig, struct sigaction *in, struct sigaction *out);
int mingw_open (const char *filename, int oflags, ...);
#define open ...Hi,
Maybe "(isalpha(path[0]) && ...)"?
And maybe make this an inline function in git-compat-util.h, like this?
static inline int has_dos_drive_prefix(const char *path)
{
#ifdef __MINGW32__
return isalpha(*path) && path[1] == ':';
#else
return 0;
#endif
I think if you rename it to is_dir_separator(), you can put it into
git-compat-util.h, right after the PATH_SEPARATOR Paolo suggested.
Thanks,
Dscho
-
In this function we must be careful to handle drive-local paths else there
is a danger that it runs into an infinite loop.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
sha1_file.c | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 453bc43..0c60849 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -83,14 +83,20 @@ int get_sha1_hex(const char *hex, unsigned char *sha1)
return 0;
}
+static inline int offset_1st_component(const char *path)
+{
+#ifdef __MINGW32__
+ if (isalpha(path[0]) && path[1] == ':')
+ return 2 + (path[2] == '/');
+#endif
+ return *path == '/';
+}
+
int safe_create_leading_directories(char *path)
{
- char *pos = path;
+ char *pos = path + offset_1st_component(path);
struct stat st;
- if (is_absolute_path(path))
- pos++;
-
while (pos) {
pos = strchr(pos, '/');
if (!pos)
--
1.5.4.1.126.ge5a7d
-
For this purpose we make my_mktime of date.c public and use it to convert
a struct tm to a time_t.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
compat/mingw.c | 14 ++++++++++++++
date.c | 2 +-
2 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 71bca96..24f783b 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -38,6 +38,20 @@ int mkstemp(char *template)
int gettimeofday(struct timeval *tv, void *tz)
{
+ extern time_t my_mktime(struct tm *tm);
+ SYSTEMTIME st;
+ struct tm tm;
+ GetSystemTime(&st);
+ tm.tm_year = st.wYear-1900;
+ tm.tm_mon = st.wMonth-1;
+ tm.tm_mday = st.wDay;
+ tm.tm_hour = st.wHour;
+ tm.tm_min = st.wMinute;
+ tm.tm_sec = st.wSecond;
+ tv->tv_sec = my_mktime(&tm);
+ if (tv->tv_sec < 0)
+ return -1;
+ tv->tv_usec = st.wMilliseconds*1000;
return 0;
}
diff --git a/date.c b/date.c
index 8f70500..9c16eae 100644
--- a/date.c
+++ b/date.c
@@ -6,7 +6,7 @@
#include "cache.h"
-static time_t my_mktime(struct tm *tm)
+time_t my_mktime(struct tm *tm)
{
static const int mdays[] = {
0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334
--
1.5.4.1.126.ge5a7d
-
Since GIT calls into Microsoft's MSVCRT.DLL, it must use the printf format that this DLL uses for 64-bit integers, which is %I64u instead of %llu. Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at> --- git-compat-util.h | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index 3b57464..c576f5a 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -103,7 +103,11 @@ #endif #ifndef PRIuMAX +#ifndef __MINGW32__ #define PRIuMAX "llu" +#else +#define PRIuMAX "I64u" +#endif #endif #ifdef __GNUC__ -- 1.5.4.1.126.ge5a7d -
Hi, Would this not be better as a patch to Makefile, extending the COMPAT_CFLAGS with -DPRIuMAX=\"I64u\"? Ciao, Dscho -
The timer is implemented using a thread that calls the signal handler
at regular intervals.
We also replace Windows's signal() function because we must intercept
that SIGALRM is set (which is used when a timer is canceled).
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
compat/mingw.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
git-compat-util.h | 3 +
2 files changed, 113 insertions(+), 1 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 24f783b..4888a03 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -129,12 +129,121 @@ struct passwd *getpwuid(int uid)
return &p;
}
-int setitimer(int type, struct itimerval *in, struct itimerval *out)
+static HANDLE timer_event;
+static HANDLE timer_thread;
+static int timer_interval;
+static int one_shot;
+static sig_handler_t timer_fn = SIG_DFL;
+
+/* The timer works like this:
+ * The thread, ticktack(), is basically a trivial routine that most of the
+ * time only waits to receive the signal to terminate. The main thread
+ * tells the thread to terminate by setting the timer_event to the signalled
+ * state.
+ * But ticktack() does not wait indefinitely; instead, it interrupts the
+ * wait state every now and then, namely exactly after timer's interval
+ * length. At these opportunities it calls the signal handler.
+ */
+
+static __stdcall unsigned ticktack(void *dummy)
+{
+ while (WaitForSingleObject(timer_event, timer_interval) == WAIT_TIMEOUT) {
+ if (timer_fn == SIG_DFL)
+ die("Alarm");
+ if (timer_fn != SIG_IGN)
+ timer_fn(SIGALRM);
+ if (one_shot)
+ break;
+ }
+ return 0;
+}
+
+static int start_timer_thread(void)
{
+ timer_event = CreateEvent(NULL, FALSE, FALSE, NULL);
+ if (timer_event) {
+ timer_thread = (HANDLE) _beginthreadex(NULL, 0, ticktack, NULL, 0, NULL);
+ if (!timer_thread )
+ return errno = ENOMEM,
+ error("cannot start timer thread");
+ } else
+ return errno = ENOMEM,
+ error("cannot allocate resources ...On Windows, vsnprintf deviates in two regards from the "usual" behavior
that its callers in GIT expect:
- It returns -1 if the buffer is too small instead of the number of
characters that the operation produces.
- The size parameter is the number of characters to write, which does not
include the trailing NUL byte, instead of the available space.
This wrapper computes the needed buffer size by trying various sizes with
exponential growth. A large growth factor is used so as only few trials are
required if a really large result needs to be stored.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
compat/mingw.c | 34 ++++++++++++++++++++++++++++++++++
git-compat-util.h | 3 +++
2 files changed, 37 insertions(+), 0 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 4888a03..77e4b83 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -115,6 +115,40 @@ int mingw_rename(const char *pold, const char *pnew)
return -1;
}
+#undef vsnprintf
+/* Note that the size parameter specifies the available space, i.e.
+ * includes the trailing NUL byte; but Windows's vsnprintf expects the
+ * number of characters to write without the trailing NUL.
+ */
+
+/* This is out of line because it uses alloca() behind the scenes,
+ * which must not be called in a loop (alloca() reclaims the allocations
+ * only at function exit).
+ */
+static int try_vsnprintf(size_t size, const char *fmt, va_list args)
+{
+ char buf[size]; /* gcc-ism */
+ return vsnprintf(buf, size-1, fmt, args);
+}
+
+int mingw_vsnprintf(char *buf, size_t size, const char *fmt, va_list args)
+{
+ int len;
+ if (size > 0) {
+ len = vsnprintf(buf, size-1, fmt, args);
+ if (len >= 0)
+ return len;
+ }
+ /* ouch, buffer too small; need to compute the size */
+ if (size < 250)
+ size = 250;
+ do {
+ size *= 4;
+ len = try_vsnprintf(size, fmt, args);
+ } while (len < 0);
+ return len;
+}
+
struct passwd *getpwuid(int uid)
{
static char user_name[100];
diff ...When an external git command is invoked, it can be a Bourne shell script.
This patch looks into the command file to see whether it is one.
In this case, the command line is rearranged to invoke the shell
with the proper arguments.
With this change, scripted git commands work. Command line arguments
to those scripts cannot be complex (contain spaces or double-quotes), yet.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
compat/mingw.c | 246 +++++++++++++++++++++++++++++++++++++++++++++++++++++
git-compat-util.h | 3 +
2 files changed, 249 insertions(+), 0 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 77e4b83..9d03ea5 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -90,6 +90,252 @@ char *mingw_getcwd(char *pointer, int len)
return ret;
}
+static const char *parse_interpreter(const char *cmd)
+{
+ static char buf[100];
+ char *p, *opt;
+ int n, fd;
+
+ /* don't even try a .exe */
+ n = strlen(cmd);
+ if (n >= 4 && !strcasecmp(cmd+n-4, ".exe"))
+ return NULL;
+
+ fd = open(cmd, O_RDONLY);
+ if (fd < 0)
+ return NULL;
+ n = read(fd, buf, sizeof(buf)-1);
+ close(fd);
+ if (n < 4) /* at least '#!/x' and not error */
+ return NULL;
+
+ if (buf[0] != '#' || buf[1] != '!')
+ return NULL;
+ buf[n] = '\0';
+ p = strchr(buf, '\n');
+ if (!p)
+ return NULL;
+
+ *p = '\0';
+ if (!(p = strrchr(buf+2, '/')) && !(p = strrchr(buf+2, '\\')))
+ return NULL;
+ /* strip options */
+ if ((opt = strchr(p+1, ' ')))
+ *opt = '\0';
+ return p+1;
+}
+
+/*
+ * Splits the PATH into parts.
+ */
+static char **get_path_split(void)
+{
+ char *p, **path, *envpath = getenv("PATH");
+ int i, n = 0;
+
+ if (!envpath || !*envpath)
+ return NULL;
+
+ envpath = xstrdup(envpath);
+ p = envpath;
+ while (p) {
+ char *dir = p;
+ p = strchr(p, ';');
+ if (p) *p++ = '\0';
+ if (*dir) { /* not earlier, catches series of ; */
+ ++n;
+ }
+ }
+ if (!n)
+ return NULL;
+
+ path = xmalloc((n+1)*sizeof(char*));
+ p ...On Unix the idiom to use a pipe is as follows:
pipe(fd);
pid = fork();
if (!pid) {
dup2(fd[1], 1);
close(fd[1]);
close(fd[0]);
...
}
close(fd[1]);
i.e. the child process closes the both pipe ends after duplicating one
to the file descriptors where they are needed.
On Windows, which does not have fork(), we never have an opportunity to
(1) duplicate a pipe end in the child, (2) close unused pipe ends. Instead,
we must use this idiom:
save1 = dup(1);
pipe(fd);
dup2(fd[1], 1);
spawn(...);
dup2(save1, 1);
close(fd[1]);
i.e. save away the descriptor at the destination slot, replace by the pipe
end, spawn process, restore the saved file.
But there is a problem: Notice that the child did not only inherit the
dup2()ed descriptor, but also *both* original pipe ends. Although the one
end that was dup()ed could be closed before the spawn(), we cannot close
the other end - the child inherits it, no matter what.
The solution is to generate non-inheritable pipes. At the first glance,
this looks strange: The purpose of pipes is usually to be inherited to
child processes. But notice that in the course of actions as outlined
above, the pipe descriptor that we want to inherit to the child is
dup2()ed, and as it so happens, Windows's dup2() creates inheritable
duplicates.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
compat/mingw.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
git-compat-util.h | 5 +----
2 files changed, 46 insertions(+), 4 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 9d03ea5..fe6f6ce 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -55,6 +55,51 @@ int gettimeofday(struct timeval *tv, void *tz)
return 0;
}
+int pipe(int filedes[2])
+{
+ int fd;
+ HANDLE h[2], parent;
+
+ if (_pipe(filedes, 8192, 0) < 0)
+ return -1;
+
+ parent = GetCurrentProcess();
+
+ if (!DuplicateHandle (parent, ...On Windows, we have spawnv() variants to run a child process instead of
fork()/exec(). In order to attach pipe ends to stdin, stdout, and stderr,
we have to use this idiom:
save1 = dup(1);
dup2(pipe[1], 1);
spawnv();
dup2(save1, 1);
close(pipe[1]);
assuming that the descriptors created by pipe() are not inheritable.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
git-compat-util.h | 8 +++++
run-command.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 87 insertions(+), 10 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index 170c279..046891d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -608,6 +608,14 @@ void mingw_execvp(const char *cmd, char *const *argv);
sig_handler_t mingw_signal(int sig, sig_handler_t handler);
#define signal mingw_signal
+/*
+ * helpers
+ */
+
+char **copy_environ(void);
+void free_environ(char **env);
+char **env_setenv(char **env, const char *name);
+
#endif /* __MINGW32__ */
#endif
diff --git a/run-command.c b/run-command.c
index 476d00c..873f6d0 100644
--- a/run-command.c
+++ b/run-command.c
@@ -53,17 +53,8 @@ int start_command(struct child_process *cmd)
cmd->err = fderr[0];
}
+#ifndef __MINGW32__
cmd->pid = fork();
- if (cmd->pid < 0) {
- if (need_in)
- close_pair(fdin);
- if (need_out)
- close_pair(fdout);
- if (need_err)
- close_pair(fderr);
- return -ERR_RUN_COMMAND_FORK;
- }
-
if (!cmd->pid) {
if (cmd->no_stdin)
dup_devnull(0);
@@ -112,6 +103,84 @@ int start_command(struct child_process *cmd)
}
die("exec %s failed.", cmd->argv[0]);
}
+#else
+ int s0 = -1, s1 = -1, s2 = -1; /* backups of stdin, stdout, stderr */
+ const char *sargv0 = cmd->argv[0];
+ char **env = environ;
+ struct strbuf git_cmd;
+
+ if (cmd->no_stdin) {
+ s0 = dup(0);
+ dup_devnull(0);
+ } else if (need_in) {
+ s0 = dup(0);
+ dup2(fdin[0], 0);
+ } else if (cmd->in) {
+ s0 = ...We don't have fnmatch and regular expressions on Windows. We borrow fnmatch.[ch] from the GNU C library (license is LGPL 2 or later) and GNU regexp (regexp.c[ch], license is GPL 2 or later). Note that regexp.c was changed slightly to avoid warnings with gcc. We make the addition of these files an extra commit so as not to clutter the next commits. Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at> --- This patch is intentially broken. There's no need to comment on the details of the files that it adds. -- Hannes compat/fnmatch.c | 488 ++++++ compat/fnmatch.h | 84 + compat/regex.c | 4927 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ compat/regex.h | 490 ++++++ 4 files changed, 5989 insertions(+), 0 deletions(-) create mode 100644 compat/fnmatch.c create mode 100644 compat/fnmatch.h create mode 100644 compat/regex.c create mode 100644 compat/regex.h diff --git a/compat/fnmatch.c b/compat/fnmatch.c new file mode 100644 index 0000000..1f4ead5 --- /dev/null +++ b/compat/fnmatch.c @@ -0,0 +1,488 @@ This patch is intentionally broken. diff --git a/compat/fnmatch.h b/compat/fnmatch.h new file mode 100644 index 0000000..cc3ec37 --- /dev/null +++ b/compat/fnmatch.h @@ -0,0 +1,84 @@ This patch is intentionally broken. diff --git a/compat/regex.c b/compat/regex.c new file mode 100644 index 0000000..1d39e08 --- /dev/null +++ b/compat/regex.c @@ -0,0 +1,4927 @@ This patch is intentionally broken. diff --git a/compat/regex.h b/compat/regex.h new file mode 100644 index 0000000..408dd21 --- /dev/null +++ b/compat/regex.h @@ -0,0 +1,490 @@ This patch is intentionally broken. -- 1.5.4.1.126.ge5a7d -
Hi, Heh. You mean to say: "all bugs you find here are _not_ ours!" ;-) Ciao, Dscho -
Since on Windows there is no 'executable' bit whose absence would deny
execution of a script, we must change the hook scripts' names entirely
to inhibit that they can be invoked by the tools.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
Makefile | 3 ++-
templates/Makefile | 8 +++++++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 069939f..7d4a591 100644
--- a/Makefile
+++ b/Makefile
@@ -547,6 +547,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
COMPAT_OBJS += compat/mingw.o compat/fnmatch.o compat/regex.o
EXTLIBS += -lws2_32
X = .exe
+ NOEXECTEMPL = .noexec
endif
ifneq (,$(findstring arm,$(uname_M)))
ARM_SHA1 = YesPlease
@@ -849,7 +850,7 @@ ifndef NO_TCLTK
$(QUIET_SUBDIR0)gitk-git $(QUIET_SUBDIR1) all
endif
$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all
- $(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1)
+ $(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1) NOEXECTEMPL='$(NOEXECTEMPL)'
strip: $(PROGRAMS) git$X
$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
diff --git a/templates/Makefile b/templates/Makefile
index ebd3a62..b341105 100644
--- a/templates/Makefile
+++ b/templates/Makefile
@@ -10,6 +10,8 @@ RM ?= rm -f
prefix ?= $(HOME)
template_dir ?= $(prefix)/share/git-core/templates
# DESTDIR=
+# set NOEXECTEMPL to non-empty to change the names of hook scripts
+# so that the tools will not find them
# Shell quote (do not use $(call) to accommodate ancient setups);
DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
@@ -32,7 +34,11 @@ boilerplates.made : $(bpsrc)
mkdir -p blt/$$dir && \
case "$$boilerplate" in \
*--) ;; \
- *) cp $$boilerplate blt/$$dst ;; \
+ *) if test -n "$$(sed -ne '/^#!\//p' -e '1q' < "$$boilerplate")"; then \
+ cp "$$boilerplate" "blt/$${dst}$(NOEXECTEMPL)"; \
+ else \
+ cp "$$boilerplate" "blt/$$dst"; \
+ fi ;; \
esac || exit; \
done && \
date >$@
--
1.5.4.1.126.ge5a7d
-
Hi, Why not just append .noexec to all of the hooks? Ciao, Dscho --
Because some of the files are not hooks. Or do you mean to check for the files hooks--* explicitly? -- Hannes --
Hi, Then you have to make sure even more urgently that only the hooks are treated that way. Ciao, Dscho --
This emulation of poll() is by far not general. It assumes that the
fds that are to be waited for are connected to pipes. The pipes are
polled in a loop until data becomes available in at least one of them.
If only a single fd is waited for, the implementation actually does
not wait at all, but assumes that a subsequent read() will block.
In order not to needlessly burn CPU time, the CPU is yielded to other
processes before the next round in the poll loop using Sleep(0). Note that
any sleep timeout greater than zero will reduce the efficiency by a
magnitude.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
compat/mingw.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 55 insertions(+), 0 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index fe6f6ce..837d741 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -102,6 +102,61 @@ int pipe(int filedes[2])
int poll(struct pollfd *ufds, unsigned int nfds, int timeout)
{
+ int i, pending;
+
+ if (timeout != -1)
+ return errno = EINVAL, error("poll timeout not supported");
+
+ /* When there is only one fd to wait for, then we pretend that
+ * input is available and let the actual wait happen when the
+ * caller invokes read().
+ */
+ if (nfds == 1) {
+ if (!(ufds[0].events & POLLIN))
+ return errno = EINVAL, error("POLLIN not set");
+ ufds[0].revents = POLLIN;
+ return 0;
+ }
+
+repeat:
+ pending = 0;
+ for (i = 0; i < nfds; i++) {
+ DWORD avail = 0;
+ HANDLE h = (HANDLE) _get_osfhandle(ufds[i].fd);
+ if (h == INVALID_HANDLE_VALUE)
+ return -1; /* errno was set */
+
+ if (!(ufds[i].events & POLLIN))
+ return errno = EINVAL, error("POLLIN not set");
+
+ /* this emulation works only for pipes */
+ if (!PeekNamedPipe(h, NULL, 0, NULL, &avail, NULL)) {
+ int err = GetLastError();
+ if (err == ERROR_BROKEN_PIPE) {
+ ufds[i].revents = POLLHUP;
+ pending++;
+ } else {
+ errno = EINVAL;
+ return error("PeekNamedPipe ...For the future, would it be better to first use WaitForMultipleObjects, and then use PeekNamedPipe to find which handles have data on it? That's how the mingw port of GNU Smalltalk does it. Paolo -
I tried but I failed. If you can show me code where WaitForMultipleObjects works on handles that MSVCRT.DLL's open() created, I'll gladly accept it! -- Hannes --
Have you checked _get_osfhandle? There's a warning though in the knowledgebase with number 99173, though. There's also another function that works the other way, _open_osfhandle. -- robin --
Handwaving you are! This stuff is well-known. Show me code. -- Hannes --
If on Windows a path is specified as C:/path, then this is also a valid
SSH URL. To disambiguate between the two interpretations we require that
a SSH URL must have a host name with at least two characters.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
connect.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/connect.c b/connect.c
index 5ac3572..7e18ac8 100644
--- a/connect.c
+++ b/connect.c
@@ -529,7 +529,13 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
end = host;
path = strchr(end, c);
- if (path) {
+#ifdef __MINGW32__
+ /* host must have at least 2 chars to catch DOS C:/path */
+ if (path && path - end > 1)
+#else
+ if (path)
+#endif
+ {
if (c == ':') {
protocol = PROTO_SSH;
*path++ = '\0';
--
1.5.4.1.126.ge5a7d
-
Hi, This could be un-#ifdef'ed by using the has_dos_drive_prefix() function I suggested earlier. Ciao, Dscho --
Ah, great! I'm convinced. -- Hannes --
In upload-pack we must explicitly close the output channel of rev-list.
(On Unix, the channel is closed automatically because process that runs
rev-list terminates.)
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
run-command.c | 29 ++++++++++++++++++++++++++++-
run-command.h | 5 +++++
upload-pack.c | 2 ++
3 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/run-command.c b/run-command.c
index 873f6d0..3834f86 100644
--- a/run-command.c
+++ b/run-command.c
@@ -276,13 +276,23 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
return run_command(&cmd);
}
+#ifdef __MINGW32__
+static __stdcall unsigned run_thread(void *data)
+{
+ struct async *async = data;
+ return async->proc(async->fd_for_proc, async->data);
+}
+#endif
+
int start_async(struct async *async)
{
int pipe_out[2];
if (pipe(pipe_out) < 0)
return error("cannot create pipe: %s", strerror(errno));
+ async->out = pipe_out[0];
+#ifndef __MINGW32__
async->pid = fork();
if (async->pid < 0) {
error("fork (async) failed: %s", strerror(errno));
@@ -293,16 +303,33 @@ int start_async(struct async *async)
close(pipe_out[0]);
exit(!!async->proc(pipe_out[1], async->data));
}
- async->out = pipe_out[0];
close(pipe_out[1]);
+#else
+ async->fd_for_proc = pipe_out[1];
+ async->tid = (HANDLE) _beginthreadex(NULL, 0, run_thread, async, 0, NULL);
+ if (!async->tid) {
+ error("cannot create thread: %s", strerror(errno));
+ close_pair(pipe_out);
+ return -1;
+ }
+#endif
return 0;
}
int finish_async(struct async *async)
{
+#ifndef __MINGW32__
int ret = 0;
if (wait_or_whine(async->pid))
ret = error("waitpid (async) failed");
+#else
+ DWORD ret = 0;
+ if (WaitForSingleObject(async->tid, INFINITE) != WAIT_OBJECT_0)
+ ret = error("waiting for thread failed: %lu", GetLastError());
+ else if (!GetExitCodeThread(async->tid, &ret))
+ ret = error("cannot get thread exit code: %lu", ...Hi, When I read this patch, my impression was that it litters the source code with #ifdef's. IMO this makes the code less readable, and as a consequence easer to fsck up. Unfortunately, I have no idea how to help that, other than implementing compat/thread.[ch], abstracting the thread functions, and introducing a NO_FORK Makefile variable and preprocessor constant. Hmpf. Ciao, Dscho --
The number of #ifdef/#endif is already at a minimum unless you are willing to have entire functions in separate #ifdef/#else/#endif branches. Whether to have compat/thread.[ch] or not is just a question of whether you want to have asynchronous functions in threads also on Unix or not. -- Hannes --
Hi, Well, my idea was that Git's code looks as good as it does now not only because the functions are well written, but because the conditionally-compiled code is not classified by _platform_ but by _reason_. For example, we do not write that on FreeBSD, we do this and that, but we define for FreeBSD that there is no strlcpy(), and if NO_STRLCPY is defined, we implement it. Likewise, I would prefer to have it in the code here _not_ that we do this and that on MinGW32, but that in the absence of a usable fork(), we require certain functions (declared in compat/thread.h, for example), and use them. But I know this is a lot of work, so I hoped for a better solution, that still excites my "elegance neuron". Ciao, Dscho --
This reminds me of a crazy idea and I know it would be quite a bit of
work. Do you think it would be possible to change git to use Apache's
apr library? The benefit would be that the cross platform stuff is
already written. I understand that it would not be tweaked to be
optimized for git's usage. But it would allow people to focus on the
heart of the git and not the way git interacts with the OS level stuff.
Obviously, the scripts that make up part of git are not as portable but
that is a different animal unless you want to either support a cross
platform scripting language as part of git.
As I said it is a crazy idea.
Paul Franz
--
-------------------------------------------
There are seven sins in the world.
Wealth without work.
Pleasure without conscience.
Knowledge without character.
Commerce without morality.
Science without humanity.
Worship without sacrifice.
Politics without principle.
-- Mohandas Gandhi
-------------------------------------------
--
Hi, Possible? Yes. Sensible? That's another issue. For one, we already rely on a pretty good POSIX shell support. That is not something helped by APR. So if we already need a POSIX shell, we might just as well rely on a (kind of) POSIX environment. Besides, I cannot say that I found compiling APR in msysGit _easy_. So if there are as many disadvantages as I suspect, and as few advantages, as I suspect, too, I think it would be a waste of time to bother to port Git to APR. Besides, I think there are -- again, as always when things seem to be only fun -- legal issues. I do not know the details, but there are people who say that APL is incompatible with GPL (v2). Not that I care too much about those bastards who devised licenses just to make my life miserable. But them lawyer types have ways to _make_ my life miserable. So I avoid those kind of problems. I guess you could call this "collateral legal damage". Ciao, Dscho --
True there are the legal issues. And it is not known if it would help on
other platforms. My may concern is Windows because if git could be made
available without some of msysGit's requirements. It would definitely
make it more palletable (sp?) to that community which is a large chunk
of developers, including my own work environment. I would love to switch
from ClearCase to git but there are issues. One of them is integration
with the Windows environment.
Another concern is performance penalty of using non-native calls under
Windows and that is one of the problems that I was hoping the using APR
would solve. I figure if it is good enough to support the network stuff
under Windows well maybe it would be good for git too.
Paul Franz
--
-------------------------------------------
There are seven sins in the world.
Wealth without work.
Pleasure without conscience.
Knowledge without character.
Commerce without morality.
Science without humanity.
Worship without sacrifice.
Politics without principle.
-- Mohandas Gandhi
-------------------------------------------
--
Hi, [top-posting?] I think that you do not help the effort to bring Git to Windows, by suggesting even more work, for dubitable benefits. And that is what I think of APR here. NOTE: my only experience with APR on Windows is to get git-svn to compile, The problems with Git and Windows are not network related. They are with Windows' pitiful performance when it comes to spawn processes. IMO your efforts to help Git on Windows would be better spent on the msysGit list, where you would learn about the real issues we face. Ciao, Dscho --
Which makes me believe that it sounds like git depends on spawning
Excuse my ignorance, could you tell me where the mysGit list is, so
--
-------------------------------------------
There are seven sins in the world.
Wealth without work.
Pleasure without conscience.
Knowledge without character.
Commerce without morality.
Science without humanity.
Worship without sacrifice.
Politics without principle.
-- Mohandas Gandhi
-------------------------------------------
--
Thanks. Noted for future reference.
--
-------------------------------------------
There are seven sins in the world.
Wealth without work.
Pleasure without conscience.
Knowledge without character.
Commerce without morality.
Science without humanity.
Worship without sacrifice.
Politics without principle.
-- Mohandas Gandhi
-------------------------------------------
--
Hi, My favourite way to describe it (a quote): +A: Because it messes up the order in which people normally read text. +Q: Why is top-posting such a bad thing? +A: Top-posting. It is referenced right on the first page on http://msysgit.googlecode.com/ under the keyword "Groups:". Hth, Dscho --
The default of pwd of MSYS's bash and /bin/pwd are to use /c/rest/of/path
notation instead of c:/rest/of/path. But the former is not supported
by programs that use the standard C runtime (instead of MSYS's
runtime). Hence, we must make sure that only drive letter notations
are generated by using pwd's -W option.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
git-clone.sh | 21 ++++++++++++++++++++-
1 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/git-clone.sh b/git-clone.sh
index 0d686c3..640e29d 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -32,6 +32,25 @@ die() {
exit 1
}
+# Fix some commands on Windows
+case $(uname -s) in
+*MINGW*)
+ # pwd must return a path with a drive letter
+ bin_pwd() {
+ # there are no symlinks to resolve: /bin/pwd is not needed
+ builtin pwd -W
+ }
+ pwd() {
+ builtin pwd -W
+ }
+ ;;
+*)
+ bin_pwd() {
+ /bin/pwd
+ }
+ ;;
+esac
+
usage() {
exec "$0" -h
}
@@ -40,7 +59,7 @@ eval "$(echo "$OPTIONS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)
get_repo_base() {
(
- cd "`/bin/pwd`" &&
+ cd "$(bin_pwd)" &&
cd "$1" || cd "$1.git" &&
{
cd .git
--
1.5.4.1.126.ge5a7d
-
Hi, I guess that this patch will be obsolete when we have builtin clone. So I have no objection to this pretty local workaround until then. Ciao, Dscho --
If the PATH lists the Windows system directories before the MSYS
directories, Windows's own incompatible sort and find commands would be
picked up. We implement these commands as functions and call the real
tools by absolute path.
Also add a dummy implementation of sync to avoid an error in git-repack.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
git-clone.sh | 4 ++++
git-sh-setup.sh | 17 +++++++++++++++++
t/test-lib.sh | 13 +++++++++++++
3 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/git-clone.sh b/git-clone.sh
index 640e29d..1fc5c92 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -35,6 +35,10 @@ die() {
# Fix some commands on Windows
case $(uname -s) in
*MINGW*)
+ # Windows has its own (incompatible) find
+ find () {
+ /usr/bin/find "$@"
+ }
# pwd must return a path with a drive letter
bin_pwd() {
# there are no symlinks to resolve: /bin/pwd is not needed
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index a44b1c7..822aa6f 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -142,3 +142,20 @@ then
}
: ${GIT_OBJECT_DIRECTORY="$GIT_DIR/objects"}
fi
+
+# Fix some commands on Windows
+case $(uname -s) in
+*MINGW*)
+ # Windows has its own (incompatible) sort and find
+ sort () {
+ /usr/bin/sort "$@"
+ }
+ find () {
+ /usr/bin/find "$@"
+ }
+ # sync is missing
+ sync () {
+ : # no implementation
+ }
+ ;;
+esac
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 83889c4..e2010d5 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -366,3 +366,16 @@ do
test_done
esac
done
+
+# Fix some commands on Windows
+case $(uname -s) in
+*MINGW*)
+ # Windows has its own (incompatible) sort and find
+ sort () {
+ /usr/bin/sort "$@"
+ }
+ find () {
+ /usr/bin/find "$@"
+ }
+ ;;
+esac
--
1.5.4.1.126.ge5a7d
-
We use 'xargs cp' to emulate 'cpio -pumd'.
cpio is fed from the output of 'find --depth' without filtering out
directories. We don't need to copy directories except the empty ones, so
we need to filter out non-empty directories. Then we can use 'cp -r' and
it will not actually recurse anything.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
git-clone.sh | 27 +++++++++++++++++++++++++++
1 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/git-clone.sh b/git-clone.sh
index 1fc5c92..87fc0fb 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -39,6 +39,33 @@ case $(uname -s) in
find () {
/usr/bin/find "$@"
}
+ # need an emulation of cpio
+ cpio() {
+ case "$1" in
+ -pumd) cp_arg=-pr;;
+ -pumdl) cp_arg=-lr;;
+ *) die "cpio $1 unexpected";;
+ esac
+ # copy only files and empty directories
+ prev=
+ while read f; do
+ if test -d "$f"; then
+ # here we assume that directories are listed after
+ # its files (aka 'find -depth'), hence, a directory
+ # that is not empty will be a leading sub-string
+ # of the preceding entry
+ case "$prev" in
+ "$f"/* ) ;;
+ *) echo "$f";;
+ esac
+ else
+ echo "$f"
+ fi
+ prev="$f"
+ done |
+ xargs --no-run-if-empty \
+ cp $cp_arg --target-directory="$2" --parents
+ }
# pwd must return a path with a drive letter
bin_pwd() {
# there are no symlinks to resolve: /bin/pwd is not needed
--
1.5.4.1.126.ge5a7d
-
gethostbyname() is the first function that calls into the Winsock library,
and it is wrapped only to initialize the library.
socket() is wrapped for two reasons:
- Windows's socket() creates things that are like low-level file handles,
and they must be converted into file descriptors first.
- And these handles cannot be used with plain ReadFile()/WriteFile()
because they are opened for "overlapped IO". We have to use WSASocket()
to create non-overlapped IO sockets.
connect() must be wrapped because Windows's connect() expects the low-level
sockets, not file descriptors, and we must first unwrap the file descriptor
before we can pass it on to Windows's connect().
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
compat/mingw.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
git-compat-util.h | 9 +++++++++
2 files changed, 55 insertions(+), 0 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 837d741..1260be8 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -436,6 +436,52 @@ char **env_setenv(char **env, const char *name)
return env;
}
+/* this is the first function to call into WS_32; initialize it */
+#undef gethostbyname
+struct hostent *mingw_gethostbyname(const char *host)
+{
+ WSADATA wsa;
+
+ if (WSAStartup(MAKEWORD(2,2), &wsa))
+ die("unable to initialize winsock subsystem, error %d",
+ WSAGetLastError());
+ atexit((void(*)(void)) WSACleanup);
+ return gethostbyname(host);
+}
+
+int mingw_socket(int domain, int type, int protocol)
+{
+ int sockfd;
+ SOCKET s = WSASocket(domain, type, protocol, NULL, 0, 0);
+ if (s == INVALID_SOCKET) {
+ /*
+ * WSAGetLastError() values are regular BSD error codes
+ * biased by WSABASEERR.
+ * However, strerror() does not know about networking
+ * specific errors, which are values beginning at 38 or so.
+ * Therefore, we choose to leave the biased error code
+ * in errno so that _if_ someone looks up the code somewhere,
+ * then it is at least the ...The problem with Windows's own implementation is that it tries to be
clever when a console program is invoked from a GUI application: In this
case it sometimes automatically allocates a new console window. As a
consequence, the IO channels of the spawned program are directed to the
console, but the invoking application listens on channels that are now
directed to nowhere.
In this implementation we use the lowlevel facilities of CreateProcess(),
which offers a flag to tell the system not to open a console. As a side
effect, only stdin, stdout, and stderr channels will be accessible from
C programs that are spawned. Other channels (file handles, pipe handles,
etc.) are still inherited by the spawned program, but it doesn't get
enough information to access them.
Johannes Schindelin integrated path quoting and unified the various
*execv* and *spawnv* helpers.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
compat/mingw.c | 203 +++++++++++++++++++++++++++++++++++++++++++++++++++--
git-compat-util.h | 1 +
run-command.c | 2 +-
3 files changed, 199 insertions(+), 7 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 1260be8..146c170 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1,4 +1,5 @@
#include "../git-compat-util.h"
+#include "../strbuf.h"
unsigned int _CRT_fmode = _O_BINARY;
@@ -190,6 +191,65 @@ char *mingw_getcwd(char *pointer, int len)
return ret;
}
+/*
+ * See http://msdn2.microsoft.com/en-us/library/17w5ykft(vs.71).aspx
+ * (Parsing C++ Command-Line Arguments)
+ */
+static const char *quote_arg(const char *arg)
+{
+ /* count chars to quote */
+ int len = 0, n = 0;
+ int force_quotes = 0;
+ char *q, *d;
+ const char *p = arg;
+ if (!*p) force_quotes = 1;
+ while (*p) {
+ if (isspace(*p) || *p == '*' || *p == '?')
+ force_quotes = 1;
+ else if (*p == '"')
+ n++;
+ else if (*p == '\\') {
+ int count = 0;
+ while (*p == '\\') {
+ count++;
+ p++;
+ len++;
+ }
+ if ...Hi, Should this not be "#define spawnvpe mingw_spanvpe" in git-compat-util.h instead? Ciao, Dscho --
No. We don't mimic the API of the original spawnvpe. Why obfuscate this fact? -- Hannes --
Hi, Oops. I forgot that. Maybe we do not want to call the function mingw_spawnvpe(), then? (That fact is what made me believe that the #define was a good idea, after all.) Ciao, Dscho --
From: Marius Storm-Olsen <mstormo_git@storm-olsen.com> From: Marius Storm-Olsen <mstormo_git@storm-olsen.com> This gives us a significant speedup when adding, committing and stat'ing files. Also, since Windows doesn't really handle symlinks, we let stat just uses lstat. We also need to replace fstat, since our implementation and the standard stat() functions report slightly different timestamps, possibly due to timezones. We simply report UTC in our implementation, and do our FILETIME to time_t conversion based on the document at http://support.microsoft.com/kb/167296. With Moe's repo structure (100K files in 100 dirs, containing 2-4 bytes) mkdir bummer && cd bummer; for ((i=0;i<100;i++)); do mkdir $i && pushd $i; for ((j=0;j<1000;j++)); do echo "$j" >$j; done; popd; done We get the following performance boost: With normal lstat & stat Custom lstat/fstat ------------------------ ------------------------ Command: git init Command: git init ------------------------ ------------------------ real 0m 0.047s real 0m 0.063s user 0m 0.031s user 0m 0.015s sys 0m 0.000s sys 0m 0.015s ------------------------ ------------------------ Command: git add . Command: git add . ------------------------ ------------------------ real 0m19.390s real 0m12.031s 1.6x user 0m 0.015s user 0m 0.031s sys 0m 0.030s sys 0m 0.000s ------------------------ ------------------------ Command: git commit -a.. Command: git commit -a.. ------------------------ ------------------------ real 0m30.812s real 0m16.875s 1.8x user 0m 0.015s user 0m 0.015s sys 0m 0.000s sys 0m 0.015s ------------------------ ------------------------ 3x Command: git-status 3x Command: git-status ------------------------ ...
On MinGW, we won't compile some programs. Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at> --- Makefile | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index d33a556..870a862 100644 --- a/Makefile +++ b/Makefile @@ -255,7 +255,6 @@ PROGRAMS = \ git-fetch-pack$X \ git-hash-object$X git-index-pack$X \ git-fast-import$X \ - git-daemon$X \ git-merge-index$X git-mktag$X git-mktree$X git-patch-id$X \ git-receive-pack$X \ git-send-pack$X git-shell$X \ @@ -264,7 +263,7 @@ PROGRAMS = \ git-update-server-info$X \ git-upload-pack$X \ git-pack-redundant$X git-var$X \ - git-merge-tree$X git-imap-send$X \ + git-merge-tree$X \ git-merge-recursive$X \ $(EXTRA_PROGRAMS) @@ -583,6 +582,11 @@ ifdef ZLIB_PATH endif EXTLIBS += -lz +ifndef NO_EXTRA_PROGRAMS + EXTRA_PROGRAMS += \ + git-daemon$X \ + git-imap-send$X +endif ifndef NO_OPENSSL OPENSSL_LIBSSL = -lssl ifdef OPENSSLDIR -- 1.5.4.1.126.ge5a7d -
Hi, This explanation is good (even if I recall that we compile git-daemon.exe We do not compile git-imap-send because MinGW lacks socketpair() and getpass(). Why not say it explicitely, and have a NO_SOCKETPAIR variable? The name NO_EXTRA_PROGRAMS is definitely not illustrating the reasons why we exclude those programs, so I'd rather not have this patch as-is. NOTE: I think that both programs are fixable, but only git-daemon is important enough that I would like to see patch of the "daemon" branch in 4msysgit.git cherry picked into this series (maybe we should implement a compat/syslog.c using the ReportEvent() function of the win32 API?). Ciao, Dscho -
Where are NO_WNOHANG, NO_GETPASS, NO_SELECT, NO_SIGCHLD, NO_EXECVE etc in your openlog() etc. is only one problem in git-daemon. It also depends on SIGCHLD, a non-blocking waitpid, and a lot more. The patch that is in 4msysgit.git allows only a single connection, IIRC, after which it terminates. But I also think that git-daemon can be made more complete on Windows, but it certainly requires a major surgery. -- Hannes --
Hi, Just pick the most difficult one. But just saying "this does not work on Okay, I did not look closely enough. But even a single-connection daemon is better than no daemon, no? Ciao, Dscho --
In principle, yes. But: - git-fetch needs two connections if there are new tags in the source repo. - The daemon patches in 4msysgit.git could be prettier, they certainly would not pass your scrutiny. git-daemon is not my itch at this time, so I won't scratch it. -- Hannes --
Hi, Right. I will mail the correct person. Ciao, Dscho --
Windows's struct stat does not have a st_blocks member. Since we already
have our own stat/lstat/fstat implementations, we can just as well use
a customized struct stat. This patch introduces just that, and also fills
in the st_blocks member. On the other hand, we don't provide members that
are never used.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
compat/mingw.c | 36 ++++++++++++++++++++++++++++--------
git-compat-util.h | 18 +++++++++++++-----
2 files changed, 41 insertions(+), 13 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index d44fbb3..0888288 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -31,6 +31,11 @@ static inline time_t filetime_to_time_t(const FILETIME *ft)
return (time_t)winTime;
}
+static inline size_t size_to_blocks(size_t s)
+{
+ return (s+511)/512;
+}
+
extern int _getdrive( void );
/* We keep the do_lstat code in a separate function to avoid recursion.
* When a path ends with a slash, the stat will fail with ENOENT. In
@@ -52,10 +57,10 @@ static int do_lstat(const char *file_name, struct stat *buf)
buf->st_ino = 0;
buf->st_gid = 0;
buf->st_uid = 0;
- buf->st_nlink = 1;
buf->st_mode = fMode;
buf->st_size = fdata.nFileSizeLow; /* Can't use nFileSizeHigh, since it's not a stat64 */
- buf->st_dev = buf->st_rdev = (_getdrive() - 1);
+ buf->st_blocks = size_to_blocks(buf->st_size);
+ buf->st_dev = _getdrive() - 1;
buf->st_atime = filetime_to_time_t(&(fdata.ftLastAccessTime));
buf->st_mtime = filetime_to_time_t(&(fdata.ftLastWriteTime));
buf->st_ctime = filetime_to_time_t(&(fdata.ftCreationTime));
@@ -89,7 +94,7 @@ static int do_lstat(const char *file_name, struct stat *buf)
* complete. Note that Git stat()s are redirected to mingw_lstat()
* too, since Windows doesn't really handle symlinks that well.
*/
-int mingw_lstat(const char *file_name, struct stat *buf)
+int mingw_lstat(const char *file_name, struct mingw_stat *buf)
{
int namelen;
static char ...builtin_exec_path returns the hard-coded installation path, which is used
as the ultimate fallback to look for git commands. Making it into a function
enables us in a follow-up patch to return a computed value instead of just
a constant string.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
exec_cmd.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/exec_cmd.c b/exec_cmd.c
index 343545d..3f3fa79 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -4,9 +4,13 @@
#define MAX_ARGS 32
extern char **environ;
-static const char *builtin_exec_path = GIT_EXEC_PATH;
static const char *argv_exec_path;
+static const char *builtin_exec_path(void)
+{
+ return GIT_EXEC_PATH;
+}
+
void git_set_argv_exec_path(const char *exec_path)
{
argv_exec_path = exec_path;
@@ -26,7 +30,7 @@ const char *git_exec_path(void)
return env;
}
- return builtin_exec_path;
+ return builtin_exec_path();
}
static void add_path(struct strbuf *out, const char *path)
@@ -54,7 +58,7 @@ void setup_path(const char *cmd_path)
add_path(&new_path, argv_exec_path);
add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));
- add_path(&new_path, builtin_exec_path);
+ add_path(&new_path, builtin_exec_path());
add_path(&new_path, cmd_path);
if (old_path)
--
1.5.4.1.126.ge5a7d
-
Since on Windows the user is fairly free where to install programs, we
cannot rely on a hard-coded path. We use the program name to derive the
installation directory and use that as exec_path.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
exec_cmd.c | 29 +++++++++++++++++++++++++++++
1 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/exec_cmd.c b/exec_cmd.c
index 3f3fa79..6d2f740 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -8,7 +8,36 @@ static const char *argv_exec_path;
static const char *builtin_exec_path(void)
{
+#ifndef __MINGW32__
return GIT_EXEC_PATH;
+#else
+ int len;
+ char *p, *q, *sl;
+ static char *ep;
+ if (ep)
+ return ep;
+
+ len = strlen(_pgmptr);
+ if (len < 2)
+ return ep = ".";
+
+ p = ep = xmalloc(len+1);
+ q = _pgmptr;
+ sl = NULL;
+ /* copy program name, turn '\\' into '/', skip last part */
+ while ((*p = *q)) {
+ if (*q == '\\' || *q == '/') {
+ *p = '/';
+ sl = p;
+ }
+ p++, q++;
+ }
+ if (sl)
+ *sl = '\0';
+ else
+ ep[0] = '.', ep[1] = '\0';
+ return ep;
+#endif
}
void git_set_argv_exec_path(const char *exec_path)
--
1.5.4.1.126.ge5a7d
-
With this definition the templates and system config file will be found irrespective of the installation location. Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at> --- Makefile | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/Makefile b/Makefile index 7d4a591..15b13c0 100644 --- a/Makefile +++ b/Makefile @@ -548,6 +548,8 @@ ifneq (,$(findstring MINGW,$(uname_S))) EXTLIBS += -lws2_32 X = .exe NOEXECTEMPL = .noexec + template_dir = ../share/git-core/templates/ + ETC_GITCONFIG = ../etc/gitconfig endif ifneq (,$(findstring arm,$(uname_M))) ARM_SHA1 = YesPlease -- 1.5.4.1.126.ge5a7d -
Since the Makefile in the template/ subdirectory is only used to install the templates, we do not simply pass down the setting of template_dir when it is relative, but construct the intended destination in a new variable: A relative template_dir is relative to gitexecdir. Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at> --- Makefile | 9 ++++++++- templates/Makefile | 8 ++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 15b13c0..53a4e2a 100644 --- a/Makefile +++ b/Makefile @@ -197,7 +197,7 @@ GITWEB_FAVICON = git-favicon.png GITWEB_SITE_HEADER = GITWEB_SITE_FOOTER = -export prefix bindir gitexecdir sharedir template_dir htmldir sysconfdir +export prefix bindir gitexecdir sharedir htmldir sysconfdir CC = gcc AR = ar @@ -1094,6 +1094,13 @@ remove-dashes: ### Installation rules +ifeq ($(firstword $(subst /, ,$(template_dir))),..) +template_instdir = $(gitexecdir)/$(template_dir) +else +template_instdir = $template_dir +endif +export template_instdir + install: all $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)' $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexecdir_SQ)' diff --git a/templates/Makefile b/templates/Makefile index b341105..eb08702 100644 --- a/templates/Makefile +++ b/templates/Makefile @@ -8,14 +8,14 @@ INSTALL ?= install TAR ?= tar RM ?= rm -f prefix ?= $(HOME) -template_dir ?= $(prefix)/share/git-core/templates +template_instdir ?= $(prefix)/share/git-core/templates # DESTDIR= # set NOEXECTEMPL to non-empty to change the names of hook scripts # so that the tools will not find them # Shell quote (do not use $(call) to accommodate ancient setups); DESTDIR_SQ = $(subst ','\'',$(DESTDIR)) -template_dir_SQ = $(subst ','\'',$(template_dir)) +template_instdir_SQ = $(subst ','\'',$(template_instdir)) all: boilerplates.made custom @@ -52,6 +52,6 @@ clean: $(RM) -r blt boilerplates.made install: all - $(INSTALL) -d -m 755 ...
This is just a little more versatile: ifeq ($(firstword $(subst /, ,ALONE-IF-ABSOLUTE$(template_dir))),ALONE-IF-ABSOLUTE) template_instdir = $template_dir else template_instdir = $(gitexecdir)/$(template_dir) endif export template_instdir Paolo -
Hi, How portable is "firstword"? Would it not be better to use ifneq ($(patsubst ../%,,$(template_dir)),) Hmm? Ciao, Dscho --
This programming language is sort of write-only, so I don't know because I can't read it ;) If you have tested your version and it works and you have thought through all consequences (like how does it work if template_dir=/a/../b), why not? -- Hannes --
Hi, I did not think through all consequences, but hoped that some knowledgable person could tell me that/if I am wrong in my assumption that patsubst is older than firstword. Since I encountered "firstword" for the very first time today, I thought it might be younger than "patsubst", which I met plenty a times. Ciao, Dscho --
At least make 3.78.1 (1999), search func_firstword in http://snipurl.com/20lle (Google Code Search). Paolo --
Since we have neither fork() nor exec(), we have to spawn the pager and
feed it with the program's output.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
pager.c | 40 ++++++++++++++++++++++++++++++++++++++--
1 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/pager.c b/pager.c
index ca002f9..3f7f8c1 100644
--- a/pager.c
+++ b/pager.c
@@ -1,12 +1,13 @@
#include "cache.h"
/*
- * This is split up from the rest of git so that we might do
- * something different on Windows, for example.
+ * This is split up from the rest of git so that we can do
+ * something different on Windows.
*/
static int spawned_pager;
+#ifndef __MINGW32__
static void run_pager(const char *pager)
{
/*
@@ -22,11 +23,31 @@ static void run_pager(const char *pager)
execlp(pager, pager, NULL);
execl("/bin/sh", "sh", "-c", pager, NULL);
}
+#else
+#include "run-command.h"
+
+static const char *pager_argv[] = { "sh", "-c", NULL, NULL };
+static struct child_process pager_process = {
+ .argv = pager_argv,
+ .in = -1
+};
+static void wait_for_pager(void)
+{
+ fflush(stdout);
+ fflush(stderr);
+ /* signal EOF to pager */
+ close(1);
+ close(2);
+ finish_command(&pager_process);
+}
+#endif
void setup_pager(void)
{
+#ifndef __MINGW32__
pid_t pid;
int fd[2];
+#endif
const char *pager = getenv("GIT_PAGER");
if (!isatty(1))
@@ -45,6 +66,7 @@ void setup_pager(void)
spawned_pager = 1; /* means we are emitting to terminal */
+#ifndef __MINGW32__
if (pipe(fd) < 0)
return;
pid = fork();
@@ -72,6 +94,20 @@ void setup_pager(void)
run_pager(pager);
die("unable to execute pager '%s'", pager);
exit(255);
+#else
+ /* spawn the pager */
+ pager_argv[2] = pager;
+ if (start_command(&pager_process))
+ return;
+
+ /* original process continues, but writes to the pipe */
+ dup2(pager_process.in, 1);
+ dup2(pager_process.in, 2);
+ close(pager_process.in);
+
+ /* this makes sure that the parent terminates ...On Windows, write() is implemented using WriteFile(). After the reader
closed its end of the pipe, the first WriteFile() returns
ERROR_BROKEN_PIPE (which translates to EPIPE), subsequent WriteFile()s
return ERROR_NO_DATA, which is translated to EINVAL.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
write_or_die.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/write_or_die.c b/write_or_die.c
index e125e11..f3b8566 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -34,7 +34,12 @@ void maybe_flush_or_die(FILE *f, const char *desc)
return;
}
if (fflush(f)) {
- if (errno == EPIPE)
+ /*
+ * On Windows, EPIPE is returned only by the first write()
+ * after the reading end has closed its handle; subsequent
+ * write()s return EINVAL.
+ */
+ if (errno == EPIPE || errno == EINVAL)
exit(0);
die("write failure on %s: %s", desc, strerror(errno));
}
--
1.5.4.1.126.ge5a7d
-
If the status output already goes to stdout, then it is not necessary to
redirect stdout for the diff machinery. (This case hangs on Windows for
unknown reasons.)
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
wt-status.c | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 32d780a..e4999ea 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -312,15 +312,17 @@ static void wt_status_print_untracked(struct wt_status *s)
static void wt_status_print_verbose(struct wt_status *s)
{
struct rev_info rev;
- int saved_stdout;
+ int saved_stdout = -1;
fflush(s->fp);
/* Sigh, the entire diff machinery is hardcoded to output to
* stdout. Do the dup-dance...*/
- saved_stdout = dup(STDOUT_FILENO);
- if (saved_stdout < 0 ||dup2(fileno(s->fp), STDOUT_FILENO) < 0)
- die("couldn't redirect stdout\n");
+ if (fileno(s->fp) != STDOUT_FILENO) {
+ saved_stdout = dup(STDOUT_FILENO);
+ if (saved_stdout < 0 ||dup2(fileno(s->fp), STDOUT_FILENO) < 0)
+ die("couldn't redirect stdout\n");
+ }
init_revisions(&rev, NULL);
setup_revisions(0, NULL, &rev, s->reference);
@@ -330,7 +332,7 @@ static void wt_status_print_verbose(struct wt_status *s)
fflush(stdout);
- if (dup2(saved_stdout, STDOUT_FILENO) < 0)
+ if (saved_stdout >= 0 && dup2(saved_stdout, STDOUT_FILENO) < 0)
die("couldn't restore stdout\n");
close(saved_stdout);
}
--
1.5.4.1.126.ge5a7d
-
Hi, Just in case people like a diff machinery that can output somewhere else than stdout, I have a patch to do that, and also a corresponding patch to avoid the dup dance. Ciao, Dscho --
git help -a scans the PATH for git commands. On Windows it failed for two
reasons:
- The PATH separator is ';', not ':' on Windows.
- stat() does not set the executabe bit.
We now open the file and guess whether it is executable.
The result of the guess is good enough for the list of git commands, but
it is of no use for a general stat() implementation because (1) it is a
guess, (2) the user has no way to influence the outcome (via chmod or
similar), and (3) it would reduce stat() performance by an unacceptable
amount. Therefore, this strategy is a special-case local to help.c.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
help.c | 38 +++++++++++++++++++++++++++++++++-----
1 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/help.c b/help.c
index 8143dcd..0248c76 100644
--- a/help.c
+++ b/help.c
@@ -163,6 +163,32 @@ static void pretty_print_string_list(struct cmdnames *cmds, int longest)
}
}
+static int is_executable(const char *name)
+{
+ struct stat st;
+
+ if (stat(name, &st) || /* stat, not lstat */
+ !S_ISREG(st.st_mode))
+ return 0;
+
+#ifdef __MINGW32__
+ /* cannot trust the executable bit, 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)
+ /* DOS executables start with "MZ" */
+ if (!strcmp(buf, "#!") || !strcmp(buf, "MZ"))
+ st.st_mode |= S_IXUSR;
+ close(fd);
+ }
+#endif
+ return st.st_mode & S_IXUSR;
+}
+
static unsigned int list_commands_in_dir(struct cmdnames *cmds,
const char *path)
{
@@ -176,15 +202,12 @@ static unsigned int list_commands_in_dir(struct cmdnames *cmds,
return 0;
while ((de = readdir(dir)) != NULL) {
- struct stat st;
int entlen;
if (prefixcmp(de->d_name, prefix))
continue;
- if (stat(de->d_name, &st) || /* stat, not lstat */
- !S_ISREG(st.st_mode) ||
- !(st.st_mode & S_IXUSR))
+ if ...Aha! When I replied to patch 4/40 I knew there would be one more. :-) Paolo -
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at> --- path.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/path.c b/path.c index 4260952..7f18539 100644 --- a/path.c +++ b/path.c @@ -75,6 +75,13 @@ int git_mkstemp(char *path, size_t len, const char *template) size_t n; tmp = getenv("TMPDIR"); +#ifdef __MINGW32__ + /* on Windows it is TMP and TEMP */ + if (!tmp) + tmp = getenv("TMP"); + if (!tmp) + tmp = getenv("TEMP"); +#endif if (!tmp) tmp = "/tmp"; n = snprintf(path, len, "%s/%s", tmp, template); -- 1.5.4.1.126.ge5a7d -
With this change GIT can be compiled and linked using MinGW. Builtins that only read the repository such as the log family and grep already work. Simple stubs are provided for a number of functions that the Windows C runtime does not offer. They will be completed in later patches. Dmitry Kakurin pointed out that access(..., X_OK) would always fails on Vista and suggested the -D__USE_MINGW_ACCESS workaround. Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at> --- Makefile | 25 +++++++++ compat/mingw.c | 57 ++++++++++++++++++++ git-compat-util.h | 148 +++++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 227 insertions(+), 3 deletions(-) create mode 100644 compat/mingw.c diff --git a/Makefile b/Makefile index 870a862..bc8a487 100644 --- a/Makefile +++ b/Makefile @@ -522,6 +522,31 @@ ifeq ($(uname_S),HP-UX) NO_HSTRERROR = YesPlease NO_SYS_SELECT_H = YesPlease endif +ifneq (,$(findstring MINGW,$(uname_S))) + NO_MMAP = YesPlease + NO_PREAD = YesPlease + NO_OPENSSL = YesPlease + NO_CURL = YesPlease + NO_SYMLINK_HEAD = YesPlease + NO_IPV6 = YesPlease + NO_SETENV = YesPlease + NO_UNSETENV = YesPlease + NO_STRCASESTR = YesPlease + NO_STRLCPY = YesPlease + NO_MEMMEM = YesPlease + NEEDS_LIBICONV = YesPlease + OLD_ICONV = YesPlease + NO_C99_FORMAT = YesPlease + NO_STRTOUMAX = YesPlease + NO_MKDTEMP = YesPlease + NO_SVN_TESTS = YesPlease + NO_PERL_MAKEMAKER = YesPlease + NO_EXTRA_PROGRAMS = YesPlease + COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat + COMPAT_OBJS += compat/mingw.o compat/fnmatch.o compat/regex.o + EXTLIBS += -lws2_32 + X = .exe +endif ifneq (,$(findstring arm,$(uname_M))) ARM_SHA1 = YesPlease endif diff --git a/compat/mingw.c b/compat/mingw.c new file mode 100644 index 0000000..0c87e43 --- /dev/null +++ b/compat/mingw.c @@ -0,0 +1,57 @@ +#include "../git-compat-util.h" + +unsigned int _CRT_fmode = _O_BINARY; + +unsigned int sleep (unsigned int ...
Hi, Should it not return -1, for failure? (I know, I know, probably a few programs do not work, then, but it is not correct that it succeeded.) ... and this in compat/mingw.h? And then, we'd only have #ifdef __MINGW32__ #include "mingw.h" #endif in git-compat-util.h? Ciao, Dscho -
Agreed. The return value of these functions should not make a difference at No, becauser xmkstemp needs the forward declaration of mkstemp(). But we could I thought about this, but I decided against it: git-compat-util.h is the place to look for compatibility functions. A file compat/mingw.h only introduces an extra indirection and only *hides* stuff instead of making it obvious. -- Hannes --
Hi, Actually, I was thinking of including it at the same spot where you declare mkstemp conditionally. (Of course, since I spotted very well that Well, the thing is, there are quite a few definitions and declarations that are _only_ _ever_ interesting if you are on MinGW32. So maybe it is a good idea to hide it to all the other users. Ciao, Dscho --
You get compat/mingw.h, but you don't get a zillion NO_<insert feature here>. The two are pretty much exclusive anyway. Deal? -- Hannes --
Hi, Thanks! With this, I think I have nothing to add to your series (except maybe an Acked-by: where it applies, or a Reviewed-by:, but I think that this would only be a burden on our maintainer). Ciao, Dscho --
I think Reviewed-by: would indeed be a very good addition to our patch flow convention, borrowing from the kernel folks. --
Hi, You mean you have more people to blame, then? ;-) Ciao, Dscho --
No. The procedure would help me keep my impatience from making me merge patches that have not been adequately reviewed on the list. Recently, I ended up wasting two nights because I was not careful enough earlier, when I was short of time and apparent backlog was beginning to get larger and larger. I queued some patches from the backlog to 'next' saying "ah, they look good enough, people will notice breakages anyway," but the breakage was not caught until 'master' got broken. Not good. And the list is not to blame. By merging to 'next' I am sending a message that I think they have been adequately reviewed (either by me or by people whose judgement I trust), so I shouldn't have applied them to 'next' in the first place. I instead should have ignored them, and waited until I had enough time and concentration to properly review them. Or until somebody else did --- by that time, hopefully other people might have commented on them, saying "these look all ok to me", or "ah that's crap". These wasted two nights was all my fault, and as a result, there are more patches on the list archive that I have seen (notice I did not say "have read") that are unapplied. As to those "more patches on the list that are unapplied", I'll keep them unapplied for now, until there are positive feedbacks on them. The positive feedback may come from myself. I am not saying I will stop reviewing and/or applying patches nobody else commented on. --
Hi, Ah, but I think that you are too harsh onto yourself. Recently, there was a surge of patches, mainly because 1.5.4 was held of -- but for a good reason: 1.5.4 was not ready before the point in time where you decided to release it. If at all, the list is to blame, for just sending patches, but not reviewing them. Now, personally I know that I am not half as good a reviewer as you are, since you catch way more bugs than me, just by looking at the patch. But still, "many eyes make bugs shallow" is a principle to be heeded _everywhere_. So I'd say: if you think that you are short of time, and patches have not been reviewed properly, do not assume it _your sole_ responsibility to review the patches. Make it known that other people have to step in (even if it is a mediocre reviewer like me). Do not overload yourself. So my comment about the "blame" was really tongue-in-cheek. Please do not take it for anything but a joke. And when I say that I think you are a kick-ass maintainer, I _mean_ it. 'nuff said, Dscho --
The most common use of addf() was to init a strbuf and addf() right away.
Since it is so common, it makes sense to have a function strbuf_initf()
to wrap both calls into one.
Unfortunately, C (and cpp) has no way to make this easy without code
duplication, as we need to va_init() in strbuf_addf() possibly a few
times. So the code for addf() is copied. Fortunately, the code is
pretty short, so not too much had to be copied as-is.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
On Wed, 5 Mar 2008, Johannes Schindelin wrote:
> On Wed, 5 Mar 2008, Junio C Hamano wrote:
>
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >
> > > [... I'd submit my ] a Reviewed-by:, but I think that this
> > > would only be a burden on our maintainer.
> >
> > I think Reviewed-by: would indeed be a very good addition to
> > our patch flow convention, borrowing from the kernel folks.
>
> You mean you have more people to blame, then? ;-)
Well, it was meant as a joke...
Anyway, here is a start of a patch series that should help the
King Penguin...
strbuf_initf() is something I long planned to do; Kristian just
pushed me over the edge.
strbuf.c | 23 +++++++++++++++++++++++
strbuf.h | 2 ++
2 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index 5afa8f3..067d55a 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -147,6 +147,29 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)
strbuf_setlen(sb, sb->len + len);
}
+void strbuf_initf(struct strbuf *sb, const char *fmt, ...)
+{
+ int len;
+ va_list ap;
+
+ strbuf_init(sb, strlen(fmt) + 64);
+ va_start(ap, fmt);
+ len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
+ va_end(ap);
+ if (len < 0)
+ die("your vsnprintf is broken");
+ if (len > strbuf_avail(sb)) {
+ strbuf_grow(sb, len);
+ va_start(ap, fmt);
+ len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
+ va_end(ap);
+ if (len > ...The problem with code duplication is not about code size, but more about not forgetting to fix bugs in all incarnations of the duplicated code. Is it so ugly to use a macro ? Mike --
Why not have a strbuf_vaddf and strbuf_vinitf that take a va_arg as a parameter. This would mean that you don't have code duplication, and it is flexible enough if you want to add more customisations in the future. No macro needed. This is what the printf/scanf family of functions do. - Reece --
Hi, The problem is that we have to restart va_list() if the buffer was too small. So your suggestion is out, unless you suggest to implement the whole printf mechanism... which I hope you're not. Ciao, Dscho --
I think we've spent more time debating va_copy than it would take for somebody to just lift the implementation and checks from something like glib. But the recent patch for vsnprintf[1] doesn't actually fix the problem of reusing va_args in the general case; the va_list argument, ap, is undefined after the vsnprintf() call, yet it calls it in a loop. Just bite the bullet and pull in va_copy. Of course, I'm just adding It's not a terrible idea, honestly. There are several mature vsnprintf implementations out there under friendly licenses. We could just stick it in compat/. It's the only way to do reliable, cross platform vsnprintf, in my experience. And the issue with %I64u vs %llu could be handle by implementing both, as Wayne Davison suggests. Kristian [1] Message-Id: 200803051646.13343.michal.rokos@nextsoft.cz from Michal Rokos --
Hi, On Thu, 6 Mar 2008, Kristian H
That's how it always starts... "We just need %d and %s! It'll take me half an hour!" I know because, I've it a number of times myself ;) Just pull in the rsync one, don't waste time on this. Kristian --
And vsnprintf is a mess accross systems, returning -1 when not expected. I even had the surprise to get an interesting bug in liferea/libxml2 due to glibc's vsnprintf returning -1 in some obscure cases. Mike --
No, that was not my intention. My intention was that they were simple forwarding functions that handled the va_start and va_end calls. Is it possible to pass a void * to a strbuf_vaddf function that you can pass to va_start, so you can then restart the va_list? - Reece --
Hi, AFAIU va_list() works on the stack (on less register-challenged systems than x86, the first parameters are possibly passed as registers, too, but it has to resort to the stack at a certain number of parameters). Since the stack is also used (at least on register-challenged yadda yadda) to store the return address, va_list() would most likely pick that up, too. Now, that just _might_ still work, since the printf() family determines the number of arguments from the format string. But then there are machines which are _not_ completely stack-based, for example SPARC. IIRC the first 8 parameters are passed by a so-called register window, which changes with each function call. So no, I think there is no portable way to pass them around. Of course, having a simple implementation for addf() _not_ using vsnprintf() could help, too (and make the process more efficient, probably). I thought. The formats we'd have to support are: $ git grep strbuf_addf | sed -e 's/^[^"]*"\([^"]*\)".*$/\1/' -e 's/%%//g' -e 's/%/\ &/g' | sed -n -e 's/\(%[^a-zA-Z]*[a-zA-Z]\).*/\1/p' | sort | uniq %02X %06o %c %d %l %o %s %.*s %u So it does not look too bad. 'X', 'o', 'd', 'l' and 'u', possibly with a size specifier (left 0-padded), 'c' and 's' (possibly with a length parameter). The implementation wouldn't be _too_ short, I'd say, but doable. Ciao, Dscho --
The most common use of addf() was to init a strbuf and addf() right away. Since it is so common, it makes sense to have a function strbuf_initf() to wrap both calls into one. To do that, we implement a (really minimal) vaddf() lookalike to vsprintf(). At the moment, it only handles %u, %i, %d, %l, %o, %x and %X with size indicators '<number>', ' <number>' and '0<number>', as well as %c and %s, the latter with size indicators '.*' in addition to the same size indicators as for numbers. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- On Thu, 6 Mar 2008, Johannes Schindelin wrote: > Of course, having a simple implementation for addf() _not_ using > vsnprintf() could help, too (and make the process more efficient, > probably). Having thought about this a bit, I came up with this implementation (replacing my earlier PATCH 1/2). It is by far not complete, but that is exactly the idea: we do not _need_ a fully-fledged printf() implementation. Besides, it was fun. .gitignore | 1 + Makefile | 4 +- strbuf.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++------ strbuf.h | 3 + t/t0000-basic.sh | 8 +++ test-strbuf.c | 17 +++++++ 6 files changed, 150 insertions(+), 17 deletions(-) create mode 100644 test-strbuf.c diff --git a/.gitignore b/.gitignore index 219759f..c0ecd41 100644 --- a/.gitignore +++ b/.gitignore @@ -152,6 +152,7 @@ test-genrandom test-match-trees test-parse-options test-sha1 +test-strbuf common-cmds.h *.tar.gz *.dsc diff --git a/Makefile b/Makefile index cd0b294..58f6238 100644 --- a/Makefile +++ b/Makefile @@ -1052,7 +1052,7 @@ endif ### Testing rules -TEST_PROGRAMS = test-chmtime$X test-genrandom$X test-date$X test-delta$X test-sha1$X test-match-trees$X test-absolute-path$X test-parse-options$X +TEST_PROGRAMS = test-chmtime$X test-genrandom$X test-date$X test-delta$X test-sha1$X test-match-trees$X test-absolute-path$X ...
WTF? You did not cater for PRIuMAX, which is %llu except on Windows, where it is %I64u. We can make it %llu on Windows only if we can ensure that none of its uses ends up in a regular *printf function! -- Hannes --
Oh, well, PRIuMAX is *only* used in regular *printf and in die() calls. That's fine, too. -- Hannes --
Hi, Hey, no problem. I used my earlier analysis to find the _minimal_ set of formats needed to get Git running without that vsnprintf(). The idea is, of course, to enhance vaddf() as needed (if at all). Note: I did not even bother trying to convert any vsnprintf() users except for strbuf_addf() to that interface: it should be relatively easy, but there are quite a few sites (even using strbuf, but not addf() because only a va_list is available), so it is left as an exercise to the reader ;-) Ciao, Dscho --
Hi, AFAIR it is unportable to have a macro #define strbuf_initf(buf, fmt, ...) \ (strbuf_init(buf, strlen(fmt) + 64), strbuf_addf(buf, fmt, args);) (GNU cpp groks it, but what about the others?) Or you mean the whole body of the implementation of strbuf_addf()? Now, _that_ would be ugly. Ciao, Dscho --
C99 has variable-argument macros, but C89 does not. I have no idea in practice how many of the compilers git targets support them (in one form or another). -Peff --
Now you can conveniently add lines with format-patch, adding missing
Reviewed-by: lines (if it is already present at the end of the commit
message, it will not be repeated).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
And this is the actual patch...
It is just a patch for format-patch... the patch for git-am should
be trivial.
Documentation/git-format-patch.txt | 5 ++++
builtin-log.c | 9 +++++++
log-tree.c | 44 +++++++++++++++++++++++++++++++++++-
revision.h | 1 +
t/t4014-format-patch.sh | 26 +++++++++++++++++++++
5 files changed, 84 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index b5207b7..da52720 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -11,6 +11,7 @@ SYNOPSIS
[verse]
'git-format-patch' [-k] [-o <dir> | --stdout] [--thread]
[--attach[=<boundary>] | --inline[=<boundary>]]
+ [--reviewed-by=<ident>]
[-s | --signoff] [<common diff options>]
[-n | --numbered | -N | --no-numbered]
[--start-number <n>] [--numbered-files]
@@ -96,6 +97,10 @@ include::diff-options.txt[]
Do not strip/add '[PATCH]' from the first line of the
commit log message.
+--reviewed-by=<ident>::
+ Add `Reviewed-by:` line to the commit message, using
+ the given identity.
+
-s|--signoff::
Add `Signed-off-by:` line to the commit message, using
the committer identity of yourself.
diff --git a/builtin-log.c b/builtin-log.c
index fe8fc6f..90573b3 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -753,6 +753,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
const char *in_reply_to = NULL;
struct patch_ids ids;
char *add_signoff = NULL;
+ struct path_list add_reviewed_by = { NULL, 0, 0, 0 };
struct strbuf buf;
git_config(git_format_config);
@@ -821,6 +822,13 @@ int ...What's the expected workflow this patch intends to help? - You see a patch by somebody, you look at it deeply, you apply to your tree (presumably with your own Signed-off-by). - You inspect the result further, and decide it is good. - You format-patch with the option, which would now have a Reviewed-by: too. - You send it out. If so, it might make sense to simply always use the committer ident. If the person who adds the reviewed-by is trusted so much that her reviewed-by counts, the commits might even be transfered with "Please pull". In such a case, the workflow might become: - You see a patch by somebody, you look at it deeply, you apply to your tree (presumably with your own Signed-off-by). - You inspect the result further, and decide it is good. - You run "rebase --add-reviewed-by" to prepare a series on a branch to be pulled from. - You send a request-pull. In that workflow, it would also make sense to use the committer ident. I am trying to come up with a plausible workflow that wants to add somebody else's reviewed-by. - You send out your patch to the list. People give comments, you reroll, you get more comments, eventually people say "Ah, that's good, Ack." and/or "I am not the primary person who knows this area, but I reviewed it and I know my reviewed-by would count, so here is my Ok". - You format-patch the final version, with Acked-by and Reviewed-by adding other people's names. Then I think it makes sense to take names of other people if that is the case. You probably meant that that is the expected workflow, as you can give more than one of these options. Why not use a single -m for the first three reviewed-bys, instead of making them into separate paragraphs using multiple -m? --
Hi, Yes, exactly. That was the usage I had in mind. The problem with the two others you suggested (basically that the reviewer herself sends out another patch series) is a little fragile, since a malicous reviewer could add/modify code, but for nice reviewers, it is better in that you know which code was reviewed. So I'd rather have it not resent by the reviewer as a patch series, but I think that pulling it is fine, since it is much easier to verify (with log Frankly, I did not make up my mind on the other workflows, I only wanted to make the maintainer's life (yours) easier. I have no problem posting several revisions of this patch, or scrap it altogether... as somebody recently said, we even joke around here via I am always unsure how to embed newlines in strings in a shell script (except with single-ticks), and besides, I do not like breaking the indentation. But I could make it a temporary file... Hmm? Ciao, Dscho --
Thank you! I now wait for vsnprintf patches and the recent start_command patch to hit master, then I'll do a final rebase and fix-up before I consider the series ready for inclusion into git.git. -- Hannes --
I must admit I was very sloppy with the previous round. I had to make has_dos_drive_prefix a macro; otherwise we would get numerous warnings about "undeclared function isalpha", because the declaration appears later in git-compat-util.h. On the positive side, we can now reuse Michal's vsnprintf wrapper, which fixes snprintf, too, (which was not the case previously). Note that on Windows we have to adjust the size parameter. There's also a change in the setup of stderr in start_command() that corresponds to ce2cf27adc. And I made is_dir_sep into a conditional macro similar to has_dos_drive_prefix to get rid of another #ifdef/#endif. -- Hannes Here's the interdiff: diff --git a/Makefile b/Makefile index 68d60e7..6619523 100644 --- a/Makefile +++ b/Makefile @@ -309,7 +309,7 @@ LIB_H = \ tree-walk.h log-tree.h dir.h path-list.h unpack-trees.h builtin.h \ utf8.h reflog-walk.h patch-ids.h attr.h decorate.h progress.h \ mailmap.h remote.h parse-options.h transport.h diffcore.h hash.h fsck.h \ - pack-revindex.h + pack-revindex.h compat/mingw.h DIFF_OBJS = \ diff.o diff-lib.o diffcore-break.o diffcore-order.o \ @@ -549,10 +549,12 @@ ifneq (,$(findstring MINGW,$(uname_S))) NO_C99_FORMAT = YesPlease NO_STRTOUMAX = YesPlease NO_MKDTEMP = YesPlease + SNPRINTF_RETURNS_BOGUS = YesPlease NO_SVN_TESTS = YesPlease NO_PERL_MAKEMAKER = YesPlease NO_POSIX_ONLY_PROGRAMS = YesPlease COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat + COMPAT_CFLAGS += -DSNPRINTF_SIZE_CORR=1 COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\" COMPAT_OBJS += compat/mingw.o compat/fnmatch.o compat/regex.o EXTLIBS += -lws2_32 diff --git a/compat/mingw.c b/compat/mingw.c index 6733727..7c8fd0e 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -847,40 +847,6 @@ int mingw_rename(const char *pold, const char *pnew) return -1; } -#undef vsnprintf -/* Note that the size parameter specifies the available space, i.e. - * includes the trailing NUL byte; but ...
Hi, Actually, I have to admit that I like strbuf_vaddf() more and more. Anything you would wish me to implement there? Ciao, Dscho --
Only that you keep an eye on PRIuMAX; if it is ever used in a format string of strbuf_vaddf() then you ought to implement %I64u as well as %llu. ;) -- Hannes --
FWIW, this is overwritten by later patches in the series anyway. All the other suggestions that Dscho wrote about, make a lot of sense to me too. Paolo -
Hi, Yes, I know that. But the whole point of having a nice patch series is to have proper revisions, and IMO returning 0 here is not correct. Ciao, Dscho --
From: Steffen Prohaska <prohaska@zib.de>
From: Steffen Prohaska <prohaska@zib.de>
On Windows, ntohl() returns unsigned long. On Unix it returns
uint32_t. This makes choosing a suitable printf format string
hard.
This commit introduces a mingw specific helper function
git_ntohl() that casts to unsigned int before returning. This
makes gcc's printf format check happy. It should be safe because
we expect ntohl to use 32-bit numbers.
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
git-compat-util.h | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index 4a53b3b..5ecdd40 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -631,6 +631,10 @@ pid_t mingw_spawnvpe(const char *cmd, const char **argv, char **env);
void mingw_execvp(const char *cmd, char *const *argv);
#define execvp mingw_execvp
+static inline unsigned int git_ntohl(unsigned int x)
+{ return (unsigned int)ntohl(x); }
+#define ntohl git_ntohl
+
sig_handler_t mingw_signal(int sig, sig_handler_t handler);
#define signal mingw_signal
--
1.5.4.1.126.ge5a7d
-
From: Steffen Prohaska <prohaska@zib.de> From: Steffen Prohaska <prohaska@zib.de> read_in_full() is used in compat/pread.c. read_in_full() is declared in cache.h. But we can't include cache.h because too many macros are defined there. Using read_in_full() without including cache.h is dangerous because we wouldn't recognize if its prototyp changed. gcc issues a warning about that. This commit adds a forward decl to git-compat-util.h. git-compat-util.h is included by compat/pread.c _and_ cache.h. Hence, changes in cache.h would be detected. Signed-off-by: Steffen Prohaska <prohaska@zib.de> Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at> --- git-compat-util.h | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index 5ecdd40..4a8df8e 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -168,6 +168,10 @@ extern int git_munmap(void *start, size_t length); #define pread git_pread extern ssize_t git_pread(int fd, void *buf, size_t count, off_t offset); #endif +/* Forward decl that will remind us if its twin in cache.h changes. + This function in used in compat/pread.c. But we can't include + cache.h there. */ +extern int read_in_full(int fd, void *buf, size_t count); #ifdef NO_SETENV #define setenv gitsetenv -- 1.5.4.1.126.ge5a7d -
Hi, s/ in / is / Thanks, Dscho --
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at> --- Documentation/git.txt | 6 +++--- exec_cmd.c | 4 ++++ sha1_file.c | 4 ++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Documentation/git.txt b/Documentation/git.txt index d57bed6..b2a5b60 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -382,9 +382,9 @@ git so take care if using Cogito etc. 'GIT_ALTERNATE_OBJECT_DIRECTORIES':: Due to the immutable nature of git objects, old objects can be archived into shared, read-only directories. This variable - specifies a ":" separated list of git object directories which - can be used to search for git objects. New objects will not be - written to these directories. + specifies a ":" separated (on Windows ";" separated) list + of git object directories which can be used to search for git + objects. New objects will not be written to these directories. 'GIT_DIR':: If the 'GIT_DIR' environment variable is set then it diff --git a/exec_cmd.c b/exec_cmd.c index e189cac..343545d 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -37,7 +37,11 @@ static void add_path(struct strbuf *out, const char *path) else strbuf_addstr(out, make_absolute_path(path)); +#ifdef __MINGW32__ + strbuf_addch(out, ';'); +#else strbuf_addch(out, ':'); +#endif } } diff --git a/sha1_file.c b/sha1_file.c index 1ddb96b..453bc43 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -389,7 +389,11 @@ void prepare_alt_odb(void) if (!alt) alt = ""; alt_odb_tail = &alt_odb_list; +#ifdef __MINGW32__ + link_alt_odb_entries(alt, alt + strlen(alt), ';', NULL, 0); +#else link_alt_odb_entries(alt, alt + strlen(alt), ':', NULL, 0); +#endif read_info_alternates(get_object_directory(), 0); } -- 1.5.4.1.126.ge5a7d -
Why not adding a PATH_SEPARATOR #define? Paolo -
Because IMO it is obfuscating: 1. When you read through the code and see PATH_SEPARATOR, you still have to go look how it's defined. Why? Because you always will ask: Is this about ':' vs. ';' or '/' vs. '\\'? 2. When you look for where ":" or ";" are treated, you'll end up at the #define. Then you need an extra step to search for PATH_SEPARATOR. -- Hannes --
One is a DIR_SEPARATOR, the other is a PATH_SEPARATOR. It's a matter of conventions. Paolo --
Hi, I agree that it would be good, also for documentation purposes, to add the #defines. However, there is something ugly waiting for us: we often have case '/': and for Windows, this needs to add case '\\': Now, we could #define CASE_DIR_SEPARATOR, which expands to "case '/': case '\\':" on Windows, but I am prepared to let Windows uglify our source code only so far *does the famous imitation of Linus where he puts two finger so close that you cannot see between them*. Ciao, Dscho --
More often than in an "if" statement (so you can add IS_DIR_SEPARATOR which is much less ugly than CASE_DIR_SEPARATOR)? Paolo --
Hi, I have not grepped through the patches (as yourself, I am too lazy), but I remember seeing the "case" case in two instances, and I do not remember seeing the "if" case. In the end, I think suggesting something like PATH_SEPARATOR is good. But insisting on it without working on it is less good. So I think that it is up to Hannes now; he got enough input, and he has a fine sense of taste, so he will do the Right Thing. Ciao, Dscho --
Hi, IMO these are not good arguments. If they were, we could not typedef size_t and socklen_t, either. Ciao, Dscho --
Before we can successfully parse a builtin command from the program name
we must strip off unneeded parts, that is, the file extension.
Additionally, we must take Windows style path names into account.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
Makefile | 1 +
git.c | 14 ++++++++++++++
2 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index bc8a487..069939f 100644
--- a/Makefile
+++ b/Makefile
@@ -543,6 +543,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
NO_PERL_MAKEMAKER = YesPlease
NO_EXTRA_PROGRAMS = YesPlease
COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat
+ COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
COMPAT_OBJS += compat/mingw.o compat/fnmatch.o compat/regex.o
EXTLIBS += -lws2_32
X = .exe
diff --git a/git.c b/git.c
index 0cb8688..32166a4 100644
--- a/git.c
+++ b/git.c
@@ -372,6 +372,15 @@ static void handle_internal_command(int argc, const char **argv)
};
int i;
+#ifdef STRIP_EXTENSION
+ i = strlen(argv[0]) - strlen(STRIP_EXTENSION);
+ if (i > 0 && !strcmp(argv[0] + i, STRIP_EXTENSION)) {
+ char *argv0 = strdup(argv[0]);
+ argv[0] = cmd = argv0;
+ argv0[i] = '\0';
+ }
+#endif
+
/* Turn "git cmd --help" into "git help cmd" */
if (argc > 1 && !strcmp(argv[1], "--help")) {
argv[1] = argv[0];
@@ -398,6 +407,11 @@ int main(int argc, const char **argv)
* name, and the dirname as the default exec_path
* if we don't have anything better.
*/
+#ifdef __MINGW32__
+ char *bslash = strrchr(cmd, '\\');
+ if (!slash || (bslash && bslash > slash))
+ slash = bslash;
+#endif
if (slash) {
*slash++ = 0;
cmd_path = cmd;
--
1.5.4.1.126.ge5a7d
-
The wrapper does two things:
- Requests to open /dev/null are redirected to open the nul pseudo file.
- A request to open a file that currently exists as a directory on
Windows fails with EACCES; this is changed to EISDIR.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
compat/mingw.c | 20 ++++++++++++++++++++
git-compat-util.h | 7 +++++++
2 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 0c87e43..faa6df3 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -2,6 +2,26 @@
unsigned int _CRT_fmode = _O_BINARY;
+#undef open
+int mingw_open (const char *filename, int oflags, ...)
+{
+ va_list args;
+ unsigned mode;
+ va_start(args, oflags);
+ mode = va_arg(args, int);
+ va_end(args);
+
+ if (!strcmp(filename, "/dev/null"))
+ filename = "nul";
+ int fd = open(filename, oflags, mode);
+ if (fd < 0 && (oflags & O_CREAT) && errno == EACCES) {
+ DWORD attrs = GetFileAttributes(filename);
+ if (attrs != INVALID_FILE_ATTRIBUTES && (attrs & FILE_ATTRIBUTE_DIRECTORY))
+ errno = EISDIR;
+ }
+ return fd;
+}
+
unsigned int sleep (unsigned int seconds)
{
Sleep(seconds*1000);
diff --git a/git-compat-util.h b/git-compat-util.h
index b1f3f92..672c074 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -577,6 +577,13 @@ struct passwd *getpwuid(int uid);
int setitimer(int type, struct itimerval *in, struct itimerval *out);
int sigaction(int sig, struct sigaction *in, struct sigaction *out);
+/*
+ * replacements of existing functions
+ */
+
+int mingw_open (const char *filename, int oflags, ...);
+#define open mingw_open
+
#endif /* __MINGW32__ */
#endif
--
1.5.4.1.126.ge5a7d
-
getpwuid() is implemented just enough that GIT does not issue errors.
Since the information that it returns is not very useful, users are
required to set up user.name and user.email configuration.
All uses of getpwuid() are like getpwuid(getuid()), hence, the return value
of getuid() is irrelevant and the uid parameter is not even looked at.
Side note: getpwnam() is only used to resolve '~' and '~username' paths,
which is an idiom not known on Windows, hence, we don't implement it.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
compat/mingw.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index faa6df3..0c1d0e4 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -62,7 +62,15 @@ struct tm *localtime_r(const time_t *timep, struct tm *result)
struct passwd *getpwuid(int uid)
{
+ static char user_name[100];
static struct passwd p;
+
+ DWORD len = sizeof(user_name);
+ if (!GetUserName(user_name, &len))
+ return NULL;
+ p.pw_name = user_name;
+ p.pw_gecos = "unknown";
+ p.pw_dir = NULL;
return &p;
}
--
1.5.4.1.126.ge5a7d
-
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
On Windows, a read-only files cannot be deleted. To make sure that
deletion does not fail because of this, always call chmod() before
unlink().
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
git-compat-util.h | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index 672c074..06ac2c1 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -551,6 +551,14 @@ static inline int mingw_mkdir(const char *path, int mode)
}
#define mkdir mingw_mkdir
+static inline int mingw_unlink(const char *pathname)
+{
+ /* read-only files cannot be removed */
+ chmod(pathname, 0666);
+ return unlink(pathname);
+}
+#define unlink mingw_unlink
+
static inline int waitpid(pid_t pid, unsigned *status, unsigned options)
{
if (options == 0)
--
1.5.4.1.126.ge5a7d
-
Hi, May I request that this embarrassing typo be fixed before applying? ("a read-only files" -> "read-only files".) Thanks, Dscho -
I've integrated the feedback that I received in this series. Thanks to
has_dos_drive_prefix() as suggested by Dscho quite a number of #ifdef's could
be removed. Below is the interdiff between the old and the new state.
However, I have not yet moved stuff out of git-compat-util.h into
compat/mingw.h to keep this message readable. I'll do that later after I have
rebased the series on top of the latest git.git master.
BTW, the last hunk in the interdiff is a new bug-fix.
-- Hannes
diff --git a/Makefile b/Makefile
index 53a4e2a..2ea53c0 100644
--- a/Makefile
+++ b/Makefile
@@ -265,6 +265,7 @@ PROGRAMS = \
git-pack-redundant$X git-var$X \
git-merge-tree$X \
git-merge-recursive$X \
+ $(POSIX_ONLY_PROGRAMS) \
$(EXTRA_PROGRAMS)
# Empty...
@@ -541,9 +542,11 @@ ifneq (,$(findstring MINGW,$(uname_S)))
NO_MKDTEMP = YesPlease
NO_SVN_TESTS = YesPlease
NO_PERL_MAKEMAKER = YesPlease
- NO_EXTRA_PROGRAMS = YesPlease
+ NO_POSIX_ONLY_PROGRAMS = YesPlease
COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat
+ COMPAT_CFLAGS += -DPATH_SEP="';'"
COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
+ COMPAT_CFLAGS += -DPRIuMAX=\"I64u\"
COMPAT_OBJS += compat/mingw.o compat/fnmatch.o compat/regex.o
EXTLIBS += -lws2_32
X = .exe
@@ -611,8 +614,8 @@ ifdef ZLIB_PATH
endif
EXTLIBS += -lz
-ifndef NO_EXTRA_PROGRAMS
- EXTRA_PROGRAMS += \
+ifndef NO_POSIX_ONLY_PROGRAMS
+ POSIX_ONLY_PROGRAMS = \
git-daemon$X \
git-imap-send$X
endif
diff --git a/cache.h b/cache.h
index 3e4e10a..781fa40 100644
--- a/cache.h
+++ b/cache.h
@@ -441,11 +441,7 @@ int safe_create_leading_directories(char *path);
char *enter_repo(char *path, int strict);
static inline int is_absolute_path(const char *path)
{
-#ifndef __MINGW32__
- return path[0] == '/';
-#else
- return path[0] == '/' || (path[0] && path[1] == ':');
-#endif
+ return path[0] == '/' || has_dos_drive_prefix(path);
}
const char *make_absolute_path(const char *path);
diff --git a/compat/mingw.c ...Hi, Thanks! And especially thanks for the interdiff, it makes reviewing much easier This must be the bug fix ;-) I like it! Thanks, Dscho --
It's not my fault, I swear it. It went like this: I erased the line, but then changed my mind and hit Undo. But then I thought, this is a serious project, we don't need such language. And hit Redo. Which made me think again... So I said to my little rascal, who was playing Lego: "Say 'stop'", and I hit Undo and Redo as fast as I could. And when he said "Stop", oh well, I had just hit Redo. So here we have it - no longer. ;-) -- Hannes --
I just have to say, MASSIVE work Hannes! Thank you so much for your=20 efforts in making the MinGW port the great port that it is! You too Dscho, and the rest of the MSys Git team! ;-) -- =2Emarius
On Thu, Feb 28, 2008 at 11:01 AM, Marius Storm-Olsen Indeed. Amazing stuff. I do some occasional work on Windows, and it is a sheer pleasure to use msys git. Simple install, it just works, and it is fast fast FAST. Hats off! m -
On Thu, Feb 28, 2008 at 6:34 AM, Martin Langhoff Yeah. I've been waiting this moment for too long. Thanks all. -- Duy -
Hi, We have patches in msysGit for that. Maybe they should come _before_ the AFAIR this is needed to call git from git-gui, because of that fscked-up semantics of Windows' CreateProcess(). I'll try to review the patches in the next days, but I have no doubt that there is not much to complain about. Thanks, Hannes, for all your valuable and persistent work on mingw.git! Ciao, Dscho -
