threads / patch / 39900

patch, 16 partsMake the msvc-build scripts work again

Subject: [PATCH v2b 00/16, 2 updates] Make the msvc-build scripts work again

## tl;dr

9 messages between Jul 20, 2015 and Jul 22, 2015. Diffs are folded; open one to read it.

replies: 8people: 3as markdown or json

Philip Oakley· Jul 20, 2015, 22:54 UTC · lore
This updates two patches in the series based on Eric Sunshine's comments.
Patch 8b updates the commit message to make clear what was going wrong.
Patch 10b improves the perl code.
Junio: would a full re-roll be appropriate at a suitable point?
Philip Oakley (2):
  engine.pl: ignore invalidcontinue.obj which is known to MSVC
  engine.pl: delete the captured stderr file if empty
-- 
2.4.2.windows.1.5.gd32afb6
Philip Oakley· Jul 20, 2015, 22:54 UTC · re: Philip Oakley · lore

[PATCH v2 8b/16] engine.pl: ignore invalidcontinue.obj which is known to MSVC

Commit 4b623d8 (MSVC: link in invalidcontinue.obj for better POSIX compatibility, 2014-03-29) introduced invalidcontinue.obj into the Makefile output, which was not parsed correctly by the buildsystem. Ignore it, as it is known to Visual Studio and, there is no matching source file.

Only substitute filenames ending with .o when generating the source .c filename, otherwise a .cbj file may be expected.

Split the .o and .obj processing; 'make' does not produce .obj files.

In the future there may be source files that produce .obj files so keep the two issues (.obj files with & without source files) separate.

Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
 contrib/buildsystems/engine.pl | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
Show changes to contrib/buildsystems/engine.pl +7 −3
diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl
index 60c7a7d..9db3d43 100755
--- a/contrib/buildsystems/engine.pl
+++ b/contrib/buildsystems/engine.pl
@@ -289,7 +289,7 @@ sub handleLibLine
 #    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"}});
@@ -333,8 +333,12 @@ sub handleLinkLine
         } elsif ($part =~ /\.(a|lib)$/) {
             $part =~ s/\.a$/.lib/;
             push(@libs, $part);
-        } elsif ($part =~ /\.(o|obj)$/) {
+        } elsif ($part eq 'invalidcontinue.obj') {
+            # ignore - known to MSVC
+        } elsif ($part =~ /\.o$/) {
             push(@objfiles, $part);
+        } elsif ($part =~ /\.obj$/) {
+            # do nothing, 'make' should not be producing .obj, only .o files
         } else {
             die "Unhandled link option @ line $lineno: $part";
         }
@@ -343,7 +347,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"}});
-- 
2.3.1
Philip Oakley· Jul 20, 2015, 22:54 UTC · re: Philip Oakley · lore

[PATCH v2 10b/16] engine.pl: delete the captured stderr file if empty

Keep the build clean of extraneous files if it is indeed clean. Otherwise leave the msvc-build-makedryerrors.txt file both as a flag for any CI system or for manual debugging.

Alternatively, with improved syntactic sugar[1]:
	unlink $ErrsFile if -f -z $ErrsFile;
could be used but requires Perl 5.10 or later, which is not
available on Msysgit, but is available on the newer Git-for-Windows
SDK on Msys2 [2].

Note that the file will contain the new values of the GIT_VERSION and GITGUI_VERSION if they were generated by the make file. They are omitted if the release is tagged and identically defined in their respective GIT_VERSION_GEN file DEF_VER variables.

[1]: http://perldoc.perl.org/functions/-X.html [2]: http://git-for-windows.github.io/

Helped-by:  Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
Eric's help gmane.comp.version-control.msysgit/21745
---
 contrib/buildsystems/engine.pl | 2 ++
 1 file changed, 2 insertions(+)
Show changes to contrib/buildsystems/engine.pl +2 −0
diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl
index a6999b6..154575d 100755
--- a/contrib/buildsystems/engine.pl
+++ b/contrib/buildsystems/engine.pl
@@ -77,6 +77,8 @@ EOM
 
 my $ErrsFile = "msvc-build-makedryerrors.txt";
 @makedry = `cd $git_dir && make -n MSVC=1 V=1 2>$ErrsFile` if !@makedry;
+# test for an empty Errors file and remove it
+unlink $ErrsFile if -f $ErrsFile && -z _;
 
 # Parse the make output into usable info
 parseMakeOutput();
-- 
2.3.1
Junio C Hamano· Jul 20, 2015, 23:07 UTC · re: Philip Oakley · lore

