Re: [PATCH] scripts: support -I <dir> -L <dir> and -l <lib> for cl in compile

[Peter Rosin]

Stefano Lattarini skrev 2012-03-05 21:00:
> On 03/05/2012 02:25 PM, Peter Rosin wrote:
>> Hi!
>>
>> I noticed that some expect the compiler to accept a space between
>> the option letter and the option argument.  Looking at POSIX, this
>> seems like an acceptable expectation, if not even preferred.  So,
>> here's an update for the compile script when used to wrap MSVC.  I
>> didn't update the testsuite to prefer -l <lib> (with a space) since
>> I haven't seen that in practice anywhere.
>>
>> Ok for msvc and merges into master/branch-1.11?
>>
> Basically yes, but I'd like some changes to the testsuite edits to
> have a better coverage.  See below.
> 
>> POSIX mandates that the compiler accepts a space between the -I,
>> -l and -L options and their respective arguments.
>>
> (BTW, a link here would be great IMHO; absolutely not required for
> an ACK though)
> 
>> * lib/compile (func_cl_dashl): New function with factored out code
>> for implementing the -l option for the cl wrapper.
>> (func_cl_dashL): New function with factored out code implementing
>> the -L option for the cl wrapper.
>> (func_cl_wrapper): Use func_cl_dashl to implement both -l <lib>
>> and -l<lib>, and func_cl_dashL to implement both -L <dir> and
>> -L<dir>.  Also add support for -I <dir>.
>> (scriptversion): Update.
>> * tests/compile4.test: Prefer -L <dir> over -L<dir>.
>> * tests/compile5.test: Likewise.
>> * tests/compile6.test: Likewise.
>> * tests/compile3.test: Likewise.  Also add checks for the new
>> -I <dir> and -l <lib> formats while keeping a check for the no
>> longer preferred -L<dir> format.
>> * NEWS: Update.
> 
>> diff --git a/tests/compile3.test b/tests/compile3.test
>> index 15064a6..b27fc65 100755
>> --- a/tests/compile3.test
>> +++ b/tests/compile3.test
>> @@ -32,23 +32,23 @@ END
>>  chmod +x ./cl
>>  
>>  # Check if compile handles "-o foo.obj"
>> -opts=`./compile ./cl -c foo.c -o foo.obj -Ibaz`
>> +opts=`./compile ./cl -c foo.c -o foo.obj -I baz`
>>  test x"$opts" = x"-c foo.c -Fofoo.obj -Ibaz"
>>
> Instead of this (and similar changes), why not make the test cases "loop"
> so that they test both with and without spaces after the compile options?
> 
> Something like this:
> 
>   # POSIX mandates that the compiler accepts a space between the -I,
>   # -l and -L options and their respective arguments.  Traditionally,
>   # this should work also without a space.  Try both usages.
>   for sp in '' ' '; do
>     ...
>     # Check if compile handles "-o foo.obj"
>     opts=`./compile ./cl -c foo.c -o foo.obj -I${sp}baz`
>     test x"$opts" = x"-c foo.c -Fofoo.obj -Ibaz"
>     ...
>   done

I made the changes and pushed, but it feels a bit hard to read -l${sp}foo
compared to -lfoo, so I'm not too happy about it.  Oh well...

> Extra kudos if you then feel like writing a follow-up to convert these
> tests to use TAP and thus have a better granularity (of course, this
> is absolutely not required for an ACK).

Personally, I think a simple "make check" is a bit too verbose on the
terminal, especially for some tap tests that repeat a very similar test
with some minor variation.  These compile*.tests fall in that category,
and it's not likely that they are going to *regress* for some odd third
party reason, the likely failure is some change in the compile script
itself or some previously untested oddish system.  In both those cases
it's not too hard to take a look in the log file.  So, my (small)
complaint is that tap tests add too much noise (sometimes hundreds of
similar and trivial tests) to the "make check" output, and I'm going to
use that as an excuse to not add more noise. :-)

Anyway, thanks for looking!

Cheers,
Peter