{"thread":{"id":"25351","subject":"[PULL] Pull request from msysGit","startedAt":"2010-10-04T23:52:20Z","lastAt":"2010-10-09T17:56:11Z","messageCount":7,"participants":["Pat Thoyts","Junio C Hamano","Ramsay Jones","Peter Harris","Erik Faye-Lund"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"152622","messageId":"87ocb9zfbf.fsf@fox.patthoyts.tk","threadId":"25351","inReplyTo":null,"subject":"[PULL] Pull request from msysGit","fromName":"Pat Thoyts","fromEmail":"patthoyts@users.sourceforge.net","sentAt":"2010-10-04T23:52:20Z","receivedAt":"2010-10-04T23:52:20Z","isPatch":false,"sender":{"key":"patthoyts@users.sourceforge.net","avatar":"https://avatars.githubusercontent.com/u/30739?v=4"},"body":"This follows up on the comments made for the previous pull request for\npatches from msysGit. This batch is restricted to those that passed\nreview or have been modified as a result of the earlier review.\nI've added some Acked-by's resulting from comments made.\n\nThe following changes since commit 1e633418479926bc85ed21a4f91c845a3dd3ad66:\n\n  Merge branch 'maint' (2010-09-30 14:59:53 -0700)\n\nare available in the git repository at:\n\n  git://repo.or.cz/git/mingw/4msysgit.git work/pt/for-junio\nor alternatively\n  http://repo.or.cz/w/git/mingw/4msysgit.git/shortlog/refs/heads/work/pt/for-junio\n\n5debf9a Add MinGW-specific execv() override.\n77df1f1 Fix Windows-specific macro redefinition warning.\nb248e95 Fix 'clone' failure at DOS root directory.\n1a40420 mingw: do not crash on open(NULL, ...)\n5e9677c git-am: fix detection of absolute paths for windows\n36e035f Side-step MSYS-specific path \"corruption\" leading to t5560 failure.\nca02ad3 Side-step sed line-ending \"corruption\" leading to t6038 failure.\n97f2c33 Skip 'git archive --remote' test on msysGit\na94114a Do not strip CR when grepping HTTP headers.\n3ba9ba8 Skip t1300.70 and 71 on msysGit.\n4e57baf merge-octopus: Work around environment issue on Windows\n442dada MinGW: Report errors when failing to launch the html browser.\n9b9784c MinGW: fix stat() and lstat() implementations for handling symlinks\n4091bfc MinGW: Add missing file mode bit defines\ne7cf4e9 MinGW: Use pid_t more consequently, introduce uid_t for greater compatibility\n\n abspath.c                        |    6 +++-\n compat/mingw.c                   |   56 ++++++++++++++++++++++++++++++++-----\n compat/mingw.h                   |   36 +++++++++++++++++++-----\n git-am.sh                        |   12 ++++----\n git-merge-octopus.sh             |    5 +++\n git-sh-setup.sh                  |   15 ++++++++++\n t/t1300-repo-config.sh           |    6 ++--\n t/t5000-tar-tree.sh              |    2 +-\n t/t5503-tagfollow.sh             |    9 +-----\n t/t5560-http-backend-noserver.sh |    5 ++-\n t/t6038-merge-text-auto.sh       |    4 ++-\n t/test-lib.sh                    |    2 +\n 12 files changed, 122 insertions(+), 36 deletions(-)\n"},{"id":"152692","messageId":"7v8w2c7fn6.fsf@alter.siamese.dyndns.org","threadId":"25351","inReplyTo":"87ocb9zfbf.fsf@fox.patthoyts.tk","subject":"Re: [PULL] Pull request from msysGit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-10-05T16:45:01Z","receivedAt":"2010-10-05T16:45:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pat Thoyts <patthoyts@users.sourceforge.net> writes:\n\n> This follows up on the comments made for the previous pull request for\n> patches from msysGit. This batch is restricted to those that passed\n> review or have been modified as a result of the earlier review.\n> I've added some Acked-by's resulting from comments made.\n>\n> The following changes since commit 1e633418479926bc85ed21a4f91c845a3dd3ad66:\n>\n>   Merge branch 'maint' (2010-09-30 14:59:53 -0700)\n>\n> are available in the git repository at:\n>\n>   git://repo.or.cz/git/mingw/4msysgit.git work/pt/for-junio\n> or alternatively\n>   http://repo.or.cz/w/git/mingw/4msysgit.git/shortlog/refs/heads/work/pt/for-junio\n\nThanks.\n\n> 5debf9a Add MinGW-specific execv() override.\n> 77df1f1 Fix Windows-specific macro redefinition warning.\n> b248e95 Fix 'clone' failure at DOS root directory.\n> 1a40420 mingw: do not crash on open(NULL, ...)\n> 5e9677c git-am: fix detection of absolute paths for windows\n> 36e035f Side-step MSYS-specific path \"corruption\" leading to t5560 failure.\n> ca02ad3 Side-step sed line-ending \"corruption\" leading to t6038 failure.\n> 97f2c33 Skip 'git archive --remote' test on msysGit\n> a94114a Do not strip CR when grepping HTTP headers.\n> 3ba9ba8 Skip t1300.70 and 71 on msysGit.\n> 4e57baf merge-octopus: Work around environment issue on Windows\n> 442dada MinGW: Report errors when failing to launch the html browser.\n> 9b9784c MinGW: fix stat() and lstat() implementations for handling symlinks\n> 4091bfc MinGW: Add missing file mode bit defines\n> e7cf4e9 MinGW: Use pid_t more consequently, introduce uid_t for greater compatibility\n>\n>  abspath.c                        |    6 +++-\n>  compat/mingw.c                   |   56 ++++++++++++++++++++++++++++++++-----\n>  compat/mingw.h                   |   36 +++++++++++++++++++-----\n>  git-am.sh                        |   12 ++++----\n>  git-merge-octopus.sh             |    5 +++\n>  git-sh-setup.sh                  |   15 ++++++++++\n>  t/t1300-repo-config.sh           |    6 ++--\n>  t/t5000-tar-tree.sh              |    2 +-\n>  t/t5503-tagfollow.sh             |    9 +-----\n>  t/t5560-http-backend-noserver.sh |    5 ++-\n>  t/t6038-merge-text-auto.sh       |    4 ++-\n>  t/test-lib.sh                    |    2 +\n>  12 files changed, 122 insertions(+), 36 deletions(-)\n"},{"id":"152883","messageId":"4CAE00C5.1050509@ramsay1.demon.co.uk","threadId":"25351","inReplyTo":"87ocb9zfbf.fsf@fox.patthoyts.tk","subject":"Re: [PULL] Pull request from msysGit","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2010-10-07T17:17:57Z","receivedAt":"2010-10-07T17:17:57Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Pat Thoyts wrote:\n> The following changes since commit 1e633418479926bc85ed21a4f91c845a3dd3ad66:\n> \n>   Merge branch 'maint' (2010-09-30 14:59:53 -0700)\n> \n> are available in the git repository at:\n> \n>   git://repo.or.cz/git/mingw/4msysgit.git work/pt/for-junio\n> or alternatively\n>   http://repo.or.cz/w/git/mingw/4msysgit.git/shortlog/refs/heads/work/pt/for-junio\n> \n> 5debf9a Add MinGW-specific execv() override.\n> 77df1f1 Fix Windows-specific macro redefinition warning.\n> b248e95 Fix 'clone' failure at DOS root directory.\n> 1a40420 mingw: do not crash on open(NULL, ...)\n> 5e9677c git-am: fix detection of absolute paths for windows\n> 36e035f Side-step MSYS-specific path \"corruption\" leading to t5560 failure.\n> ca02ad3 Side-step sed line-ending \"corruption\" leading to t6038 failure.\n> 97f2c33 Skip 'git archive --remote' test on msysGit\n> a94114a Do not strip CR when grepping HTTP headers.\n> 3ba9ba8 Skip t1300.70 and 71 on msysGit.\n> 4e57baf merge-octopus: Work around environment issue on Windows\n> 442dada MinGW: Report errors when failing to launch the html browser.\n> 9b9784c MinGW: fix stat() and lstat() implementations for handling symlinks\n> 4091bfc MinGW: Add missing file mode bit defines\n\nThis commit (4091bfc) may well introduce logic errors into the msvc\nbuild; I haven't checked (it depends on what the msvc compiler does\nwhen a macro is redefined - does the new definition replace the old?).\nHowever, no matter what else may be wrong, this commit introduces a\nshed load of new warnings, thus:\n\n    $ make clean\n    $ make MSVC=1 >out 2>&1\n    $ grep warning out | grep S_I | wc -l\n    1000\n    $ \n\nso 1000 additional warnings which, looking at the start of the out\nfile, look like this:\n\nGIT_VERSION = 1.7.3.dirty\n    * new build flags or prefix\n    CC fast-import.o\nfast-import.c\nc:\\cygwin\\home\\ramsay\\git\\compat/mingw.h(16) : warning C4005: 'S_IRUSR' : macro redefinition\n        compat/vcbuild/include\\unistd.h(85) : see previous definition of 'S_IRUSR'\nc:\\cygwin\\home\\ramsay\\git\\compat/mingw.h(17) : warning C4005: 'S_IWUSR' : macro redefinition\n        compat/vcbuild/include\\unistd.h(84) : see previous definition of 'S_IWUSR'\nc:\\cygwin\\home\\ramsay\\git\\compat/mingw.h(18) : warning C4005: 'S_IXUSR' : macro redefinition\n        compat/vcbuild/include\\unistd.h(83) : see previous definition of 'S_IXUSR'\nc:\\cygwin\\home\\ramsay\\git\\compat/mingw.h(19) : warning C4005: 'S_IRWXU' : macro redefinition\n        compat/vcbuild/include\\unistd.h(82) : see previous definition of 'S_IRWXU'\n\nNow, Peter Harris has already submitted a fix for this, which is\ncurrently on the work/msvc-fixes branch, which contains:\n\n    358f1be Modify MSVC wrapper script\n    38bd27d Fix MSVC build\n\nThe suggested fix is given in commit 38bd27d. However, I prefer a\ndifferent solution, which is given below:\n\n--- >8 ---\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex afedf3a..445d1a1 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -12,12 +12,6 @@ typedef int pid_t;\n #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)\n #define S_ISSOCK(x) 0\n \n-#ifndef _STAT_H_\n-#define S_IRUSR 0\n-#define S_IWUSR 0\n-#define S_IXUSR 0\n-#define S_IRWXU (S_IRUSR | S_IWUSR | S_IXUSR)\n-#endif\n #define S_IRGRP 0\n #define S_IWGRP 0\n #define S_IXGRP 0\n--- 8< ---\n\nNote that, for *both* MinGW and MSVC, the deleted #defines\nare not wanted, pointless and just plain wrong! :-D\n\nIf you squash the above into 4091bfc then we find:\n\n    $ make clean\n    $ make MSVC=1 >out1 2>&1\n    $ grep warning out1 | grep S_I | wc -l\n    0\n    $ \n\nand there is also no chance of introducing a logic error.\n\nAlthough I'm not recommending you use one of the commits on\nthe work/msvc-fixes branch, can I request, once again, that\nyou include:\n\n    358f1be Modify MSVC wrapper script\n\nIf it makes any difference, you can add:\n\n    Acked-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>\n\nATB,\nRamsay Jones\n"},{"id":"152920","messageId":"4CAE1FE6.9020306@opentext.com","threadId":"25351","inReplyTo":"4CAE00C5.1050509@ramsay1.demon.co.uk","subject":"Re: [PULL] Pull request from msysGit","fromName":"Peter Harris","fromEmail":"pharris@opentext.com","sentAt":"2010-10-07T19:30:46Z","receivedAt":"2010-10-07T19:30:46Z","isPatch":false,"sender":{"key":"pharris@opentext.com","avatar":"https://gravatar.com/avatar/ce4904dee5af2425a32fe0a6e3cefa7895a043d4d97d14dcd7425986bd41ba04?d=mp&s=160"},"body":"On 2010-10-07 13:17, Ramsay Jones wrote:\n> Now, Peter Harris has already submitted a fix for this, which is\n> currently on the work/msvc-fixes branch, which contains:\n> \n>     358f1be Modify MSVC wrapper script\n>     38bd27d Fix MSVC build\n> \n> The suggested fix is given in commit 38bd27d. However, I prefer a\n> different solution, which is given below:\n> \n> --- >8 ---\n> diff --git a/compat/mingw.h b/compat/mingw.h\n> index afedf3a..445d1a1 100644\n> --- a/compat/mingw.h\n> +++ b/compat/mingw.h\n> @@ -12,12 +12,6 @@ typedef int pid_t;\n>  #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)\n>  #define S_ISSOCK(x) 0\n>  \n> -#ifndef _STAT_H_\n> -#define S_IRUSR 0\n> -#define S_IWUSR 0\n> -#define S_IXUSR 0\n> -#define S_IRWXU (S_IRUSR | S_IWUSR | S_IXUSR)\n> -#endif\n>  #define S_IRGRP 0\n>  #define S_IWGRP 0\n>  #define S_IXGRP 0\n> --- 8< ---\n> \n> Note that, for *both* MinGW and MSVC, the deleted #defines\n> are not wanted, pointless and just plain wrong! :-D\n\nI didn't realize that the defines were not wanted for MinGW either.\n\nI heartily approve of removing code rather than just ifdefing around it.\nPlease use this version of the patch instead of mine.\n\nPeter Harris\n-- \n               Open Text Connectivity Solutions Group\nPeter Harris                    http://connectivity.opentext.com/\nResearch and Development        Phone: +1 905 762 6001\npharris@opentext.com            Toll Free: 1 877 359 4866\n"},{"id":"152924","messageId":"AANLkTinSjFDdwqTEU6XzOVHupph0G2ZKM+u3r7t_W3DD@mail.gmail.com","threadId":"25351","inReplyTo":"4CAE1FE6.9020306@opentext.com","subject":"Re: [msysGit] Re: [PULL] Pull request from msysGit","fromName":"Pat Thoyts","fromEmail":"patthoyts@gmail.com","sentAt":"2010-10-07T20:18:43Z","receivedAt":"2010-10-07T20:18:43Z","isPatch":false,"sender":{"key":"patthoyts@gmail.com","avatar":"https://gravatar.com/avatar/bee887a777c790bd241f398217723fbe4b854428671db83db32216a28654cb25?d=mp&s=160"},"body":"On 7 October 2010 20:30, Peter Harris <pharris@opentext.com> wrote:\n> On 2010-10-07 13:17, Ramsay Jones wrote:\n>> Now, Peter Harris has already submitted a fix for this, which is\n>> currently on the work/msvc-fixes branch, which contains:\n>>\n>>     358f1be Modify MSVC wrapper script\n>>     38bd27d Fix MSVC build\n>>\n>> The suggested fix is given in commit 38bd27d. However, I prefer a\n>> different solution, which is given below:\n>>\n>> --- >8 ---\n>> diff --git a/compat/mingw.h b/compat/mingw.h\n>> index afedf3a..445d1a1 100644\n>> --- a/compat/mingw.h\n>> +++ b/compat/mingw.h\n>> @@ -12,12 +12,6 @@ typedef int pid_t;\n>>  #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)\n>>  #define S_ISSOCK(x) 0\n>>\n>> -#ifndef _STAT_H_\n>> -#define S_IRUSR 0\n>> -#define S_IWUSR 0\n>> -#define S_IXUSR 0\n>> -#define S_IRWXU (S_IRUSR | S_IWUSR | S_IXUSR)\n>> -#endif\n>>  #define S_IRGRP 0\n>>  #define S_IWGRP 0\n>>  #define S_IXGRP 0\n>> --- 8< ---\n>>\n>> Note that, for *both* MinGW and MSVC, the deleted #defines\n>> are not wanted, pointless and just plain wrong! :-D\n>\n> I didn't realize that the defines were not wanted for MinGW either.\n>\n> I heartily approve of removing code rather than just ifdefing around it.\n> Please use this version of the patch instead of mine.\n>\n> Peter Harris\n\nThe patch in question has been on the msysGit tree for about 10 months\nnow. Its somewhat disappointing not to have had it spotted before we\npushed it upstream. Are the msvc builders only working against junio's\nrepository?\n\nReverting it seems to make no difference to the msysGit build at all -\npresumably because S_IRUSR and friends are all defined in the mingw\n<sys/stat.h> anyway. Sebastian - can you recall why this got added?\nThe commit comment is not all that enlightening.\n\nI also wonder why changes to a compat/mingw.h file should affect the\nmsvc build. As it has it's own compat/vcbuild and headers in there,\nsurely it should be independent of mingw-gcc compatability headers?\n\nPat Thoyts\n"},{"id":"152925","messageId":"AANLkTimYywBtG-tD-aV6uDK+HPerDHqaJyt1Sx4tOXJT@mail.gmail.com","threadId":"25351","inReplyTo":"AANLkTinSjFDdwqTEU6XzOVHupph0G2ZKM+u3r7t_W3DD@mail.gmail.com","subject":"Re: [msysGit] Re: [PULL] Pull request from msysGit","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2010-10-07T20:22:48Z","receivedAt":"2010-10-07T20:22:48Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Thu, Oct 7, 2010 at 10:18 PM, Pat Thoyts <patthoyts@gmail.com> wrote:\n>\n> I also wonder why changes to a compat/mingw.h file should affect the\n> msvc build. As it has it's own compat/vcbuild and headers in there,\n> surely it should be independent of mingw-gcc compatability headers?\n>\n\ncompat/msvc.h includes compat/mingw.h. We really should move more\nstuff to compat/win32.h, and have cygwin include it's own header or\nsomething instead.\n"},{"id":"153068","messageId":"4CB0ACBB.9040601@ramsay1.demon.co.uk","threadId":"25351","inReplyTo":"AANLkTinSjFDdwqTEU6XzOVHupph0G2ZKM+u3r7t_W3DD@mail.gmail.com","subject":"Re: [msysGit] Re: [PULL] Pull request from msysGit","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2010-10-09T17:56:11Z","receivedAt":"2010-10-09T17:56:11Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Pat Thoyts wrote:\n> The patch in question has been on the msysGit tree for about 10 months\n> now. Its somewhat disappointing not to have had it spotted before we\n> pushed it upstream. Are the msvc builders only working against junio's\n> repository?\n\nWell, I can't speak for anyone else, but for me the answer is yes. ;-)\nI build git (from the git.kernel.org repository) on Linux, cygwin,\ncygwin+msvc and msysGit. (For a short while I also built it on FreeBSD\nhosted in a VM - but that was so slow, I soon stopped doing that!).\n\nSo, the git I *use* on MinGW/msysGit is built from junio's repository.\nI only installed msysGit to enable me to check that my patches worked\non MinGW (I was tired of always saying \"could somebody test this on\nMinGW ...\"). As you may have noticed, the mingw and msvc builds share\nquite a bit of compatibility code...\n\n[I don't follow 4msysgit or subscribe to the msysgit mailing list, so\nI would not normally see any 4msysgit patches, unless they were also\ndiscussed on the git mailing list.]\n\nATB,\nRamsay Jones\n"}]}