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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Nov 21, 2014, 09:48 UTC
Message-ID: <alpine.DEB.1.00.1411211043050.13845@s15462909.onlinehome-server.info>
In-Reply-To: <1416526682-6024-4-git-send-email-philipoakley@iee.org>

Hi Philip,

On Thu, 20 Nov 2014, Philip Oakley wrote:

Show 5 quoted lines

> 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:

Show 7 quoted lines

> 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.

Show 12 quoted lines

> 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.

Show 6 quoted lines

> @@ -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.

Show 9 quoted lines

> @@ -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