Re: [PATCH v2b 00/16, 2 updates] Make the msvc-build scripts work again

Philip Oakley <philipoakley@iee.org> writes:
Show 5 quoted lines
> This updates two patches in the series based on Eric Sunshine's comments.
>
> Patch 8b updates the commit message to make clear what was going wrong.
>
> Patch 10b improves the perl code.

Is v2b like saying v3 or something else? Does 8b replaces 8 or updates it (i.e. comes between 8 and 9)?

> Junio: would a full re-roll be appropriate at a suitable point?

Probably, but I'd like to see people try it out and give positive feedback first. This part of the tree I can give no input or pre- pushout testing at all.

Who are the people involved in this part of the system in the past? Does "shortlog -n --no-merges contrib/buildsystems compat/vcbuild" tell us anything?

> Philip Oakley (2):
>
>   engine.pl: ignore invalidcontinue.obj which is known to MSVC
>   engine.pl: delete the captured stderr file if empty
Philip Oakley· Jul 21, 2015, 16:46 UTC · re: Junio C Hamano · lore

Re: [PATCH v2b 00/16, 2 updates] Make the msvc-build scripts work again

From: "Junio C Hamano" <gitster@pobox.com>
Show 12 quoted lines
> Philip Oakley <philipoakley@iee.org> writes:
>
>> This updates two patches in the series based on Eric Sunshine's
>> comments.
>>
>> Patch 8b updates the commit message to make clear what was going
>> wrong.
>>
>> Patch 10b improves the perl code.
>
> Is v2b like saying v3 or something else?  Does 8b replaces 8 or
> updates it (i.e. comes between 8 and 9)?

Sorry for the confusion. Yes these are quick replacements, rather than re-rolling the whole series immediately.

Show 6 quoted lines
>
>> Junio: would a full re-roll be appropriate at a suitable point?
>
> Probably, but I'd like to see people try it out and give positive
> feedback first.  This part of the tree I can give no input or pre-
> pushout testing at all.

It has been tried out on the Msysgit list, and also against the github Pull Request, with responses noted there.

I'll do another branch / rebase and PR onto the G4W SDK as well, for an extra chance at getting more replies. Ideally, if part of this mainstream Git, it would get picked up automatically by them (rather than being local 'fixes' endlessly carried forward).

Show 5 quoted lines
>
> Who are the people involved in this part of the system in the past?
> Does "shortlog -n --no-merges contrib/buildsystems compat/vcbuild"
> tell us anything?
>

There has been no activity here on the 'create a visual studio project' aspects in the last few years. Any changes listed in the logs relate to ensuring that the MSVC compiler will run as part of a regular Makefile run (IIUC). The last significant commit was 74cf9bd (engine.pl: Fix a recent breakage of the buildsystem generator, 2010-01-22) Ramsay Jones, so that's five and a half years. Mind you, it's taken me a while to find all the bit rots.

>
>> Philip Oakley (2):
>
Junio C Hamano· Jul 21, 2015, 19:25 UTC · re: Philip Oakley · lore

Re: [PATCH v2b 00/16, 2 updates] Make the msvc-build scripts work again

"Philip Oakley" <philipoakley@iee.org> writes:
> ... Ideally, if part of this
> mainstream Git, it would get picked up automatically by them
> (rather than being local 'fixes' endlessly carried forward).
Actually, that is not "ideal", but what I want to avoid.

As I do not do Windows, it simply is wrong for me to apply changes that are very likely to affect Windows folks without seeing their support first, which they may end up having to revert on their end and endlessly carry that revert forward. That is why I very much prefer to see changes get there first and then forwarded in my direction once they are happy with them.

Show 6 quoted lines
> There has been no activity here on the 'create a visual studio project'
> aspects in the last few years. Any changes listed in the logs relate to
> ensuring that the MSVC compiler will run as part of a regular Makefile
> run (IIUC). The last significant commit was 74cf9bd (engine.pl: Fix a
> recent breakage of the buildsystem generator, 2010-01-22) Ramsay Jones,
> so that's five and a half years.

I think Ramsay is still around on the list; I do not know if he still does Windows, though.

Junio C Hamano· Jul 21, 2015, 20:04 UTC · re: Junio C Hamano · lore

Re: [PATCH v2b 00/16, 2 updates] Make the msvc-build scripts work again

