* [PULL] Pull request from msysGit @ 2010-10-04 23:52 Pat Thoyts 2010-10-05 16:45 ` Junio C Hamano 2010-10-07 17:17 ` Ramsay Jones 0 siblings, 2 replies; 7+ messages in thread From: Pat Thoyts @ 2010-10-04 23:52 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, msysgit This follows up on the comments made for the previous pull request for patches from msysGit. This batch is restricted to those that passed review or have been modified as a result of the earlier review. I've added some Acked-by's resulting from comments made. The following changes since commit 1e633418479926bc85ed21a4f91c845a3dd3ad66: Merge branch 'maint' (2010-09-30 14:59:53 -0700) are available in the git repository at: git://repo.or.cz/git/mingw/4msysgit.git work/pt/for-junio or alternatively http://repo.or.cz/w/git/mingw/4msysgit.git/shortlog/refs/heads/work/pt/for-junio 5debf9a Add MinGW-specific execv() override. 77df1f1 Fix Windows-specific macro redefinition warning. b248e95 Fix 'clone' failure at DOS root directory. 1a40420 mingw: do not crash on open(NULL, ...) 5e9677c git-am: fix detection of absolute paths for windows 36e035f Side-step MSYS-specific path "corruption" leading to t5560 failure. ca02ad3 Side-step sed line-ending "corruption" leading to t6038 failure. 97f2c33 Skip 'git archive --remote' test on msysGit a94114a Do not strip CR when grepping HTTP headers. 3ba9ba8 Skip t1300.70 and 71 on msysGit. 4e57baf merge-octopus: Work around environment issue on Windows 442dada MinGW: Report errors when failing to launch the html browser. 9b9784c MinGW: fix stat() and lstat() implementations for handling symlinks 4091bfc MinGW: Add missing file mode bit defines e7cf4e9 MinGW: Use pid_t more consequently, introduce uid_t for greater compatibility abspath.c | 6 +++- compat/mingw.c | 56 ++++++++++++++++++++++++++++++++----- compat/mingw.h | 36 +++++++++++++++++++----- git-am.sh | 12 ++++---- git-merge-octopus.sh | 5 +++ git-sh-setup.sh | 15 ++++++++++ t/t1300-repo-config.sh | 6 ++-- t/t5000-tar-tree.sh | 2 +- t/t5503-tagfollow.sh | 9 +----- t/t5560-http-backend-noserver.sh | 5 ++- t/t6038-merge-text-auto.sh | 4 ++- t/test-lib.sh | 2 + 12 files changed, 122 insertions(+), 36 deletions(-) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PULL] Pull request from msysGit 2010-10-04 23:52 [PULL] Pull request from msysGit Pat Thoyts @ 2010-10-05 16:45 ` Junio C Hamano 2010-10-07 17:17 ` Ramsay Jones 1 sibling, 0 replies; 7+ messages in thread From: Junio C Hamano @ 2010-10-05 16:45 UTC (permalink / raw) To: Pat Thoyts; +Cc: git, msysgit Pat Thoyts <patthoyts@users.sourceforge.net> writes: > This follows up on the comments made for the previous pull request for > patches from msysGit. This batch is restricted to those that passed > review or have been modified as a result of the earlier review. > I've added some Acked-by's resulting from comments made. > > The following changes since commit 1e633418479926bc85ed21a4f91c845a3dd3ad66: > > Merge branch 'maint' (2010-09-30 14:59:53 -0700) > > are available in the git repository at: > > git://repo.or.cz/git/mingw/4msysgit.git work/pt/for-junio > or alternatively > http://repo.or.cz/w/git/mingw/4msysgit.git/shortlog/refs/heads/work/pt/for-junio Thanks. > 5debf9a Add MinGW-specific execv() override. > 77df1f1 Fix Windows-specific macro redefinition warning. > b248e95 Fix 'clone' failure at DOS root directory. > 1a40420 mingw: do not crash on open(NULL, ...) > 5e9677c git-am: fix detection of absolute paths for windows > 36e035f Side-step MSYS-specific path "corruption" leading to t5560 failure. > ca02ad3 Side-step sed line-ending "corruption" leading to t6038 failure. > 97f2c33 Skip 'git archive --remote' test on msysGit > a94114a Do not strip CR when grepping HTTP headers. > 3ba9ba8 Skip t1300.70 and 71 on msysGit. > 4e57baf merge-octopus: Work around environment issue on Windows > 442dada MinGW: Report errors when failing to launch the html browser. > 9b9784c MinGW: fix stat() and lstat() implementations for handling symlinks > 4091bfc MinGW: Add missing file mode bit defines > e7cf4e9 MinGW: Use pid_t more consequently, introduce uid_t for greater compatibility > > abspath.c | 6 +++- > compat/mingw.c | 56 ++++++++++++++++++++++++++++++++----- > compat/mingw.h | 36 +++++++++++++++++++----- > git-am.sh | 12 ++++---- > git-merge-octopus.sh | 5 +++ > git-sh-setup.sh | 15 ++++++++++ > t/t1300-repo-config.sh | 6 ++-- > t/t5000-tar-tree.sh | 2 +- > t/t5503-tagfollow.sh | 9 +----- > t/t5560-http-backend-noserver.sh | 5 ++- > t/t6038-merge-text-auto.sh | 4 ++- > t/test-lib.sh | 2 + > 12 files changed, 122 insertions(+), 36 deletions(-) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PULL] Pull request from msysGit 2010-10-04 23:52 [PULL] Pull request from msysGit Pat Thoyts 2010-10-05 16:45 ` Junio C Hamano @ 2010-10-07 17:17 ` Ramsay Jones 2010-10-07 19:30 ` Peter Harris 1 sibling, 1 reply; 7+ messages in thread From: Ramsay Jones @ 2010-10-07 17:17 UTC (permalink / raw) To: Pat Thoyts; +Cc: Junio C Hamano, git, msysgit, pharris, sschuberth Pat Thoyts wrote: > The following changes since commit 1e633418479926bc85ed21a4f91c845a3dd3ad66: > > Merge branch 'maint' (2010-09-30 14:59:53 -0700) > > are available in the git repository at: > > git://repo.or.cz/git/mingw/4msysgit.git work/pt/for-junio > or alternatively > http://repo.or.cz/w/git/mingw/4msysgit.git/shortlog/refs/heads/work/pt/for-junio > > 5debf9a Add MinGW-specific execv() override. > 77df1f1 Fix Windows-specific macro redefinition warning. > b248e95 Fix 'clone' failure at DOS root directory. > 1a40420 mingw: do not crash on open(NULL, ...) > 5e9677c git-am: fix detection of absolute paths for windows > 36e035f Side-step MSYS-specific path "corruption" leading to t5560 failure. > ca02ad3 Side-step sed line-ending "corruption" leading to t6038 failure. > 97f2c33 Skip 'git archive --remote' test on msysGit > a94114a Do not strip CR when grepping HTTP headers. > 3ba9ba8 Skip t1300.70 and 71 on msysGit. > 4e57baf merge-octopus: Work around environment issue on Windows > 442dada MinGW: Report errors when failing to launch the html browser. > 9b9784c MinGW: fix stat() and lstat() implementations for handling symlinks > 4091bfc MinGW: Add missing file mode bit defines This commit (4091bfc) may well introduce logic errors into the msvc build; I haven't checked (it depends on what the msvc compiler does when a macro is redefined - does the new definition replace the old?). However, no matter what else may be wrong, this commit introduces a shed load of new warnings, thus: $ make clean $ make MSVC=1 >out 2>&1 $ grep warning out | grep S_I | wc -l 1000 $ so 1000 additional warnings which, looking at the start of the out file, look like this: GIT_VERSION = 1.7.3.dirty * new build flags or prefix CC fast-import.o fast-import.c c:\cygwin\home\ramsay\git\compat/mingw.h(16) : warning C4005: 'S_IRUSR' : macro redefinition compat/vcbuild/include\unistd.h(85) : see previous definition of 'S_IRUSR' c:\cygwin\home\ramsay\git\compat/mingw.h(17) : warning C4005: 'S_IWUSR' : macro redefinition compat/vcbuild/include\unistd.h(84) : see previous definition of 'S_IWUSR' c:\cygwin\home\ramsay\git\compat/mingw.h(18) : warning C4005: 'S_IXUSR' : macro redefinition compat/vcbuild/include\unistd.h(83) : see previous definition of 'S_IXUSR' c:\cygwin\home\ramsay\git\compat/mingw.h(19) : warning C4005: 'S_IRWXU' : macro redefinition compat/vcbuild/include\unistd.h(82) : see previous definition of 'S_IRWXU' Now, Peter Harris has already submitted a fix for this, which is currently on the work/msvc-fixes branch, which contains: 358f1be Modify MSVC wrapper script 38bd27d Fix MSVC build The suggested fix is given in commit 38bd27d. However, I prefer a different solution, which is given below: --- >8 --- diff --git a/compat/mingw.h b/compat/mingw.h index afedf3a..445d1a1 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -12,12 +12,6 @@ typedef int pid_t; #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK) #define S_ISSOCK(x) 0 -#ifndef _STAT_H_ -#define S_IRUSR 0 -#define S_IWUSR 0 -#define S_IXUSR 0 -#define S_IRWXU (S_IRUSR | S_IWUSR | S_IXUSR) -#endif #define S_IRGRP 0 #define S_IWGRP 0 #define S_IXGRP 0 --- 8< --- Note that, for *both* MinGW and MSVC, the deleted #defines are not wanted, pointless and just plain wrong! :-D If you squash the above into 4091bfc then we find: $ make clean $ make MSVC=1 >out1 2>&1 $ grep warning out1 | grep S_I | wc -l 0 $ and there is also no chance of introducing a logic error. Although I'm not recommending you use one of the commits on the work/msvc-fixes branch, can I request, once again, that you include: 358f1be Modify MSVC wrapper script If it makes any difference, you can add: Acked-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk> ATB, Ramsay Jones ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PULL] Pull request from msysGit 2010-10-07 17:17 ` Ramsay Jones @ 2010-10-07 19:30 ` Peter Harris 2010-10-07 20:18 ` [msysGit] " Pat Thoyts 0 siblings, 1 reply; 7+ messages in thread From: Peter Harris @ 2010-10-07 19:30 UTC (permalink / raw) To: Ramsay Jones; +Cc: Pat Thoyts, Junio C Hamano, git, msysgit, sschuberth On 2010-10-07 13:17, Ramsay Jones wrote: > Now, Peter Harris has already submitted a fix for this, which is > currently on the work/msvc-fixes branch, which contains: > > 358f1be Modify MSVC wrapper script > 38bd27d Fix MSVC build > > The suggested fix is given in commit 38bd27d. However, I prefer a > different solution, which is given below: > > --- >8 --- > diff --git a/compat/mingw.h b/compat/mingw.h > index afedf3a..445d1a1 100644 > --- a/compat/mingw.h > +++ b/compat/mingw.h > @@ -12,12 +12,6 @@ typedef int pid_t; > #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK) > #define S_ISSOCK(x) 0 > > -#ifndef _STAT_H_ > -#define S_IRUSR 0 > -#define S_IWUSR 0 > -#define S_IXUSR 0 > -#define S_IRWXU (S_IRUSR | S_IWUSR | S_IXUSR) > -#endif > #define S_IRGRP 0 > #define S_IWGRP 0 > #define S_IXGRP 0 > --- 8< --- > > Note that, for *both* MinGW and MSVC, the deleted #defines > are not wanted, pointless and just plain wrong! :-D I didn't realize that the defines were not wanted for MinGW either. I heartily approve of removing code rather than just ifdefing around it. Please use this version of the patch instead of mine. Peter Harris -- Open Text Connectivity Solutions Group Peter Harris http://connectivity.opentext.com/ Research and Development Phone: +1 905 762 6001 pharris@opentext.com Toll Free: 1 877 359 4866 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [msysGit] Re: [PULL] Pull request from msysGit 2010-10-07 19:30 ` Peter Harris @ 2010-10-07 20:18 ` Pat Thoyts 2010-10-07 20:22 ` Erik Faye-Lund 2010-10-09 17:56 ` Ramsay Jones 0 siblings, 2 replies; 7+ messages in thread From: Pat Thoyts @ 2010-10-07 20:18 UTC (permalink / raw) To: Peter Harris Cc: Ramsay Jones, Pat Thoyts, Junio C Hamano, git, msysgit, sschuberth On 7 October 2010 20:30, Peter Harris <pharris@opentext.com> wrote: > On 2010-10-07 13:17, Ramsay Jones wrote: >> Now, Peter Harris has already submitted a fix for this, which is >> currently on the work/msvc-fixes branch, which contains: >> >> 358f1be Modify MSVC wrapper script >> 38bd27d Fix MSVC build >> >> The suggested fix is given in commit 38bd27d. However, I prefer a >> different solution, which is given below: >> >> --- >8 --- >> diff --git a/compat/mingw.h b/compat/mingw.h >> index afedf3a..445d1a1 100644 >> --- a/compat/mingw.h >> +++ b/compat/mingw.h >> @@ -12,12 +12,6 @@ typedef int pid_t; >> #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK) >> #define S_ISSOCK(x) 0 >> >> -#ifndef _STAT_H_ >> -#define S_IRUSR 0 >> -#define S_IWUSR 0 >> -#define S_IXUSR 0 >> -#define S_IRWXU (S_IRUSR | S_IWUSR | S_IXUSR) >> -#endif >> #define S_IRGRP 0 >> #define S_IWGRP 0 >> #define S_IXGRP 0 >> --- 8< --- >> >> Note that, for *both* MinGW and MSVC, the deleted #defines >> are not wanted, pointless and just plain wrong! :-D > > I didn't realize that the defines were not wanted for MinGW either. > > I heartily approve of removing code rather than just ifdefing around it. > Please use this version of the patch instead of mine. > > Peter Harris The patch in question has been on the msysGit tree for about 10 months now. Its somewhat disappointing not to have had it spotted before we pushed it upstream. Are the msvc builders only working against junio's repository? Reverting it seems to make no difference to the msysGit build at all - presumably because S_IRUSR and friends are all defined in the mingw <sys/stat.h> anyway. Sebastian - can you recall why this got added? The commit comment is not all that enlightening. I also wonder why changes to a compat/mingw.h file should affect the msvc build. As it has it's own compat/vcbuild and headers in there, surely it should be independent of mingw-gcc compatability headers? Pat Thoyts ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [msysGit] Re: [PULL] Pull request from msysGit 2010-10-07 20:18 ` [msysGit] " Pat Thoyts @ 2010-10-07 20:22 ` Erik Faye-Lund 2010-10-09 17:56 ` Ramsay Jones 1 sibling, 0 replies; 7+ messages in thread From: Erik Faye-Lund @ 2010-10-07 20:22 UTC (permalink / raw) To: Pat Thoyts Cc: Peter Harris, Ramsay Jones, Pat Thoyts, Junio C Hamano, git, msysgit, sschuberth On Thu, Oct 7, 2010 at 10:18 PM, Pat Thoyts <patthoyts@gmail.com> wrote: > > I also wonder why changes to a compat/mingw.h file should affect the > msvc build. As it has it's own compat/vcbuild and headers in there, > surely it should be independent of mingw-gcc compatability headers? > compat/msvc.h includes compat/mingw.h. We really should move more stuff to compat/win32.h, and have cygwin include it's own header or something instead. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [msysGit] Re: [PULL] Pull request from msysGit 2010-10-07 20:18 ` [msysGit] " Pat Thoyts 2010-10-07 20:22 ` Erik Faye-Lund @ 2010-10-09 17:56 ` Ramsay Jones 1 sibling, 0 replies; 7+ messages in thread From: Ramsay Jones @ 2010-10-09 17:56 UTC (permalink / raw) To: Pat Thoyts Cc: Peter Harris, Pat Thoyts, Junio C Hamano, git, msysgit, sschuberth Pat Thoyts wrote: > The patch in question has been on the msysGit tree for about 10 months > now. Its somewhat disappointing not to have had it spotted before we > pushed it upstream. Are the msvc builders only working against junio's > repository? Well, I can't speak for anyone else, but for me the answer is yes. ;-) I build git (from the git.kernel.org repository) on Linux, cygwin, cygwin+msvc and msysGit. (For a short while I also built it on FreeBSD hosted in a VM - but that was so slow, I soon stopped doing that!). So, the git I *use* on MinGW/msysGit is built from junio's repository. I only installed msysGit to enable me to check that my patches worked on MinGW (I was tired of always saying "could somebody test this on MinGW ..."). As you may have noticed, the mingw and msvc builds share quite a bit of compatibility code... [I don't follow 4msysgit or subscribe to the msysgit mailing list, so I would not normally see any 4msysgit patches, unless they were also discussed on the git mailing list.] ATB, Ramsay Jones ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-10-09 17:58 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-10-04 23:52 [PULL] Pull request from msysGit Pat Thoyts 2010-10-05 16:45 ` Junio C Hamano 2010-10-07 17:17 ` Ramsay Jones 2010-10-07 19:30 ` Peter Harris 2010-10-07 20:18 ` [msysGit] " Pat Thoyts 2010-10-07 20:22 ` Erik Faye-Lund 2010-10-09 17:56 ` Ramsay Jones
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).