{"thread":{"id":"38019","subject":"[RFC 2/4] Properly accept quoted space in filenames","startedAt":"2014-11-20T23:37:58Z","lastAt":"2014-11-23T21:58:48Z","messageCount":19,"participants":["Philip Oakley","Johannes Schindelin","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"252311","messageId":"1416526682-6024-1-git-send-email-philipoakley@iee.org","threadId":"38019","inReplyTo":null,"subject":"[RFC 0/4] Fix the Visual Studio 2008 .sln generator","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2014-11-20T23:37:58Z","receivedAt":"2014-11-20T23:37:58Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Potential Windows developers are likely to be using Visual Studio as\ntheir IDE. The tool stack required for Windows can be tortuous as it\ncrosses the boundaries between platforms and philosophies. This RFC\nseeks to maintain the tools that could assist such developers. In\nparticular, those tools that generate an initial Visual Studio project\n(.sln ) file.\n\nThe .sln generator in contrib began to break when internationalisation\nintroduced an extra -o option. This recently worsened with the addition\nof invalidcontinue.obj for 'improved POSIX compatibility'.\n\nI hacked a bit when I first attempted to use the VS IDE and noticed the\ni18n issue. I didn't completely solve all my issues because of further\nissues with VS2010 Express, so no patches were submitted at the time.\n\nNow, with a fresh copy of VS20008 Express, I see the additional problem\nof the addition of the invalidcontinue.obj reference causing the .sln\ngeneration to fail.\n\nAre the patches going in the right direction?\nIs the processing of the .obj file in engine.pl sensible?\nand the extra care with s/\\.o$/.c/ avoiding s/obj/cbj/.\nDoes it affect the Qmake capability? (I've no idea)\nIs the quoting of filenames correct? (my perl foo is cargo cult!)\n\nI've also updated the vcbuild/README to mention Msysgit (which\nwill be replaced soon by the newer/better Git-for-windows/SDK\n(https://github.com/git-for-windows/sdk), but the benefits still\napply.\n\nI've cc'd those who have contributed or patched the engine.pl, or\nappear to be interested via a $gmane search, who can hopefully comment.\n\nObviously, the patches will need squashing together, and the commit\nmessage(s) tidied after inclusion of comments.\n\nPhilip Oakley (4):\n  Fix i18n -o option in msvc engine.pl\n  Properly accept quoted space in filenames\n  engine.pl: split the .o and .obj processing\n  Improve layout and reference msvc-build script\n\n compat/vcbuild/README          | 29 +++++++++++++++++++++--------\n contrib/buildsystems/engine.pl | 38 ++++++++++++++++++++++++++++++--------\n 2 files changed, 51 insertions(+), 16 deletions(-)\n\n-- \n1.9.4.msysgit.0\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"Git for Windows\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"252313","messageId":"1416526682-6024-2-git-send-email-philipoakley@iee.org","threadId":"38019","inReplyTo":"1416526682-6024-1-git-send-email-philipoakley@iee.org","subject":"[RFC 1/4] Fix i18n -o option in msvc engine.pl","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2014-11-20T23:37:59Z","receivedAt":"2014-11-20T23:37:59Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"    The i18n 5e9637c6 introduced an extra '-o' option\n    into the make file, which broke engine.pl code for\n    extracting the git.sln for msvc gui-IDE.\n    add tests for 'msgfmt' and its precursor 'mkdir'\n    (in same vein as 74cf9bdda6).\n\n    Signed-off-by: Philip Oakley <philipoakley@iee.org>\n---\n contrib/buildsystems/engine.pl | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl\nindex 23da787..9144ea7 100755\n--- a/contrib/buildsystems/engine.pl\n+++ b/contrib/buildsystems/engine.pl\n@@ -140,6 +140,18 @@ sub parseMakeOutput\n             next;\n         }\n \n+        if ($text =~ /^mkdir /) {\n+            # options to the Portable Object translations in the line\n+            # mkdir -p po/... && msgfmt ... (eg -o) may be mistaken for linker options\n+            next;\n+        }\n+\n+        if ($text =~ /^msgfmt /) {\n+            # options to the Portable Object translations in the line\n+            # mkdir -p po/... && msgfmt ... (eg -o) may be mistaken for linker options\n+            next;\n+        }\n+\n         if($text =~ / -c /) {\n             # compilation\n             handleCompileLine($text, $line);\n-- \n1.9.4.msysgit.0\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"Git for Windows\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"252310","messageId":"1416526682-6024-3-git-send-email-philipoakley@iee.org","threadId":"38019","inReplyTo":"1416526682-6024-1-git-send-email-philipoakley@iee.org","subject":"[RFC 2/4] Properly accept quoted space in filenames","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2014-11-20T23:38:00Z","receivedAt":"2014-11-20T23:38:00Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"    the engine.pl script barfs on the properly quoted space\n    in filename options prevalent on Windows. Use quotewords()\n    rather than split() to separate such options.\n\n    Signed-off-by: Philip Oakley <philipoakley@iee.org>\n---\n contrib/buildsystems/engine.pl | 11 ++++++++---\n 1 file changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl\nindex 9144ea7..8e41808 100755\n--- a/contrib/buildsystems/engine.pl\n+++ b/contrib/buildsystems/engine.pl\n@@ -12,6 +12,7 @@ use File::Basename;\n use File::Spec;\n use Cwd;\n use Generators;\n+use Text::ParseWords;\n \n my (%build_structure, %compile_options, @makedry);\n my $out_dir = getcwd();\n@@ -243,7 +244,8 @@ sub removeDuplicates\n sub handleCompileLine\n {\n     my ($line, $lineno) = @_;\n-    my @parts = split(' ', $line);\n+    # my @parts = split(' ', $line);\n+    my @parts = quotewords('\\s+', 0, $line);\n     my $sourcefile;\n     shift(@parts); # ignore cmd\n     while (my $part = shift @parts) {\n@@ -277,7 +279,8 @@ sub handleLibLine\n     my (@objfiles, @lflags, $libout, $part);\n     # kill cmd and rm 'prefix'\n     $line =~ s/^rm -f .* && .* rcs //;\n-    my @parts = split(' ', $line);\n+    # my @parts = split(' ', $line);\n+    my @parts = quotewords('\\s+', 0, $line);\n     while ($part = shift @parts) {\n         if ($part =~ /^-/) {\n             push(@lflags, $part);\n@@ -318,7 +321,9 @@ sub handleLinkLine\n {\n     my ($line, $lineno) = @_;\n     my (@objfiles, @lflags, @libs, $appout, $part);\n-    my @parts = split(' ', $line);\n+    # my @parts = split(' ', $line);\n+    my @parts = quotewords('\\s+', 0, $line);\n+\n     shift(@parts); # ignore cmd\n     while ($part = shift @parts) {\n         if ($part =~ /^-IGNORE/) {\n-- \n1.9.4.msysgit.0\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"Git for Windows\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"252312","messageId":"1416526682-6024-4-git-send-email-philipoakley@iee.org","threadId":"38019","inReplyTo":"1416526682-6024-1-git-send-email-philipoakley@iee.org","subject":"[RFC 3/4] engine.pl: split the .o and .obj processing","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2014-11-20T23:38:01Z","receivedAt":"2014-11-20T23:38:01Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Commit 4b623d80f7352 added an .obj file (invalidcontinue.obj) which was not\nprocessed correctly.\n\nThe generate engine then mistakenly did a s/.o/.c/  to create a request\nto compile invalidcontinue.cbj.\n\nSplit the '/\\.(o|obj)$' in engine.pl#L353 into:\n\n        } elsif ($part =~ /\\.o$/) { # was '/\\.(o|obj)$'\n            push(@objfiles, $part);\n        } elsif ($part =~ /\\.obj$/) { # was '/\\.(o|obj)$'\n            # push(@objfiles, $part); # do nothing\n        } else {\n\nAnd correct the substitution to only operate on .o files.\n\nAlso report all errors rather than dieing on the first\n\n $ git describe 4b623d80f7352\n v1.9.1-1-g4b623d8\nThe problem exists for V1.9.2 onward\n\nSigned-off-by: Philip Oakley <philipoakley@iee.org>\n---\n contrib/buildsystems/engine.pl | 15 ++++++++++-----\n 1 file changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl\nindex 8e41808..16c3dd5 100755\n--- a/contrib/buildsystems/engine.pl\n+++ b/contrib/buildsystems/engine.pl\n@@ -264,7 +264,9 @@ sub handleCompileLine\n         } elsif ($part =~ /\\.(c|cc|cpp)$/) {\n             $sourcefile = $part;\n         } else {\n-            die \"Unhandled compiler option @ line $lineno: $part\";\n+            print \"full line: $line\\n\";\n+            print \"A:-Unhandled compiler option @ line $lineno: $part\\n\"; # die (if !DEBUG)\n+#            die \"Unhandled compiler option @ line $lineno: $part\";\n         }\n     }\n     @{$compile_options{\"${sourcefile}_CFLAGS\"}} = @cflags;\n@@ -290,14 +292,15 @@ sub handleLibLine\n             $libout = $part;\n             $libout =~ s/\\.a$//;\n         } else {\n-            die \"Unhandled lib option @ line $lineno: $part\";\n+            print \"A:-Unhandled lib option @ line $lineno: $part\\n\"; # die (if !DEBUG)\n+#           die \"Unhandled lib option @ line $lineno: $part\";\n         }\n     }\n #    print \"LibOut: '$libout'\\nLFlags: @lflags\\nOfiles: @objfiles\\n\";\n #    exit(1);\n     foreach (@objfiles) {\n         my $sourcefile = $_;\n-        $sourcefile =~ s/\\.o/.c/;\n+        $sourcefile =~ s/\\.o$/.c/;\n         push(@sources, $sourcefile);\n         push(@cflags, @{$compile_options{\"${sourcefile}_CFLAGS\"}});\n         push(@defines, @{$compile_options{\"${sourcefile}_DEFINES\"}});\n@@ -343,8 +346,10 @@ sub handleLinkLine\n         } elsif ($part =~ /\\.(a|lib)$/) {\n             $part =~ s/\\.a$/.lib/;\n             push(@libs, $part);\n-        } elsif ($part =~ /\\.(o|obj)$/) {\n+        } elsif ($part =~ /\\.o$/) { # was '/\\.(o|obj)$'\n             push(@objfiles, $part);\n+        } elsif ($part =~ /\\.obj$/) { # was '/\\.(o|obj)$'\n+            # push(@objfiles, $part); # do nothing\n         } else {\n             die \"Unhandled lib option @ line $lineno: $part\";\n         }\n@@ -353,7 +358,7 @@ sub handleLinkLine\n #    exit(1);\n     foreach (@objfiles) {\n         my $sourcefile = $_;\n-        $sourcefile =~ s/\\.o/.c/;\n+        $sourcefile =~ s/\\.o$/.c/;\n         push(@sources, $sourcefile);\n         push(@cflags, @{$compile_options{\"${sourcefile}_CFLAGS\"}});\n         push(@defines, @{$compile_options{\"${sourcefile}_DEFINES\"}});\n-- \n1.9.4.msysgit.0\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"Git for Windows\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"252314","messageId":"1416526682-6024-5-git-send-email-philipoakley@iee.org","threadId":"38019","inReplyTo":"1416526682-6024-1-git-send-email-philipoakley@iee.org","subject":"[RFC 4/4] Improve layout and reference msvc-build script","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2014-11-20T23:38:02Z","receivedAt":"2014-11-20T23:38:02Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Layout the 'either/or' with more white space to clarify\nwhich alternatives are matched up.\n\nReference the build script which automates one sequence of options.\n\nSigned-off-by: Philip Oakley <philipoakley@iee.org>\n---\n compat/vcbuild/README | 29 +++++++++++++++++++++--------\n 1 file changed, 21 insertions(+), 8 deletions(-)\n\ndiff --git a/compat/vcbuild/README b/compat/vcbuild/README\nindex df8a657..f4f6723 100644\n--- a/compat/vcbuild/README\n+++ b/compat/vcbuild/README\n@@ -3,20 +3,24 @@ The Steps of Build Git with VS2008\n 1. You need the build environment, which contains the Git dependencies\n    to be able to compile, link and run Git with MSVC.\n \n-   You can either use the binary repository:\n+   You can either:\n+      use the binary repository:\n \n        WWW: http://repo.or.cz/w/msvcgit.git\n        Git: git clone git://repo.or.cz/msvcgit.git\n        Zip: http://repo.or.cz/w/msvcgit.git?a=snapshot;h=master;sf=zip\n \n-   and call the setup_32bit_env.cmd batch script before compiling Git,\n-   (see repo/package README for details), or the source repository:\n+      and call the setup_32bit_env.cmd batch script before compiling Git,\n+     (see repo/package README for details),\n+\n+   or:\n+      use the source repository:\n \n        WWW: http://repo.or.cz/w/gitbuild.git\n        Git: git clone git://repo.or.cz/gitbuild.git\n        Zip: (None, as it's a project with submodules)\n \n-   and build the support libs as instructed in that repo/package.\n+     and build the support libs as instructed in that repo/package.\n \n 2. Ensure you have the msysgit environment in your path, so you have\n    GNU Make, bash and perl available.\n@@ -33,18 +37,27 @@ The Steps of Build Git with VS2008\n        make common-cmds.h\n    to generate the common-cmds.h file needed to compile git.\n \n-4. Then either build Git with the GNU Make Makefile in the Git projects\n-   root\n+4. Then either\n+\n+     build Git with the GNU Make Makefile in the Git projects root\n        make MSVC=1\n-   or generate Visual Studio solution/projects (.sln/.vcproj) with the\n+   or\n+\n+   generate Visual Studio solution/projects (.sln/.vcproj) with the\n    command\n        perl contrib/buildsystems/generate -g Vcproj\n    and open and build the solution with the IDE\n        devenv git.sln /useenv\n-   or build with the IDE build engine directly from the command line\n+     or\n+\n+   build with the IDE build engine directly from the command line\n        devenv git.sln /useenv /build \"Release|Win32\"\n    The /useenv option is required, so Visual Studio picks up the\n    environment variables for the support libraries required to build\n    Git, which you set up in step 1.\n \n Done!\n+\n+Or, use the msvc-build script; also in /msysgit/bin/msvc-build\n+\n+For those with VS2010: You may need to associate the generated .sln file with VS2010 rather than the MS version selector program.\n-- \n1.9.4.msysgit.0\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"Git for Windows\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"252327","messageId":"alpine.DEB.1.00.1411211032060.13845@s15462909.onlinehome-server.info","threadId":"38019","inReplyTo":"1416526682-6024-1-git-send-email-philipoakley@iee.org","subject":"Re: [RFC 0/4] Fix the Visual Studio 2008 .sln generator","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2014-11-21T09:38:06Z","receivedAt":"2014-11-21T09:38:06Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Philip,\n\nOn Thu, 20 Nov 2014, Philip Oakley wrote:\n\n> Are the patches going in the right direction?\n\nYes.\n\nA couple of general comments:\n\n- please do not comment out code. Just remove it.\n\n- the first three commit messages look funny, being indented by 4\n  spaces... unintentional?\n\n> Is the processing of the .obj file in engine.pl sensible?\n\nYes, but instead of adding dead code, it would be better to use the\ncomment \"# ignore\". I would also strongly suggest to hard-code the\ncomplete file name instead of just the extension. We know exactly which\nfile name we want to ignore, and if there should be another .obj file\neventually, it would be wrong to ignore it, too.\n\n> and the extra care with s/\\.o$/.c/ avoiding s/obj/cbj/.\n\nTechnically, you need to use a group \\($\\|[ \\t]\\), but I think that that\nwould be overkill.\n\n> Does it affect the Qmake capability? (I've no idea)\n\nFrankly, neither do I ;-) But since you touched only engine.pl, I would\nexpect Qmake not to be affected at all, right?\n\n> Is the quoting of filenames correct? (my perl foo is cargo cult!)\n\nIANAPME (I am not a Perl Monk either), but it looks good to me.\n\n> I've also updated the vcbuild/README to mention Msysgit (which\n> will be replaced soon by the newer/better Git-for-windows/SDK\n> (https://github.com/git-for-windows/sdk), but the benefits still\n> apply.\n\nThe path you used is /msysgit/bin/msvc-build, but the real path would be\n/bin/msvc-build.\n\n> Obviously, the patches will need squashing together,\n\nTo the contrary, I like the current separation of concerns.\n\nCiao,\nJohannes\n"},{"id":"252328","messageId":"alpine.DEB.1.00.1411211038570.13845@s15462909.onlinehome-server.info","threadId":"38019","inReplyTo":"1416526682-6024-2-git-send-email-philipoakley@iee.org","subject":"Re: [RFC 1/4] Fix i18n -o option in msvc engine.pl","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2014-11-21T09:41:36Z","receivedAt":"2014-11-21T09:41:36Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Philip,\n\nOn Thu, 20 Nov 2014, Philip Oakley wrote:\n\n>     The i18n 5e9637c6 introduced an extra '-o' option\n>     into the make file,\n\nI take it you are referring to\n\n\thttps://github.com/git/git/commit/5e9637c6#diff-b67911656ef5d18c4ae36cb6741b7965R2195\n\n> diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl\n> index 23da787..9144ea7 100755\n> --- a/contrib/buildsystems/engine.pl\n> +++ b/contrib/buildsystems/engine.pl\n> @@ -140,6 +140,18 @@ sub parseMakeOutput\n>              next;\n>          }\n>  \n> +        if ($text =~ /^mkdir /) {\n> +            # options to the Portable Object translations in the line\n> +            # mkdir -p po/... && msgfmt ... (eg -o) may be mistaken for linker options\n\nMaybe better\n\n\t\t# the line \"mkdir ... && msgfmt ...\" contains no linker options\n\n> +            next;\n> +        }\n> +\n> +        if ($text =~ /^msgfmt /) {\n> +            # options to the Portable Object translations in the line\n> +            # mkdir -p po/... && msgfmt ... (eg -o) may be mistaken for linker options\n> +            next;\n> +        }\n\nThese two if clauses do the same, maybe call it\n\n\tif ($test =~ /^(mkdir|msgfmt) /)\n\nCiao,\nJohannes\n"},{"id":"252329","messageId":"alpine.DEB.1.00.1411211041560.13845@s15462909.onlinehome-server.info","threadId":"38019","inReplyTo":"1416526682-6024-3-git-send-email-philipoakley@iee.org","subject":"Re: [RFC 2/4] Properly accept quoted space in filenames","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2014-11-21T09:42:41Z","receivedAt":"2014-11-21T09:42:41Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Philip,\n\napart from adding dead code:\n\nOn Thu, 20 Nov 2014, Philip Oakley wrote:\n\n> diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl\n> index 9144ea7..8e41808 100755\n> --- a/contrib/buildsystems/engine.pl\n> +++ b/contrib/buildsystems/engine.pl\n> @@ -243,7 +244,8 @@ sub removeDuplicates\n>  sub handleCompileLine\n>  {\n>      my ($line, $lineno) = @_;\n> -    my @parts = split(' ', $line);\n> +    # my @parts = split(' ', $line);\n> +    my @parts = quotewords('\\s+', 0, $line);\n>      my $sourcefile;\n\n(i.e. the commented-out line) it looks good to me.\n\nCiao,\nJohannes\n"},{"id":"252330","messageId":"alpine.DEB.1.00.1411211043050.13845@s15462909.onlinehome-server.info","threadId":"38019","inReplyTo":"1416526682-6024-4-git-send-email-philipoakley@iee.org","subject":"Re: [RFC 3/4] engine.pl: split the .o and .obj processing","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2014-11-21T09:48:27Z","receivedAt":"2014-11-21T09:48:27Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Philip,\n\nOn Thu, 20 Nov 2014, Philip Oakley wrote:\n\n> Commit 4b623d80f7352 added an .obj file (invalidcontinue.obj) which was not\n> processed correctly.\n> \n> The generate engine then mistakenly did a s/.o/.c/  to create a request\n> to compile invalidcontinue.cbj.\n\nThis is good. However, this:\n\n> Split the '/\\.(o|obj)$' in engine.pl#L353 into:\n> \n>         } elsif ($part =~ /\\.o$/) { # was '/\\.(o|obj)$'\n>             push(@objfiles, $part);\n>         } elsif ($part =~ /\\.obj$/) { # was '/\\.(o|obj)$'\n>             # push(@objfiles, $part); # do nothing\n>         } else {\n\njust repeats what the diff says, so it is unnecessary in the commit\nmessage.\n\n> diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl\n> index 8e41808..16c3dd5 100755\n> --- a/contrib/buildsystems/engine.pl\n> +++ b/contrib/buildsystems/engine.pl\n> @@ -264,7 +264,9 @@ sub handleCompileLine\n>          } elsif ($part =~ /\\.(c|cc|cpp)$/) {\n>              $sourcefile = $part;\n>          } else {\n> -            die \"Unhandled compiler option @ line $lineno: $part\";\n> +            print \"full line: $line\\n\";\n> +            print \"A:-Unhandled compiler option @ line $lineno: $part\\n\"; # die (if !DEBUG)\n> +#            die \"Unhandled compiler option @ line $lineno: $part\";\n\nThis needs to be backed out. I agree that it is nice to get going and to\ndebug, so I would split it out into its own commit while working on the\nbranch, but then drop it before submitting.\n\n> @@ -290,14 +292,15 @@ sub handleLibLine\n>[...]\n>      foreach (@objfiles) {\n>          my $sourcefile = $_;\n> -        $sourcefile =~ s/\\.o/.c/;\n> +        $sourcefile =~ s/\\.o$/.c/;\n\nAh, I see from the context that my earlier comment about the white-space\ndelimiter was wrong: at this stage, we already have split the list. So\nthis is groovy.\n\n> @@ -343,8 +346,10 @@ sub handleLinkLine\n>          } elsif ($part =~ /\\.(a|lib)$/) {\n>              $part =~ s/\\.a$/.lib/;\n>              push(@libs, $part);\n> -        } elsif ($part =~ /\\.(o|obj)$/) {\n> +        } elsif ($part =~ /\\.o$/) { # was '/\\.(o|obj)$'\n>              push(@objfiles, $part);\n> +        } elsif ($part =~ /\\.obj$/) { # was '/\\.(o|obj)$'\n> +            # push(@objfiles, $part); # do nothing\n\nHow about the following instead?\n\n+\t\t} elsif ($part eq 'invalidcontinue.obj') {\n+\t\t\t# ignore\n \t\t} elsif ($part =~ /\\.(o|obj)$/) {\n\n? After all, this change is really only about handling the newly\nintroduced invalidcontinue.obj correctly.\n\nCiao,\nJohannes\n"},{"id":"252331","messageId":"alpine.DEB.1.00.1411211049100.13845@s15462909.onlinehome-server.info","threadId":"38019","inReplyTo":"1416526682-6024-5-git-send-email-philipoakley@iee.org","subject":"Re: [RFC 4/4] Improve layout and reference msvc-build script","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2014-11-21T09:51:36Z","receivedAt":"2014-11-21T09:51:36Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Philip,\n\nOn Thu, 20 Nov 2014, Philip Oakley wrote:\n\n> [...]\n> +Or, use the msvc-build script; also in /msysgit/bin/msvc-build\n\nAs I mentioned before, from a Git Bash on Windows, the path is\n/bin/msvc-build (no /msysgit/). That is quite likely to stay the same with\nthe upcoming Git for Windows SDK, too (once I add the file to the SDK).\n\nBy the way, I think we should also start thinking about a Jenkins job to\nverify that upstream changes such as invalidcontinue.obj do not break the\nMSVC build. Please let me know if you want to give it a try, I have a\nWindows Azure instance with a Jenkins instance, sponsored by Microsoft.\n\nCiao,\nJohannes\n"},{"id":"252363","messageId":"7E90FA1615514FA1B47B268DCBCA9534@PhilipOakley","threadId":"38019","inReplyTo":"alpine.DEB.1.00.1411211032060.13845@s15462909.onlinehome-server.info","subject":"Re: [RFC 0/4] Fix the Visual Studio 2008 .sln generator","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2014-11-21T19:57:22Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Johannes Schindelin\" <Johannes.Schindelin@gmx.de>\n> Hi Philip,\n>\n> On Thu, 20 Nov 2014, Philip Oakley wrote:\n>\n>> Are the patches going in the right direction?\n>\n> Yes.\nMagic. Glad of the confirmation\n>\n> A couple of general comments:\n>\n> - please do not comment out code. Just remove it.\nIt's my debugging style until I get it working ;-0\n>\n> - the first three commit messages look funny, being indented by 4\n>  spaces... unintentional?\nI think that was from one of the tools I was using at the time. They'll \nbe tidied and reflowed for the final version(s).\n>\n>> Is the processing of the .obj file in engine.pl sensible?\n>\n> Yes, but instead of adding dead code, it would be better to use the\n> comment \"# ignore\". I would also strongly suggest to hard-code the\n> complete file name instead of just the extension.\nTrue, I realised that after posting (isnt it always the way).\n\n> We know exactly which\n> file name we want to ignore, and if there should be another .obj file\n> eventually, it would be wrong to ignore it, too.\n\nMy concern was more about why a 'make' would produce .obj files (rather \nthan .o files) in the first place. As I understand it we should never \nsee .obj files generated from make with the exception of these ones that \nare library files implicitly known to MSVC - any thoughts?\n\n>\n>> and the extra care with s/\\.o$/.c/ avoiding s/obj/cbj/.\n>\n> Technically, you need to use a group \\($\\|[ \\t]\\), but I think that \n> that\n> would be overkill.\n>\n>> Does it affect the Qmake capability? (I've no idea)\n>\n> Frankly, neither do I ;-) But since you touched only engine.pl, I \n> would\n> expect Qmake not to be affected at all, right?\n\nAs I understood the call sequence merry go round, the engine can pull in \neither function depending on command line options, hence the open \nquestion. But like you say, why expect it to be affected, and no-one has \nmentioned on list for years;-)\n\n>\n>> Is the quoting of filenames correct? (my perl foo is cargo cult!)\n>\n> IANAPME (I am not a Perl Monk either), but it looks good to me.\nOK.\n>\n>> I've also updated the vcbuild/README to mention Msysgit (which\n>> will be replaced soon by the newer/better Git-for-windows/SDK\n>> (https://github.com/git-for-windows/sdk), but the benefits still\n>> apply.\n>\n> The path you used is /msysgit/bin/msvc-build, but the real path would \n> be\n> /bin/msvc-build.\n>\n>> Obviously, the patches will need squashing together,\n>\n> To the contrary, I like the current separation of concerns.\nAt the moment the first patch doesn't fully cure the .sln build, but \nthere's no test anyway so bisecting would be problematic.\n\nI'm happy to keep them small separate and clean ;-)\n\n>\n> Ciao,\n> Johannes\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n> \n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"Git for Windows\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"252364","messageId":"A86C2AE45DB643018C05D8C766FB04F3@PhilipOakley","threadId":"38019","inReplyTo":"alpine.DEB.1.00.1411211038570.13845@s15462909.onlinehome-server.info","subject":"Re: [RFC 1/4] Fix i18n -o option in msvc engine.pl","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2014-11-21T19:57:22Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Johannes Schindelin\" <Johannes.Schindelin@gmx.de>\n> Hi Philip,\n>\n> On Thu, 20 Nov 2014, Philip Oakley wrote:\n>\n>>     The i18n 5e9637c6 introduced an extra '-o' option\n>>     into the make file,\n>\n> I take it you are referring to\n>\n> https://github.com/git/git/commit/5e9637c6#diff-b67911656ef5d18c4ae36cb6741b7965R2195\n\nYes, Makefile#L2195 (of the new version)\n>\n>> diff --git a/contrib/buildsystems/engine.pl \n>> b/contrib/buildsystems/engine.pl\n>> index 23da787..9144ea7 100755\n>> --- a/contrib/buildsystems/engine.pl\n>> +++ b/contrib/buildsystems/engine.pl\n>> @@ -140,6 +140,18 @@ sub parseMakeOutput\n>>              next;\n>>          }\n>>\n>> +        if ($text =~ /^mkdir /) {\n>> +            # options to the Portable Object translations in the \n>> line\n>> +            # mkdir -p po/... && msgfmt ... (eg -o) may be mistaken \n>> for linker options\n>\n> Maybe better\n>\n> # the line \"mkdir ... && msgfmt ...\" contains no linker options\n\nOK will use.\n>\n>> +            next;\n>> +        }\n>> +\n>> +        if ($text =~ /^msgfmt /) {\n>> +            # options to the Portable Object translations in the \n>> line\n>> +            # mkdir -p po/... && msgfmt ... (eg -o) may be mistaken \n>> for linker options\n>> +            next;\n>> +        }\n>\n> These two if clauses do the same, maybe call it\n>\n> if ($test =~ /^(mkdir|msgfmt) /)\n\nThat's good, will use.\n\n>\n> Ciao,\n> Johannes\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n> \n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"Git for Windows\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"252365","messageId":"622F65684C63407184D58ED19385AFD4@PhilipOakley","threadId":"38019","inReplyTo":"alpine.DEB.1.00.1411211043050.13845@s15462909.onlinehome-server.info","subject":"Re: [RFC 3/4] engine.pl: split the .o and .obj processing","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2014-11-21T19:57:22Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Johannes Schindelin\" <Johannes.Schindelin@gmx.de>\n> Hi Philip,\n>\n> On Thu, 20 Nov 2014, Philip Oakley wrote:\n>\n>> Commit 4b623d80f7352 added an .obj file (invalidcontinue.obj) which \n>> was not\n>> processed correctly.\n>>\n>> The generate engine then mistakenly did a s/.o/.c/  to create a \n>> request\n>> to compile invalidcontinue.cbj.\n>\n> This is good. However, this:\n>\n>> Split the '/\\.(o|obj)$' in engine.pl#L353 into:\n>>\n>>         } elsif ($part =~ /\\.o$/) { # was '/\\.(o|obj)$'\n>>             push(@objfiles, $part);\n>>         } elsif ($part =~ /\\.obj$/) { # was '/\\.(o|obj)$'\n>>             # push(@objfiles, $part); # do nothing\n>>         } else {\n>\n> just repeats what the diff says, so it is unnecessary in the commit\n> message.\nOK.\n\n>\n>> diff --git a/contrib/buildsystems/engine.pl \n>> b/contrib/buildsystems/engine.pl\n>> index 8e41808..16c3dd5 100755\n>> --- a/contrib/buildsystems/engine.pl\n>> +++ b/contrib/buildsystems/engine.pl\n>> @@ -264,7 +264,9 @@ sub handleCompileLine\n>>          } elsif ($part =~ /\\.(c|cc|cpp)$/) {\n>>              $sourcefile = $part;\n>>          } else {\n>> -            die \"Unhandled compiler option @ line $lineno: $part\";\n>> +            print \"full line: $line\\n\";\n>> +            print \"A:-Unhandled compiler option @ line $lineno: \n>> $part\\n\"; # die (if !DEBUG)\n>> +#            die \"Unhandled compiler option @ line $lineno: $part\";\n>\n> This needs to be backed out. I agree that it is nice to get going and \n> to\n> debug, so I would split it out into its own commit while working on \n> the\n> branch, but then drop it before submitting.\n\nI'll probably split this \"improvement in error reporting\" (by providing \nthe full offending line) into a separate patch, on the basis that we \nwant (Windows) folks to start helping, so let's make the error messages \nmore useful to those who don't know these scripting languages yet.\n\n>\n>> @@ -290,14 +292,15 @@ sub handleLibLine\n>>[...]\n>>      foreach (@objfiles) {\n>>          my $sourcefile = $_;\n>> -        $sourcefile =~ s/\\.o/.c/;\n>> +        $sourcefile =~ s/\\.o$/.c/;\n>\n> Ah, I see from the context that my earlier comment about the \n> white-space\n> delimiter was wrong: at this stage, we already have split the list. So\n> this is groovy.\n>\n>> @@ -343,8 +346,10 @@ sub handleLinkLine\n>>          } elsif ($part =~ /\\.(a|lib)$/) {\n>>              $part =~ s/\\.a$/.lib/;\n>>              push(@libs, $part);\n>> -        } elsif ($part =~ /\\.(o|obj)$/) {\n>> +        } elsif ($part =~ /\\.o$/) { # was '/\\.(o|obj)$'\n>>              push(@objfiles, $part);\n>> +        } elsif ($part =~ /\\.obj$/) { # was '/\\.(o|obj)$'\n>> +            # push(@objfiles, $part); # do nothing\n>\n> How about the following instead?\n>\n> + } elsif ($part eq 'invalidcontinue.obj') {\n> + # ignore\n>  } elsif ($part =~ /\\.(o|obj)$/) {\n\nLooks good, I'll use that (after deciding whether .obj files should be \nexpected in a 'make' output anyway)\n\n>\n> ? After all, this change is really only about handling the newly\n> introduced invalidcontinue.obj correctly.\nDid it ever handle .obj's correctly (rhet) - I dunno.\n\n>\n> Ciao,\n> Johannes\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n> \n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"Git for Windows\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"252366","messageId":"6D7BD7DD7E3A49EE8D7B04BC86F88F74@PhilipOakley","threadId":"38019","inReplyTo":"alpine.DEB.1.00.1411211049100.13845@s15462909.onlinehome-server.info","subject":"Re: [RFC 4/4] Improve layout and reference msvc-build script","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2014-11-21T19:57:22Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Johannes Schindelin\" <Johannes.Schindelin@gmx.de>\n> Hi Philip,\n>\n> On Thu, 20 Nov 2014, Philip Oakley wrote:\n>\n>> [...]\n>> +Or, use the msvc-build script; also in /msysgit/bin/msvc-build\n>\n> As I mentioned before, from a Git Bash on Windows, the path is\n> /bin/msvc-build (no /msysgit/). That is quite likely to stay the same \n> with\n> the upcoming Git for Windows SDK, too (once I add the file to the \n> SDK).\n\nI was trying to identify the source file (as I see it in my Msysgit \ninstall). However it will all need tidying to distinguish what is seen \nif you have G4W-SDK, rather just installing the new G4W (when ready)\n>\n> By the way, I think we should also start thinking about a Jenkins job \n> to\n> verify that upstream changes such as invalidcontinue.obj do not break \n> the\n> MSVC build. Please let me know if you want to give it a try, I have a\n> Windows Azure instance with a Jenkins instance, sponsored by \n> Microsoft.\n\nThis is not something I'm familiar with, but it's something I could have \na look at.\nOn my list is also the 'git bundle' symref problem, which we've got a \nsolution for that 'just' needs coding, along with thoughts about \nnarrow/sparse checkout/clone/fetch.\n\nOne of the confusions I have is the distinction between building with \nthe MSVC compiler (cl command line) and building within VS2008, and \nwhether they are different things (i.e. does VS2008 bring extra \nbaggage?).\n--\nPhilip\n\n\n>\n> Ciao,\n> Johannes\n> \n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"Git for Windows\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"252368","messageId":"xmqqegswyygc.fsf@gitster.dls.corp.google.com","threadId":"38019","inReplyTo":"1416526682-6024-3-git-send-email-philipoakley@iee.org","subject":"Re: [RFC 2/4] Properly accept quoted space in filenames","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-21T22:34:59Z","receivedAt":"2014-11-21T22:34:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley <philipoakley@iee.org> writes:\n\n>  sub handleCompileLine\n>  {\n>      my ($line, $lineno) = @_;\n> -    my @parts = split(' ', $line);\n> +    # my @parts = split(' ', $line);\n> +    my @parts = quotewords('\\s+', 0, $line);\n\nCan somebody enlighten me why/if quotewords is preferrable over\nshellwords in the context of this patch?\n\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"Git for Windows\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"252370","messageId":"9E7656E05CE648B3A357858B4520110D@PhilipOakley","threadId":"38019","inReplyTo":"xmqqegswyygc.fsf@gitster.dls.corp.google.com","subject":"Re: [RFC 2/4] Properly accept quoted space in filenames","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2014-11-21T23:01:26Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Junio C Hamano\" <gitster@pobox.com>\n> Philip Oakley <philipoakley@iee.org> writes:\n>\n>>  sub handleCompileLine\n>>  {\n>>      my ($line, $lineno) = @_;\n>> -    my @parts = split(' ', $line);\n>> +    # my @parts = split(' ', $line);\n>> +    my @parts = quotewords('\\s+', 0, $line);\n>\n> Can somebody enlighten me why/if quotewords is preferrable over\n> shellwords in the context of this patch?\n\n\"No\" - Ignorance is bliss ;-) I think my cargo culting was the result of \nsome googling for \"quoting perl variables\" or some such, which obviously \ncame up with quotewords - I'm happy to take advice on this one!\n\nquotewords did appear to work though back when I wrote this: 86dcfcf \n(Properly accept quoted space in filenames, 2012-05-06)\n--\nPhilip \n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"Git for Windows\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"252372","messageId":"xmqqa93kywaf.fsf@gitster.dls.corp.google.com","threadId":"38019","inReplyTo":"9E7656E05CE648B3A357858B4520110D@PhilipOakley","subject":"Re: [RFC 2/4] Properly accept quoted space in filenames","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-11-21T23:21:44Z","receivedAt":"2014-11-21T23:21:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Philip Oakley\" <philipoakley@iee.org> writes:\n\n> From: \"Junio C Hamano\" <gitster@pobox.com>\n>> Philip Oakley <philipoakley@iee.org> writes:\n>>\n>>>  sub handleCompileLine\n>>>  {\n>>>      my ($line, $lineno) = @_;\n>>> -    my @parts = split(' ', $line);\n>>> +    # my @parts = split(' ', $line);\n>>> +    my @parts = quotewords('\\s+', 0, $line);\n>>\n>> Can somebody enlighten me why/if quotewords is preferrable over\n>> shellwords in the context of this patch?\n>\n> \"No\" - Ignorance is bliss ;-) I think my cargo culting was the result\n> of some googling for \"quoting perl variables\" or some such, which\n> obviously came up with quotewords - I'm happy to take advice on this\n> one!\n>\n> quotewords did appear to work though back when I wrote this: 86dcfcf\n> (Properly accept quoted space in filenames, 2012-05-06)\n\nA quick websearch shows me:\n\n  http://cpansearch.perl.org/src/CHORNY/Text-ParseWords-3.29/ParseWords.pm\n\nand comparing the implementations of the two, the difference boils\ndown to just one line to me.\n\n    sub shellwords {\n        my (@lines) = @_;\n        my @allwords;\n\n        foreach my $line (@lines) {\n            $line =~ s/^\\s+//;\n            my @words = parse_line('\\s+', 0, $line);\n            pop @words if (@words and !defined $words[-1]);\n            return() unless (@words || !length($line));\n            push(@allwords, @words);\n        }\n        return(@allwords);\n    }\n\nIn quotewords, the call to parse_line uses $keep not hardcoded 0\n(which would not make any difference in the context of your patch),\nand it assumes parse_line() never returns a singleton \"undef\" so the\nline \"pop ... if @words is a list with 'undef' as its sole element\"\nis missing.\n\nOf course, the caller would become smaller and sweeter, i.e.\n\n    my @parts = shellwords($line);\n\nI am not familiar with if Perl folks have certain convention to\ndecide when to use which one, though ;-)\n\n\n\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"Git for Windows\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"},{"id":"252392","messageId":"alpine.DEB.1.00.1411231624230.13845@s15462909.onlinehome-server.info","threadId":"38019","inReplyTo":"622F65684C63407184D58ED19385AFD4@PhilipOakley","subject":"Re: [RFC 3/4] engine.pl: split the .o and .obj processing","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2014-11-23T15:28:48Z","receivedAt":"2014-11-23T15:28:48Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 21 Nov 2014, Philip Oakley wrote:\n\n> From: \"Johannes Schindelin\" <Johannes.Schindelin@gmx.de>\n>\n> > On Thu, 20 Nov 2014, Philip Oakley wrote:\n> >\n> > > @@ -343,8 +346,10 @@ sub handleLinkLine\n> > >          } elsif ($part =~ /\\.(a|lib)$/) {\n> > >              $part =~ s/\\.a$/.lib/;\n> > >              push(@libs, $part);\n> > > -        } elsif ($part =~ /\\.(o|obj)$/) {\n> > > +        } elsif ($part =~ /\\.o$/) { # was '/\\.(o|obj)$'\n> > >              push(@objfiles, $part);\n> > > +        } elsif ($part =~ /\\.obj$/) { # was '/\\.(o|obj)$'\n> > > +            # push(@objfiles, $part); # do nothing\n> >\n> > How about the following instead?\n> >\n> > + } elsif ($part eq 'invalidcontinue.obj') {\n> > + # ignore\n> >  } elsif ($part =~ /\\.(o|obj)$/) {\n> \n> Looks good, I'll use that (after deciding whether .obj files should be\n> expected in a 'make' output anyway)\n\nMy idea to hardcode invalidcontinue.obj was that I'd rather see a failure\nif an unexpected .obj is seen there. But I realize that my suggested\nchange does not exactly accomplish that.\n\nCiao,\nJohannes\n"},{"id":"252410","messageId":"0DE9992936544B43BD4B7FC2CC4388AD@PhilipOakley","threadId":"38019","inReplyTo":"alpine.DEB.1.00.1411231624230.13845@s15462909.onlinehome-server.info","subject":"Re: Re: [RFC 3/4] engine.pl: split the .o and .obj processing","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2014-11-23T21:58:48Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Johannes Schindelin\" <Johannes.Schindelin@gmx.de>\n>> > How about the following instead?\n>> >\n>> > + } elsif ($part eq 'invalidcontinue.obj') {\n>> > + # ignore\n>> >  } elsif ($part =~ /\\.(o|obj)$/) {\n>>\n>> Looks good, I'll use that (after deciding whether .obj files should\n>> be\n>> expected in a 'make' output anyway)\n>\n> My idea to hardcode invalidcontinue.obj was that I'd rather see a\n> failure\n> if an unexpected .obj is seen there. But I realize that my suggested\n> change does not exactly accomplish that.\n>\nI've decided to include the hard code suggested, and after a few tests\ndecided it was probably worth keeping the .obj processing, even though\nwe don't expect any.\n\nAt least I've now managed to generate a .sln project, though it won't\ncompile because of a number of other changes, and I haven't got to the\nbottom of them all.\n\nNotes:\nRevert \"Windows: correct detection of EISDIR in mingw_open()\"\ngit\\compat\\mingw.c(315) : error C2065: 'O_ACCMODE' : undeclared\nidentifier (neither VS2008 nor VS2010 declare 'O_ACCMODE'). add an \n#ifdef\n_MSC_VER to define this if we are in MSVC.\n\nRevert \"mingw.h: add dummy functions for sigset_t operations\" commit\n4e6d207c45. the Additional Note: to\nhttp://www.spinics.net/lists/git/msg240248.html indicates that this\nbreaks MSVC. I think it will also need an #ifdef to see if we are in \nMSVC.\n\nI've also found a problem with my setup (msysgit 1.7.9 for make) barfing\non the commit/ee9be06 \"perl: detect new files in MakeMaker builds\" which\n'make -n' says 'No rule to make target `PM.stamp', needed by `perl.mak'.\n\nThere was also LINK : warning LNK4098: defaultlib 'MSVCRT' conflicts\nwith use of other libs; use /NODEFAULTLIB:library.\n\nI've also got notes from way back that there was build order problem\nwith 'vcs-svn_lib.lib' - probably a sort order issue. IIRC at that time,\nI had to compile the project twice so that the lib was visible the\nsecond time.\n\nSo there's plenty to go at ;-) I'm going to be away this coming week, So\nat least I have the whole next cycle to make some progress.\n\n--\n\nPhilip\n\n-- \n-- \n*** Please reply-to-all at all times ***\n*** (do not pretend to know who is subscribed and who is not) ***\n*** Please avoid top-posting. ***\nThe msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.\n\nYou received this message because you are subscribed to the Google\nGroups \"msysGit\" group.\nTo post to this group, send email to msysgit@googlegroups.com\nTo unsubscribe from this group, send email to\nmsysgit+unsubscribe@googlegroups.com\nFor more options, and view previous threads, visit this group at\nhttp://groups.google.com/group/msysgit?hl=en_US?hl=en\n\n--- \nYou received this message because you are subscribed to the Google Groups \"Git for Windows\" group.\nTo unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.\nFor more options, visit https://groups.google.com/d/optout.\n"}]}