[RFC 0/4] Fix the Visual Studio 2008 .sln generator

[RFC 0/4] Fix the Visual Studio 2008 .sln generator

[Philip Oakley]

Potential Windows developers are likely to be using Visual Studio as
their IDE. The tool stack required for Windows can be tortuous as it
crosses the boundaries between platforms and philosophies. This RFC
seeks to maintain the tools that could assist such developers. In
particular, those tools that generate an initial Visual Studio project
(.sln ) file.

The .sln generator in contrib began to break when internationalisation
introduced an extra -o option. This recently worsened with the addition
of invalidcontinue.obj for 'improved POSIX compatibility'.

I hacked a bit when I first attempted to use the VS IDE and noticed the
i18n issue. I didn't completely solve all my issues because of further
issues with VS2010 Express, so no patches were submitted at the time.

Now, with a fresh copy of VS20008 Express, I see the additional problem
of the addition of the invalidcontinue.obj reference causing the .sln
generation to fail.

Are the patches going in the right direction?
Is the processing of the .obj file in engine.pl sensible?
and the extra care with s/\.o$/.c/ avoiding s/obj/cbj/.
Does it affect the Qmake capability? (I've no idea)
Is the quoting of filenames correct? (my perl foo is cargo cult!)

I've also updated the vcbuild/README to mention Msysgit (which
will be replaced soon by the newer/better Git-for-windows/SDK
(https://github.com/git-for-windows/sdk), but the benefits still
apply.

I've cc'd those who have contributed or patched the engine.pl, or
appear to be interested via a $gmane search, who can hopefully comment.

Obviously, the patches will need squashing together, and the commit
message(s) tidied after inclusion of comments.

Philip Oakley (4):
  Fix i18n -o option in msvc engine.pl
  Properly accept quoted space in filenames
  engine.pl: split the .o and .obj processing
  Improve layout and reference msvc-build script

 compat/vcbuild/README          | 29 +++++++++++++++++++++--------
 contrib/buildsystems/engine.pl | 38 ++++++++++++++++++++++++++++++--------
 2 files changed, 51 insertions(+), 16 deletions(-)

-- 
1.9.4.msysgit.0

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[RFC 1/4] Fix i18n -o option in msvc engine.pl

[Philip Oakley]

    The i18n 5e9637c6 introduced an extra '-o' option
    into the make file, which broke engine.pl code for
    extracting the git.sln for msvc gui-IDE.
    add tests for 'msgfmt' and its precursor 'mkdir'
    (in same vein as 74cf9bdda6).

    Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
 contrib/buildsystems/engine.pl | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl
index 23da787..9144ea7 100755
--- a/contrib/buildsystems/engine.pl
+++ b/contrib/buildsystems/engine.pl
@@ -140,6 +140,18 @@ sub parseMakeOutput
             next;
         }
 
+        if ($text =~ /^mkdir /) {
+            # options to the Portable Object translations in the line
+            # mkdir -p po/... && msgfmt ... (eg -o) may be mistaken for linker options
+            next;
+        }
+
+        if ($text =~ /^msgfmt /) {
+            # options to the Portable Object translations in the line
+            # mkdir -p po/... && msgfmt ... (eg -o) may be mistaken for linker options
+            next;
+        }
+
         if($text =~ / -c /) {
             # compilation
             handleCompileLine($text, $line);
-- 
1.9.4.msysgit.0

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[RFC 2/4] Properly accept quoted space in filenames

[Philip Oakley]

    the engine.pl script barfs on the properly quoted space
    in filename options prevalent on Windows. Use quotewords()
    rather than split() to separate such options.

    Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
 contrib/buildsystems/engine.pl | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl
index 9144ea7..8e41808 100755
--- a/contrib/buildsystems/engine.pl
+++ b/contrib/buildsystems/engine.pl
@@ -12,6 +12,7 @@ use File::Basename;
 use File::Spec;
 use Cwd;
 use Generators;
+use Text::ParseWords;
 
 my (%build_structure, %compile_options, @makedry);
 my $out_dir = getcwd();
@@ -243,7 +244,8 @@ sub removeDuplicates
 sub handleCompileLine
 {
     my ($line, $lineno) = @_;
-    my @parts = split(' ', $line);
+    # my @parts = split(' ', $line);
+    my @parts = quotewords('\s+', 0, $line);
     my $sourcefile;
     shift(@parts); # ignore cmd
     while (my $part = shift @parts) {
@@ -277,7 +279,8 @@ sub handleLibLine
     my (@objfiles, @lflags, $libout, $part);
     # kill cmd and rm 'prefix'
     $line =~ s/^rm -f .* && .* rcs //;
-    my @parts = split(' ', $line);
+    # my @parts = split(' ', $line);
+    my @parts = quotewords('\s+', 0, $line);
     while ($part = shift @parts) {
         if ($part =~ /^-/) {
             push(@lflags, $part);
@@ -318,7 +321,9 @@ sub handleLinkLine
 {
     my ($line, $lineno) = @_;
     my (@objfiles, @lflags, @libs, $appout, $part);
-    my @parts = split(' ', $line);
+    # my @parts = split(' ', $line);
+    my @parts = quotewords('\s+', 0, $line);
+
     shift(@parts); # ignore cmd
     while ($part = shift @parts) {
         if ($part =~ /^-IGNORE/) {
-- 
1.9.4.msysgit.0

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[RFC 3/4] engine.pl: split the .o and .obj processing

[Philip Oakley]

Commit 4b623d80f7352 added an .obj file (invalidcontinue.obj) which was not
processed correctly.

The generate engine then mistakenly did a s/.o/.c/  to create a request
to compile invalidcontinue.cbj.

Split the '/\.(o|obj)$' in engine.pl#L353 into:

        } elsif ($part =~ /\.o$/) { # was '/\.(o|obj)$'
            push(@objfiles, $part);
        } elsif ($part =~ /\.obj$/) { # was '/\.(o|obj)$'
            # push(@objfiles, $part); # do nothing
        } else {

And correct the substitution to only operate on .o files.

Also report all errors rather than dieing on the first

 $ git describe 4b623d80f7352
 v1.9.1-1-g4b623d8
The problem exists for V1.9.2 onward

Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
 contrib/buildsystems/engine.pl | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl
index 8e41808..16c3dd5 100755
--- a/contrib/buildsystems/engine.pl
+++ b/contrib/buildsystems/engine.pl
@@ -264,7 +264,9 @@ sub handleCompileLine
         } elsif ($part =~ /\.(c|cc|cpp)$/) {
             $sourcefile = $part;
         } else {
-            die "Unhandled compiler option @ line $lineno: $part";
+            print "full line: $line\n";
+            print "A:-Unhandled compiler option @ line $lineno: $part\n"; # die (if !DEBUG)
+#            die "Unhandled compiler option @ line $lineno: $part";
         }
     }
     @{$compile_options{"${sourcefile}_CFLAGS"}} = @cflags;
@@ -290,14 +292,15 @@ sub handleLibLine
             $libout = $part;
             $libout =~ s/\.a$//;
         } else {
-            die "Unhandled lib option @ line $lineno: $part";
+            print "A:-Unhandled lib option @ line $lineno: $part\n"; # die (if !DEBUG)
+#           die "Unhandled lib option @ line $lineno: $part";
         }
     }
 #    print "LibOut: '$libout'\nLFlags: @lflags\nOfiles: @objfiles\n";
 #    exit(1);
     foreach (@objfiles) {
         my $sourcefile = $_;
-        $sourcefile =~ s/\.o/.c/;
+        $sourcefile =~ s/\.o$/.c/;
         push(@sources, $sourcefile);
         push(@cflags, @{$compile_options{"${sourcefile}_CFLAGS"}});
         push(@defines, @{$compile_options{"${sourcefile}_DEFINES"}});
@@ -343,8 +346,10 @@ sub handleLinkLine
         } elsif ($part =~ /\.(a|lib)$/) {
             $part =~ s/\.a$/.lib/;
             push(@libs, $part);
-        } elsif ($part =~ /\.(o|obj)$/) {
+        } elsif ($part =~ /\.o$/) { # was '/\.(o|obj)$'
             push(@objfiles, $part);
+        } elsif ($part =~ /\.obj$/) { # was '/\.(o|obj)$'
+            # push(@objfiles, $part); # do nothing
         } else {
             die "Unhandled lib option @ line $lineno: $part";
         }
@@ -353,7 +358,7 @@ sub handleLinkLine
 #    exit(1);
     foreach (@objfiles) {
         my $sourcefile = $_;
-        $sourcefile =~ s/\.o/.c/;
+        $sourcefile =~ s/\.o$/.c/;
         push(@sources, $sourcefile);
         push(@cflags, @{$compile_options{"${sourcefile}_CFLAGS"}});
         push(@defines, @{$compile_options{"${sourcefile}_DEFINES"}});
-- 
1.9.4.msysgit.0

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[RFC 4/4] Improve layout and reference msvc-build script

[Philip Oakley]

Layout the 'either/or' with more white space to clarify
which alternatives are matched up.

Reference the build script which automates one sequence of options.

Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
 compat/vcbuild/README | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/compat/vcbuild/README b/compat/vcbuild/README
index df8a657..f4f6723 100644
--- a/compat/vcbuild/README
+++ b/compat/vcbuild/README
@@ -3,20 +3,24 @@ The Steps of Build Git with VS2008
 1. You need the build environment, which contains the Git dependencies
    to be able to compile, link and run Git with MSVC.
 
-   You can either use the binary repository:
+   You can either:
+      use the binary repository:
 
        WWW: http://repo.or.cz/w/msvcgit.git
        Git: git clone git://repo.or.cz/msvcgit.git
        Zip: http://repo.or.cz/w/msvcgit.git?a=snapshot;h=master;sf=zip
 
-   and call the setup_32bit_env.cmd batch script before compiling Git,
-   (see repo/package README for details), or the source repository:
+      and call the setup_32bit_env.cmd batch script before compiling Git,
+     (see repo/package README for details),
+
+   or:
+      use the source repository:
 
        WWW: http://repo.or.cz/w/gitbuild.git
        Git: git clone git://repo.or.cz/gitbuild.git
        Zip: (None, as it's a project with submodules)
 
-   and build the support libs as instructed in that repo/package.
+     and build the support libs as instructed in that repo/package.
 
 2. Ensure you have the msysgit environment in your path, so you have
    GNU Make, bash and perl available.
@@ -33,18 +37,27 @@ The Steps of Build Git with VS2008
        make common-cmds.h
    to generate the common-cmds.h file needed to compile git.
 
-4. Then either build Git with the GNU Make Makefile in the Git projects
-   root
+4. Then either
+
+     build Git with the GNU Make Makefile in the Git projects root
        make MSVC=1
-   or generate Visual Studio solution/projects (.sln/.vcproj) with the
+   or
+
+   generate Visual Studio solution/projects (.sln/.vcproj) with the
    command
        perl contrib/buildsystems/generate -g Vcproj
    and open and build the solution with the IDE
        devenv git.sln /useenv
-   or build with the IDE build engine directly from the command line
+     or
+
+   build with the IDE build engine directly from the command line
        devenv git.sln /useenv /build "Release|Win32"
    The /useenv option is required, so Visual Studio picks up the
    environment variables for the support libraries required to build
    Git, which you set up in step 1.
 
 Done!
+
+Or, use the msvc-build script; also in /msysgit/bin/msvc-build
+
+For those with VS2010: You may need to associate the generated .sln file with VS2010 rather than the MS version selector program.
-- 
1.9.4.msysgit.0

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [RFC 0/4] Fix the Visual Studio 2008 .sln generator

[Johannes Schindelin]

Hi Philip,

On Thu, 20 Nov 2014, Philip Oakley wrote:

> Are the patches going in the right direction?

Yes.

A couple of general comments:

- please do not comment out code. Just remove it.

- the first three commit messages look funny, being indented by 4
  spaces... unintentional?

> Is the processing of the .obj file in engine.pl sensible?

Yes, but instead of adding dead code, it would be better to use the
comment "# ignore". I would also strongly suggest to hard-code the
complete file name instead of just the extension. We know exactly which
file name we want to ignore, and if there should be another .obj file
eventually, it would be wrong to ignore it, too.

> and the extra care with s/\.o$/.c/ avoiding s/obj/cbj/.

Technically, you need to use a group \($\|[ \t]\), but I think that that
would be overkill.

> Does it affect the Qmake capability? (I've no idea)

Frankly, neither do I ;-) But since you touched only engine.pl, I would
expect Qmake not to be affected at all, right?

> Is the quoting of filenames correct? (my perl foo is cargo cult!)

IANAPME (I am not a Perl Monk either), but it looks good to me.

> I've also updated the vcbuild/README to mention Msysgit (which
> will be replaced soon by the newer/better Git-for-windows/SDK
> (https://github.com/git-for-windows/sdk), but the benefits still
> apply.

The path you used is /msysgit/bin/msvc-build, but the real path would be
/bin/msvc-build.

> Obviously, the patches will need squashing together,

To the contrary, I like the current separation of concerns.

Ciao,
Johannes

Re: [RFC 1/4] Fix i18n -o option in msvc engine.pl

[Johannes Schindelin]

Hi Philip,

On Thu, 20 Nov 2014, Philip Oakley wrote:

>     The i18n 5e9637c6 introduced an extra '-o' option
>     into the make file,

I take it you are referring to

	https://github.com/git/git/commit/5e9637c6#diff-b67911656ef5d18c4ae36cb6741b7965R2195

> diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl
> index 23da787..9144ea7 100755
> --- a/contrib/buildsystems/engine.pl
> +++ b/contrib/buildsystems/engine.pl
> @@ -140,6 +140,18 @@ sub parseMakeOutput
>              next;
>          }
>  
> +        if ($text =~ /^mkdir /) {
> +            # options to the Portable Object translations in the line
> +            # mkdir -p po/... && msgfmt ... (eg -o) may be mistaken for linker options

Maybe better

		# the line "mkdir ... && msgfmt ..." contains no linker options

> +            next;
> +        }
> +
> +        if ($text =~ /^msgfmt /) {
> +            # options to the Portable Object translations in the line
> +            # mkdir -p po/... && msgfmt ... (eg -o) may be mistaken for linker options
> +            next;
> +        }

These two if clauses do the same, maybe call it

	if ($test =~ /^(mkdir|msgfmt) /)

Ciao,
Johannes

Re: [RFC 2/4] Properly accept quoted space in filenames

[Johannes Schindelin]

Hi Philip,

apart from adding dead code:

On Thu, 20 Nov 2014, Philip Oakley wrote:

> diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl
> index 9144ea7..8e41808 100755
> --- a/contrib/buildsystems/engine.pl
> +++ b/contrib/buildsystems/engine.pl
> @@ -243,7 +244,8 @@ sub removeDuplicates
>  sub handleCompileLine
>  {
>      my ($line, $lineno) = @_;
> -    my @parts = split(' ', $line);
> +    # my @parts = split(' ', $line);
> +    my @parts = quotewords('\s+', 0, $line);
>      my $sourcefile;

(i.e. the commented-out line) it looks good to me.

Ciao,
Johannes

Re: [RFC 3/4] engine.pl: split the .o and .obj processing

[Johannes Schindelin]

Hi Philip,

On Thu, 20 Nov 2014, Philip Oakley wrote:

> Commit 4b623d80f7352 added an .obj file (invalidcontinue.obj) which was not
> processed correctly.
> 
> The generate engine then mistakenly did a s/.o/.c/  to create a request
> to compile invalidcontinue.cbj.

This is good. However, this:

> Split the '/\.(o|obj)$' in engine.pl#L353 into:
> 
>         } elsif ($part =~ /\.o$/) { # was '/\.(o|obj)$'
>             push(@objfiles, $part);
>         } elsif ($part =~ /\.obj$/) { # was '/\.(o|obj)$'
>             # push(@objfiles, $part); # do nothing
>         } else {

just repeats what the diff says, so it is unnecessary in the commit
message.

> diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl
> index 8e41808..16c3dd5 100755
> --- a/contrib/buildsystems/engine.pl
> +++ b/contrib/buildsystems/engine.pl
> @@ -264,7 +264,9 @@ sub handleCompileLine
>          } elsif ($part =~ /\.(c|cc|cpp)$/) {
>              $sourcefile = $part;
>          } else {
> -            die "Unhandled compiler option @ line $lineno: $part";
> +            print "full line: $line\n";
> +            print "A:-Unhandled compiler option @ line $lineno: $part\n"; # die (if !DEBUG)
> +#            die "Unhandled compiler option @ line $lineno: $part";

This needs to be backed out. I agree that it is nice to get going and to
debug, so I would split it out into its own commit while working on the
branch, but then drop it before submitting.

> @@ -290,14 +292,15 @@ sub handleLibLine
>[...]
>      foreach (@objfiles) {
>          my $sourcefile = $_;
> -        $sourcefile =~ s/\.o/.c/;
> +        $sourcefile =~ s/\.o$/.c/;

Ah, I see from the context that my earlier comment about the white-space
delimiter was wrong: at this stage, we already have split the list. So
this is groovy.

> @@ -343,8 +346,10 @@ sub handleLinkLine
>          } elsif ($part =~ /\.(a|lib)$/) {
>              $part =~ s/\.a$/.lib/;
>              push(@libs, $part);
> -        } elsif ($part =~ /\.(o|obj)$/) {
> +        } elsif ($part =~ /\.o$/) { # was '/\.(o|obj)$'
>              push(@objfiles, $part);
> +        } elsif ($part =~ /\.obj$/) { # was '/\.(o|obj)$'
> +            # push(@objfiles, $part); # do nothing

How about the following instead?

+		} elsif ($part eq 'invalidcontinue.obj') {
+			# ignore
 		} elsif ($part =~ /\.(o|obj)$/) {

? After all, this change is really only about handling the newly
introduced invalidcontinue.obj correctly.

Ciao,
Johannes

Re: [RFC 4/4] Improve layout and reference msvc-build script

[Johannes Schindelin]

Hi Philip,

On Thu, 20 Nov 2014, Philip Oakley wrote:

> [...]
> +Or, use the msvc-build script; also in /msysgit/bin/msvc-build

As I mentioned before, from a Git Bash on Windows, the path is
/bin/msvc-build (no /msysgit/). That is quite likely to stay the same with
the upcoming Git for Windows SDK, too (once I add the file to the SDK).

By the way, I think we should also start thinking about a Jenkins job to
verify that upstream changes such as invalidcontinue.obj do not break the
MSVC build. Please let me know if you want to give it a try, I have a
Windows Azure instance with a Jenkins instance, sponsored by Microsoft.

Ciao,
Johannes

Re: [RFC 0/4] Fix the Visual Studio 2008 .sln generator

[Philip Oakley]

From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>
> Hi Philip,
>
> On Thu, 20 Nov 2014, Philip Oakley wrote:
>
>> Are the patches going in the right direction?
>
> Yes.
Magic. Glad of the confirmation
>
> A couple of general comments:
>
> - please do not comment out code. Just remove it.
It's my debugging style until I get it working ;-0
>
> - the first three commit messages look funny, being indented by 4
>  spaces... unintentional?
I think that was from one of the tools I was using at the time. They'll 
be tidied and reflowed for the final version(s).
>
>> Is the processing of the .obj file in engine.pl sensible?
>
> Yes, but instead of adding dead code, it would be better to use the
> comment "# ignore". I would also strongly suggest to hard-code the
> complete file name instead of just the extension.
True, I realised that after posting (isnt it always the way).

> We know exactly which
> file name we want to ignore, and if there should be another .obj file
> eventually, it would be wrong to ignore it, too.

My concern was more about why a 'make' would produce .obj files (rather 
than .o files) in the first place. As I understand it we should never 
see .obj files generated from make with the exception of these ones that 
are library files implicitly known to MSVC - any thoughts?

>
>> and the extra care with s/\.o$/.c/ avoiding s/obj/cbj/.
>
> Technically, you need to use a group \($\|[ \t]\), but I think that 
> that
> would be overkill.
>
>> Does it affect the Qmake capability? (I've no idea)
>
> Frankly, neither do I ;-) But since you touched only engine.pl, I 
> would
> expect Qmake not to be affected at all, right?

As I understood the call sequence merry go round, the engine can pull in 
either function depending on command line options, hence the open 
question. But like you say, why expect it to be affected, and no-one has 
mentioned on list for years;-)

>
>> Is the quoting of filenames correct? (my perl foo is cargo cult!)
>
> IANAPME (I am not a Perl Monk either), but it looks good to me.
OK.
>
>> I've also updated the vcbuild/README to mention Msysgit (which
>> will be replaced soon by the newer/better Git-for-windows/SDK
>> (https://github.com/git-for-windows/sdk), but the benefits still
>> apply.
>
> The path you used is /msysgit/bin/msvc-build, but the real path would 
> be
> /bin/msvc-build.
>
>> Obviously, the patches will need squashing together,
>
> To the contrary, I like the current separation of concerns.
At the moment the first patch doesn't fully cure the .sln build, but 
there's no test anyway so bisecting would be problematic.

I'm happy to keep them small separate and clean ;-)

>
> Ciao,
> Johannes
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [RFC 1/4] Fix i18n -o option in msvc engine.pl

[Philip Oakley]

From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>
> Hi Philip,
>
> On Thu, 20 Nov 2014, Philip Oakley wrote:
>
>>     The i18n 5e9637c6 introduced an extra '-o' option
>>     into the make file,
>
> I take it you are referring to
>
> https://github.com/git/git/commit/5e9637c6#diff-b67911656ef5d18c4ae36cb6741b7965R2195

Yes, Makefile#L2195 (of the new version)
>
>> diff --git a/contrib/buildsystems/engine.pl 
>> b/contrib/buildsystems/engine.pl
>> index 23da787..9144ea7 100755
>> --- a/contrib/buildsystems/engine.pl
>> +++ b/contrib/buildsystems/engine.pl
>> @@ -140,6 +140,18 @@ sub parseMakeOutput
>>              next;
>>          }
>>
>> +        if ($text =~ /^mkdir /) {
>> +            # options to the Portable Object translations in the 
>> line
>> +            # mkdir -p po/... && msgfmt ... (eg -o) may be mistaken 
>> for linker options
>
> Maybe better
>
> # the line "mkdir ... && msgfmt ..." contains no linker options

OK will use.
>
>> +            next;
>> +        }
>> +
>> +        if ($text =~ /^msgfmt /) {
>> +            # options to the Portable Object translations in the 
>> line
>> +            # mkdir -p po/... && msgfmt ... (eg -o) may be mistaken 
>> for linker options
>> +            next;
>> +        }
>
> These two if clauses do the same, maybe call it
>
> if ($test =~ /^(mkdir|msgfmt) /)

That's good, will use.

>
> Ciao,
> Johannes
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [RFC 3/4] engine.pl: split the .o and .obj processing

[Philip Oakley]

From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>
> Hi Philip,
>
> On Thu, 20 Nov 2014, Philip Oakley wrote:
>
>> Commit 4b623d80f7352 added an .obj file (invalidcontinue.obj) which 
>> was not
>> processed correctly.
>>
>> The generate engine then mistakenly did a s/.o/.c/  to create a 
>> request
>> to compile invalidcontinue.cbj.
>
> This is good. However, this:
>
>> Split the '/\.(o|obj)$' in engine.pl#L353 into:
>>
>>         } elsif ($part =~ /\.o$/) { # was '/\.(o|obj)$'
>>             push(@objfiles, $part);
>>         } elsif ($part =~ /\.obj$/) { # was '/\.(o|obj)$'
>>             # push(@objfiles, $part); # do nothing
>>         } else {
>
> just repeats what the diff says, so it is unnecessary in the commit
> message.
OK.

>
>> diff --git a/contrib/buildsystems/engine.pl 
>> b/contrib/buildsystems/engine.pl
>> index 8e41808..16c3dd5 100755
>> --- a/contrib/buildsystems/engine.pl
>> +++ b/contrib/buildsystems/engine.pl
>> @@ -264,7 +264,9 @@ sub handleCompileLine
>>          } elsif ($part =~ /\.(c|cc|cpp)$/) {
>>              $sourcefile = $part;
>>          } else {
>> -            die "Unhandled compiler option @ line $lineno: $part";
>> +            print "full line: $line\n";
>> +            print "A:-Unhandled compiler option @ line $lineno: 
>> $part\n"; # die (if !DEBUG)
>> +#            die "Unhandled compiler option @ line $lineno: $part";
>
> This needs to be backed out. I agree that it is nice to get going and 
> to
> debug, so I would split it out into its own commit while working on 
> the
> branch, but then drop it before submitting.

I'll probably split this "improvement in error reporting" (by providing 
the full offending line) into a separate patch, on the basis that we 
want (Windows) folks to start helping, so let's make the error messages 
more useful to those who don't know these scripting languages yet.

>
>> @@ -290,14 +292,15 @@ sub handleLibLine
>>[...]
>>      foreach (@objfiles) {
>>          my $sourcefile = $_;
>> -        $sourcefile =~ s/\.o/.c/;
>> +        $sourcefile =~ s/\.o$/.c/;
>
> Ah, I see from the context that my earlier comment about the 
> white-space
> delimiter was wrong: at this stage, we already have split the list. So
> this is groovy.
>
>> @@ -343,8 +346,10 @@ sub handleLinkLine
>>          } elsif ($part =~ /\.(a|lib)$/) {
>>              $part =~ s/\.a$/.lib/;
>>              push(@libs, $part);
>> -        } elsif ($part =~ /\.(o|obj)$/) {
>> +        } elsif ($part =~ /\.o$/) { # was '/\.(o|obj)$'
>>              push(@objfiles, $part);
>> +        } elsif ($part =~ /\.obj$/) { # was '/\.(o|obj)$'
>> +            # push(@objfiles, $part); # do nothing
>
> How about the following instead?
>
> + } elsif ($part eq 'invalidcontinue.obj') {
> + # ignore
>  } elsif ($part =~ /\.(o|obj)$/) {

Looks good, I'll use that (after deciding whether .obj files should be 
expected in a 'make' output anyway)

>
> ? After all, this change is really only about handling the newly
> introduced invalidcontinue.obj correctly.
Did it ever handle .obj's correctly (rhet) - I dunno.

>
> Ciao,
> Johannes
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [RFC 4/4] Improve layout and reference msvc-build script

[Philip Oakley]

From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>
> Hi Philip,
>
> On Thu, 20 Nov 2014, Philip Oakley wrote:
>
>> [...]
>> +Or, use the msvc-build script; also in /msysgit/bin/msvc-build
>
> As I mentioned before, from a Git Bash on Windows, the path is
> /bin/msvc-build (no /msysgit/). That is quite likely to stay the same 
> with
> the upcoming Git for Windows SDK, too (once I add the file to the 
> SDK).

I was trying to identify the source file (as I see it in my Msysgit 
install). However it will all need tidying to distinguish what is seen 
if you have G4W-SDK, rather just installing the new G4W (when ready)
>
> By the way, I think we should also start thinking about a Jenkins job 
> to
> verify that upstream changes such as invalidcontinue.obj do not break 
> the
> MSVC build. Please let me know if you want to give it a try, I have a
> Windows Azure instance with a Jenkins instance, sponsored by 
> Microsoft.

This is not something I'm familiar with, but it's something I could have 
a look at.
On my list is also the 'git bundle' symref problem, which we've got a 
solution for that 'just' needs coding, along with thoughts about 
narrow/sparse checkout/clone/fetch.

One of the confusions I have is the distinction between building with 
the MSVC compiler (cl command line) and building within VS2008, and 
whether they are different things (i.e. does VS2008 bring extra 
baggage?).
--
Philip


>
> Ciao,
> Johannes
> 

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [RFC 2/4] Properly accept quoted space in filenames

[Junio C Hamano]

Philip Oakley <philipoakley@iee.org> writes:

>  sub handleCompileLine
>  {
>      my ($line, $lineno) = @_;
> -    my @parts = split(' ', $line);
> +    # my @parts = split(' ', $line);
> +    my @parts = quotewords('\s+', 0, $line);

Can somebody enlighten me why/if quotewords is preferrable over
shellwords in the context of this patch?


-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [RFC 2/4] Properly accept quoted space in filenames

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
> Philip Oakley <philipoakley@iee.org> writes:
>
>>  sub handleCompileLine
>>  {
>>      my ($line, $lineno) = @_;
>> -    my @parts = split(' ', $line);
>> +    # my @parts = split(' ', $line);
>> +    my @parts = quotewords('\s+', 0, $line);
>
> Can somebody enlighten me why/if quotewords is preferrable over
> shellwords in the context of this patch?

"No" - Ignorance is bliss ;-) I think my cargo culting was the result of 
some googling for "quoting perl variables" or some such, which obviously 
came up with quotewords - I'm happy to take advice on this one!

quotewords did appear to work though back when I wrote this: 86dcfcf 
(Properly accept quoted space in filenames, 2012-05-06)
--
Philip 

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [RFC 2/4] Properly accept quoted space in filenames

[Junio C Hamano]

"Philip Oakley" <philipoakley@iee.org> writes:

> From: "Junio C Hamano" <gitster@pobox.com>
>> Philip Oakley <philipoakley@iee.org> writes:
>>
>>>  sub handleCompileLine
>>>  {
>>>      my ($line, $lineno) = @_;
>>> -    my @parts = split(' ', $line);
>>> +    # my @parts = split(' ', $line);
>>> +    my @parts = quotewords('\s+', 0, $line);
>>
>> Can somebody enlighten me why/if quotewords is preferrable over
>> shellwords in the context of this patch?
>
> "No" - Ignorance is bliss ;-) I think my cargo culting was the result
> of some googling for "quoting perl variables" or some such, which
> obviously came up with quotewords - I'm happy to take advice on this
> one!
>
> quotewords did appear to work though back when I wrote this: 86dcfcf
> (Properly accept quoted space in filenames, 2012-05-06)

A quick websearch shows me:

  http://cpansearch.perl.org/src/CHORNY/Text-ParseWords-3.29/ParseWords.pm

and comparing the implementations of the two, the difference boils
down to just one line to me.

    sub shellwords {
        my (@lines) = @_;
        my @allwords;

        foreach my $line (@lines) {
            $line =~ s/^\s+//;
            my @words = parse_line('\s+', 0, $line);
            pop @words if (@words and !defined $words[-1]);
            return() unless (@words || !length($line));
            push(@allwords, @words);
        }
        return(@allwords);
    }

In quotewords, the call to parse_line uses $keep not hardcoded 0
(which would not make any difference in the context of your patch),
and it assumes parse_line() never returns a singleton "undef" so the
line "pop ... if @words is a list with 'undef' as its sole element"
is missing.

Of course, the caller would become smaller and sweeter, i.e.

    my @parts = shellwords($line);

I am not familiar with if Perl folks have certain convention to
decide when to use which one, though ;-)




-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [RFC 3/4] engine.pl: split the .o and .obj processing

[Johannes Schindelin]

Hi,

On Fri, 21 Nov 2014, Philip Oakley wrote:

> From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>
>
> > On Thu, 20 Nov 2014, Philip Oakley wrote:
> >
> > > @@ -343,8 +346,10 @@ sub handleLinkLine
> > >          } elsif ($part =~ /\.(a|lib)$/) {
> > >              $part =~ s/\.a$/.lib/;
> > >              push(@libs, $part);
> > > -        } elsif ($part =~ /\.(o|obj)$/) {
> > > +        } elsif ($part =~ /\.o$/) { # was '/\.(o|obj)$'
> > >              push(@objfiles, $part);
> > > +        } elsif ($part =~ /\.obj$/) { # was '/\.(o|obj)$'
> > > +            # push(@objfiles, $part); # do nothing
> >
> > How about the following instead?
> >
> > + } elsif ($part eq 'invalidcontinue.obj') {
> > + # ignore
> >  } elsif ($part =~ /\.(o|obj)$/) {
> 
> Looks good, I'll use that (after deciding whether .obj files should be
> expected in a 'make' output anyway)

My idea to hardcode invalidcontinue.obj was that I'd rather see a failure
if an unexpected .obj is seen there. But I realize that my suggested
change does not exactly accomplish that.

Ciao,
Johannes

Re: Re: [RFC 3/4] engine.pl: split the .o and .obj processing

[Philip Oakley]

From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>
>> > How about the following instead?
>> >
>> > + } elsif ($part eq 'invalidcontinue.obj') {
>> > + # ignore
>> >  } elsif ($part =~ /\.(o|obj)$/) {
>>
>> Looks good, I'll use that (after deciding whether .obj files should
>> be
>> expected in a 'make' output anyway)
>
> My idea to hardcode invalidcontinue.obj was that I'd rather see a
> failure
> if an unexpected .obj is seen there. But I realize that my suggested
> change does not exactly accomplish that.
>
I've decided to include the hard code suggested, and after a few tests
decided it was probably worth keeping the .obj processing, even though
we don't expect any.

At least I've now managed to generate a .sln project, though it won't
compile because of a number of other changes, and I haven't got to the
bottom of them all.

Notes:
Revert "Windows: correct detection of EISDIR in mingw_open()"
git\compat\mingw.c(315) : error C2065: 'O_ACCMODE' : undeclared
identifier (neither VS2008 nor VS2010 declare 'O_ACCMODE'). add an 
#ifdef
_MSC_VER to define this if we are in MSVC.

Revert "mingw.h: add dummy functions for sigset_t operations" commit
4e6d207c45. the Additional Note: to
http://www.spinics.net/lists/git/msg240248.html indicates that this
breaks MSVC. I think it will also need an #ifdef to see if we are in 
MSVC.

I've also found a problem with my setup (msysgit 1.7.9 for make) barfing
on the commit/ee9be06 "perl: detect new files in MakeMaker builds" which
'make -n' says 'No rule to make target `PM.stamp', needed by `perl.mak'.

There was also LINK : warning LNK4098: defaultlib 'MSVCRT' conflicts
with use of other libs; use /NODEFAULTLIB:library.

I've also got notes from way back that there was build order problem
with 'vcs-svn_lib.lib' - probably a sort order issue. IIRC at that time,
I had to compile the project twice so that the lib was visible the
second time.

So there's plenty to go at ;-) I'm going to be away this coming week, So
at least I have the whole next cycle to make some progress.

--

Philip

-- 
-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en

--- 
You received this message because you are subscribed to the Google Groups "Git for Windows" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.