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