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

2014-11-23 Thread Philip Oakley

From: "Johannes Schindelin" 

> 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

--
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: [RFC 3/4] engine.pl: split the .o and .obj processing

2014-11-23 Thread Johannes Schindelin
Hi,

On Fri, 21 Nov 2014, Philip Oakley wrote:

> From: "Johannes Schindelin" 
>
> > 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
--
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: [RFC 3/4] engine.pl: split the .o and .obj processing

2014-11-21 Thread Philip Oakley

From: "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.

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.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: [RFC 3/4] engine.pl: split the .o and .obj processing

2014-11-21 Thread 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
--
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