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

2015-07-20 Thread Philip Oakley

From: Eric Sunshine sunsh...@sunshineco.com
On Sun, Jul 19, 2015 at 4:08 PM, Philip Oakley philipoak...@iee.org 
wrote:

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.

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 indentically defined in
their respective GIT_VERSION_GEN file DEF_VER variables.

Signed-off-by: Philip Oakley philipoak...@iee.org
---
diff --git a/contrib/buildsystems/engine.pl 
b/contrib/buildsystems/engine.pl

index a6999b6..020776e 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
+for ($ErrsFile) {unlink $_ if (-f $_)  (!-s $_);}


Why the 'for' loop?

Also, if you're using the 'for' loop for the $_ side-effect, then why
not the simpler:


It was cargo cult programming, with some Google searching to select 
between invocations. Most examples were looping through lists in 
scripts, hence the down select.




   for ($ErrsFile) { unlink if -f  !-s; }


A lot better. Will fix.



?



 # Parse the make output into usable info
 parseMakeOutput();
--
2.4.2.windows.1.5.gd32afb6




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


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

2015-07-20 Thread Eric Sunshine
On Mon, Jul 20, 2015 at 2:16 AM, Philip Oakley philipoak...@iee.org wrote:
 From: Eric Sunshine sunsh...@sunshineco.com
 On Sun, Jul 19, 2015 at 4:08 PM, Philip Oakley philipoak...@iee.org
 wrote:
 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.

 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 indentically defined in
 their respective GIT_VERSION_GEN file DEF_VER variables.

 Signed-off-by: Philip Oakley philipoak...@iee.org
 ---
 +# test for an empty Errors file and remove it
 +for ($ErrsFile) {unlink $_ if (-f $_)  (!-s $_);}

 Why the 'for' loop?

 Also, if you're using the 'for' loop for the $_ side-effect, then why
 not the simpler:

 It was cargo cult programming, with some Google searching to select between
 invocations. Most examples were looping through lists in scripts, hence the
 down select.

for ($ErrsFile) { unlink if -f  !-s; }

 A lot better. Will fix.

Although that works, I'm not sure that it's really all that desirable
due to the unnecessary and potentially confusing 'for' loop. I'd
probably just write it as:

unlink $ErrsFile if -f $ErrsFile  !-s _;

The lone '_' is magical[1] in that it re-uses the stat() information
from the -f rather than stat'ing $ErrsFile again. I'd also probably
replace !-s (not non-zero size) with -z (zero size):

unlink $ErrsFile if -f $ErrsFile  -z _;

And, if you're using Perl 5.10 or later, you could use a little
syntactic sugar[1] and stack the file test operators up against one
another:

unlink $ErrsFile if -f -z $ErrsFile;

which is the equivalent of the above with the sugar removed.

[1]: http://perldoc.perl.org/functions/-X.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-07-20 Thread Philip Oakley

From: Eric Sunshine sunsh...@sunshineco.com

On Mon, Jul 20, 2015 at 2:16 AM, Philip Oakley philipoak...@iee.org
wrote:

From: Eric Sunshine sunsh...@sunshineco.com

On Sun, Jul 19, 2015 at 4:08 PM, Philip Oakley
philipoak...@iee.org
wrote:

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.

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 indentically defined in
their respective GIT_VERSION_GEN file DEF_VER variables.

Signed-off-by: Philip Oakley philipoak...@iee.org
---
+# test for an empty Errors file and remove it
+for ($ErrsFile) {unlink $_ if (-f $_)  (!-s $_);}


Why the 'for' loop?

Also, if you're using the 'for' loop for the $_ side-effect, then
why
not the simpler:


It was cargo cult programming, with some Google searching to select
between
invocations. Most examples were looping through lists in scripts,
hence the
down select.


   for ($ErrsFile) { unlink if -f  !-s; }


A lot better. Will fix.


Although that works, I'm not sure that it's really all that desirable
due to the unnecessary and potentially confusing 'for' loop. I'd
probably just write it as:

   unlink $ErrsFile if -f $ErrsFile  !-s _;

The lone '_' is magical[1] in that it re-uses the stat() information
from the -f rather than stat'ing $ErrsFile again. I'd also probably
replace !-s (not non-zero size) with -z (zero size):

   unlink $ErrsFile if -f $ErrsFile  -z _;

And, if you're using Perl 5.10 or later,


The Msysgit (@1.9.5) uses perl v5.8.8, while the newer G4W SDK uses perl
5, version 20, subversion 2 (v5.20.2), so there is a decision to be made
about whether to leave the Msysgit version behind.

While it would be nice to use the newest version, I'm minded that we
should keep a little backward compatibility with Msysgit, at least until
the new G4w has had a few 'proper' releases, so not use the magic
suggestion below.

I've cc'd dscho, Johannes, J6t and Sebastian in case they have any firm
opinions with respect to the Msysgit / G4W split.


you could use a little
syntactic sugar[1] and stack the file test operators up against one
another:

   unlink $ErrsFile if -f -z $ErrsFile;

which is the equivalent of the above with the sugar removed.

[1]: http://perldoc.perl.org/functions/-X.html
--


Philip 


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


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

2015-07-20 Thread Eric Sunshine
On Mon, Jul 20, 2015 at 7:55 AM, Philip Oakley philipoak...@iee.org wrote:
 From: Eric Sunshine sunsh...@sunshineco.com
 Although that works, I'm not sure that it's really all that desirable
 due to the unnecessary and potentially confusing 'for' loop. I'd
 probably just write it as:

unlink $ErrsFile if -f $ErrsFile  !-s _;

 The lone '_' is magical[1] in that it re-uses the stat() information
 from the -f rather than stat'ing $ErrsFile again. I'd also probably
 replace !-s (not non-zero size) with -z (zero size):

unlink $ErrsFile if -f $ErrsFile  -z _;

 And, if you're using Perl 5.10 or later,

 The Msysgit (@1.9.5) uses perl v5.8.8, while the newer G4W SDK uses perl
 5, version 20, subversion 2 (v5.20.2), so there is a decision to be made
 about whether to leave the Msysgit version behind.

 While it would be nice to use the newest version, I'm minded that we
 should keep a little backward compatibility with Msysgit, at least until
 the new G4w has had a few 'proper' releases, so not use the magic
 suggestion below.

 you could use a little
 syntactic sugar[1] and stack the file test operators up against one
 another:

unlink $ErrsFile if -f -z $ErrsFile;

Since msysgit is only at Perl 5.8.8, it makes plenty of sense to just
stick with:

unlink $ErrsFile if -f $ErrsFile  -z _;

since it's only a tiny bit more verbose than:

unlink $ErrsFile if -f -z $ErrsFile;

and then you don't have to worry about needing a more modern Perl version.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-07-19 Thread Eric Sunshine
On Sun, Jul 19, 2015 at 4:08 PM, Philip Oakley philipoak...@iee.org wrote:
 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.

 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 indentically defined in
 their respective GIT_VERSION_GEN file DEF_VER variables.

 Signed-off-by: Philip Oakley philipoak...@iee.org
 ---
 diff --git a/contrib/buildsystems/engine.pl b/contrib/buildsystems/engine.pl
 index a6999b6..020776e 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
 +for ($ErrsFile) {unlink $_ if (-f $_)  (!-s $_);}

Why the 'for' loop?

Also, if you're using the 'for' loop for the $_ side-effect, then why
not the simpler:

for ($ErrsFile) { unlink if -f  !-s; }

?


  # Parse the make output into usable info
  parseMakeOutput();
 --
 2.4.2.windows.1.5.gd32afb6
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html