Re: [PATCH 3/24] ver_linux: gcc.patch

[Alexander Kapshuk]

On Sat, Oct 3, 2015 at 6:07 PM, Richard Weinberger
<richard.weinberger@xxxxxxxxx> wrote:
> On Sat, Oct 3, 2015 at 3:07 PM, Alexander Kapshuk
> <alexander.kapshuk@xxxxxxxxx> wrote:
>> Using 'awk' here is a bit of an overkill. Should the output of 'gcc
>> -dumpversion' vary on another disto, or change overtime, it may no longer
>> be available at field 1, as relied on by the current implementation.
>> I believe 'sed' offers greater flexibility here in terms of processing
>> varying output as well as being more light-weight.
>>
>> Tested on:
>> Gentoo Linux
>> Debian 6.0.10
>> Oracle Linux Server release 7.1
>> Arch Linux
>> openSuSE 13.2
>>
>> Signed-off-by: Alexander Kapshuk <alexander.kapshuk@xxxxxxxxx>
>> ---
>> --- linux/scripts/ver_linux.orig    2015-10-03 13:41:57.118790241 +0300
>> +++ linux/scripts/ver_linux    2015-10-03 13:48:49.277622632 +0300
>> @@ -11,8 +11,12 @@
>>  uname -a
>>  echo ' '
>>
>> -gcc -dumpversion 2>&1| awk \
>> -'NR==1{print "Gnu C                 ", $1}'
>> +test -x "$gcc" &&
>> +$gcc -dumpversion 2>&1 |
>> +sed '
>> +    /^[0-9\.]*$/!d
>> +    s/^/GNU C\t\t\t/
>> +'
>
> I'm not sure whether replacing a bog standard idiom with a non-trivial
> sed expression
> is a good idea.
> Your argument that currently the version has to be the first field is weak,
> your script has a even stronger assumption, it assumes that the
> version is field 1
> _and_ has too be numeric.
> But maybe I'm misinterpreting your regex, regex is hard to read for a human. :)
>
> IMHO we should keep things simple. It's not that ver_linux (and other
> scripts you patch) are hot paths
> in the kernel build process.
>
> --
> Thanks,
> //richard

Thanks for your feedback.

The main objective I endeavoured to attain was to come up with an
algorithm that would possibly result in a uniform output that would
work across as many distros as possbile. The current implementation
seems to struggle with that.

In my experience, 'sed' enables handling situations where the data
being looked for is located in varying places more gracefully.

For example, 'gcc -dumpversion', outputs its version in a
dot-separated numerical format. Thankfully, this format seems to be
uniform across all the distros I have been able to test it on. So in
this particular case, the original implementation works as expected.
However, should 'gcc -dumpversion' change its output in the future,
with some distros further modifying this output, so that the version
ends up in different fields, the original awk implementation would no
longer work. Of course, perhaps a more complex awk script could be
written to handle this. It just that with 'sed', in my view, it would
be a matter of adding/modifying the current patterns in a way that
would be accommodating to the changed format.

E.g.
'ld -v 2>&1' output on
Debian is:
GNU ld (GNU Binutils for Debian) 2.20.1-system.20100303

Oracle Linux:
GNU ld version 2.23.52.0.1-30.el7_1.2 20130226

Gentoo:
GNU ld (Gentoo 2.25.1 p1.1) 2.25.1

The binutils patch:
ld -v 2>&1 |
sed '
    /[0-9]$/!d    # applies to all 3 cases;
    s/-.*//          # applies to Debian/Oracle, but doesn't affect Gentoo;
    s/.*[ \t]//      # applies to all 3 cases;
    s/^/binutils\t\t/
'
So far, I have not been able to come up with an awk solution that
would work equally well across all three distros. My best take so far
has been:
Debian:
ld -v 2>&1 | awk -F'[ \t\-]+' '{print $(NF-1)}'
2.20.1

Oracle Linux:
ld -v 2>&1 | awk -F'[ \t\-]+' '{print $(NF-2)}'
2.23.52.0.1

Gentoo:
ld -v 2>&1 | awk '{print $NF}'
2.25.1

Which is far from being a uniform implementation.

I hope I am making sense here.

I have found the proposed implementation to work well in all the
distros I have had access to, so I thought I would share it with the
community. If the folk here have some suggestions to make, I am
willing to do my best to work in with them.

At the same time, I do understand that 'ver_linux' is not a tool that
is crucial to kernel development. On this token, I do not expect, nor
insist on the proposed implementation to be accepted. I leave it to
the discretion of the maintainers whether or not to accept any of the
patches.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/