Junio C Hamano <gitster@pobox.com> writes:
Show 11 quoted lines
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>> ... Ideally, if part of this
>> mainstream Git, it would get picked up automatically by them
>> (rather than being local 'fixes' endlessly carried forward).
>
> Actually, that is not "ideal", but what I want to avoid.
>
> As I do not do Windows, it simply is wrong for me to apply changes
> that are very likely to affect Windows folks without seeing their
> support first,...

Just to clarify this part. I do not do RedHat, Solaris or OSX, either. Also MSVC may not be what GfW folks primarily target.

But the thing is that (1) Windows is so much different, and (2) GfW folks are much more qualified to judge platform-specific issues on Windows than I am.

Even though I may still need to have a say in the overall structure of the changes to the upstream tree coming from them (e.g. "Don't sprinkle #ifdef all over the generic code; instead add a wrapper or two in compat/ to keep the generic code generic" is something I may tell them when rejecting a change forwarded to me), I trust them a lot more than I trust myself when it comes to what the changes do in the platform-specific part and how they do it.

Thanks.
Philip Oakley· Jul 21, 2015, 20:39 UTC · re: Junio C Hamano · lore

Re: [PATCH v2b 00/16, 2 updates] Make the msvc-build scripts work again

From: "Junio C Hamano" <gitster@pobox.com>
Show 31 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
>> "Philip Oakley" <philipoakley@iee.org> writes:
>>
>>> ... Ideally, if part of this
>>> mainstream Git, it would get picked up automatically by them
>>> (rather than being local 'fixes' endlessly carried forward).
>>
>> Actually, that is not "ideal", but what I want to avoid.
>>
>> As I do not do Windows, it simply is wrong for me to apply changes
>> that are very likely to affect Windows folks without seeing their
>> support first,...
>
> Just to clarify this part.  I do not do RedHat, Solaris or OSX,
> either.  Also MSVC may not be what GfW folks primarily target.
>
> But the thing is that (1) Windows is so much different, and (2) GfW
> folks are much more qualified to judge platform-specific issues on
> Windows than I am.
>
> Even though I may still need to have a say in the overall structure
> of the changes to the upstream tree coming from them (e.g. "Don't
> sprinkle #ifdef all over the generic code; instead add a wrapper or
> two in compat/ to keep the generic code generic" is something I may
> tell them when rejecting a change forwarded to me), I trust them a
> lot more than I trust myself when it comes to what the changes do in
> the platform-specific part and how they do it.
>
> Thanks.
> --

Thanks for the extra clarification. I'll ask if they can confirm this update on GfW.

I've also asked Ramsay what his current situation is re- you previous email and the msvc-build.

regards
Philip 
Ramsay Jones· Jul 22, 2015, 22:57 UTC · re: Junio C Hamano · lore

Re: [PATCH v2b 00/16, 2 updates] Make the msvc-build scripts work again

On 21/07/15 20:25, Junio C Hamano wrote:
Show 24 quoted lines
> "Philip Oakley" <philipoakley@iee.org> writes:
> 
>> ... Ideally, if part of this
>> mainstream Git, it would get picked up automatically by them
>> (rather than being local 'fixes' endlessly carried forward).
> 
> Actually, that is not "ideal", but what I want to avoid.
> 
> As I do not do Windows, it simply is wrong for me to apply changes
> that are very likely to affect Windows folks without seeing their
> support first, which they may end up having to revert on their end
> and endlessly carry that revert forward.  That is why I very much
> prefer to see changes get there first and then forwarded in my
> direction once they are happy with them.
> 
>> There has been no activity here on the 'create a visual studio project'
>> aspects in the last few years. Any changes listed in the logs relate to
>> ensuring that the MSVC compiler will run as part of a regular Makefile
>> run (IIUC). The last significant commit was 74cf9bd (engine.pl: Fix a
>> recent breakage of the buildsystem generator, 2010-01-22) Ramsay Jones,
>> so that's five and a half years.
> 
> I think Ramsay is still around on the list; I do not know if he
> still does Windows, though.

[Sorry for not noticing this sooner, but I'm currently moving home (among other things) and, as a result, I have not been able to devote much time to the list ... (or anything else ;-) ]

I don't do much with MinGW or MSVC these days (I do still try to keep cygwin running - it's currently not in good shape). My MSVC installation is somewhat old (2008 I believe) and confined to my old 32-bit laptop (which doesn't get booted up too often now).

I had a quick squint at the patches and, at first glance, they looked quite reasonable to me, but I have not tested them. I can't promise to test them anytime soon (and my MSVC may be too old for these patches?). Sorry! :(

[If I find some time soon, I will give them a try and let you know.]

ATB, Ramsay Jones

← back to recent threads