Re: [gentoo-portage-dev] Fix all misc. bash errors.

2018-02-07 Thread R0b0t1 
On Wed, Feb 7, 2018 at 1:41 AM, Michał Górny  wrote:
> W dniu pon, 05.02.2018 o godzinie 20∶33 -0600, użytkownik R0b0t1
> napisał:
>> On Sun, Feb 4, 2018 at 10:45 PM, Zac Medico  wrote:
>> > On 02/04/2018 07:22 PM, R0b0t1 wrote:
>> > > This is everything that shellcheck reported as an error. They are not
>> > > as serious as the globbing issue, but it would be a good idea to
>> > > change them. They are generally "type" issues (e.g. ">" instead of
>> > > "-gt").
>> > >
>> > > Some changes are shellcheck annotations. Very interesting was:
>> > >
>> > > eval "$x=(\"\${$x[@]}\" ${QA_PREBUILT//\*/.*})"
>> > >
>> > > Which looks like a bad array expansion ("$x[@]").
>> >
>> > I don't see a shellcheck error for that, using shellcheck-0.4.7. Maybe a
>> > false positive with an older version?
>> >
>>
>> 0.4.6, I will see if I can check.
>>
>> > > diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
>> > > index b28e44f18..377b32d90 100644
>> > > --- a/bin/isolated-functions.sh
>> > > +++ b/bin/isolated-functions.sh
>> > > @@ -82,7 +82,7 @@ __dump_trace() {
>> > >  lineno=${BASH_LINENO[${n} - 1]}
>> > >  # Display function arguments
>> > >  args=
>> > > -if [[ -n "${BASH_ARGV[@]}" ]]; then
>> > > +if [[ -n "${BASH_ARGV[*]}" ]]; then
>> >
>> > I feel like the shellcheck authors might be willing to accept [[ -n
>> > ${BASH_ARGV[@]} ]] or [[ ${BASH_ARGV[@]} ]] as correct, since the
>> > "Problematic code" that they cite involves an incorrect comparison:
>> >
>> > https://github.com/koalaman/shellcheck/wiki/SC2199#problematic-code
>> >
>>
>> This example might be more illustrative:
>>
>> argc () {
>> echo $#
>> }
>>
>> argc "${BASH_ARGV[*]}"
>> argc "${BASH_ARGV[@]}"
>>
>>
>> Output (./test.sh a b):
>>
>> 1
>> 2
>>
>>
>> Which changes the semantics of the tests in which it is present. It is
>> hard to know what [[ is doing; if the same is attempted with [ it
>> would be an error for the same reason that globbing produces errors.
>> See:
>>
>> [  "${BASH_ARGV[*]}"  ]
>> [  "${BASH_ARGV[@]}"  ] # Fails; [: b: unary operator expected
>> [[ "${BASH_ARGV[*]}" ]]
>> [[ "${BASH_ARGV[@]}" ]]
>>
>>
>> It is possible [[ ignores the extra elements. I can't think of a
>> reason where this would make the results of the test different. At the
>> same time, it might give people the wrong impression of the operation
>> of [.
>>
>> In the sense that [ and [[ represent test(1), anything but the "[*]"
>> expansion is incorrect, as the error message indicates. That [[ treats
>> its arguments specially because it is a feature of the syntax just
>> makes the situation more confusing.
>>
>
> They don't. '[' works as a external command whose arguments are subject
> to word splitting. '[[' works as a builtin that does word splitting
> before expanding variables.
>

I'm aware of the differing implementations, but trying to claim [[ is
not modelled after [ is disingenuous. Needing to keep track of what
precisely each thing is doing and why adds to the difficulty of
maintaining the scripts. It'd be like trying to make use of all of the
features of C++.

Shellcheck reports an error because what was asked doesn't make sense
in context, even though it does happen to work.

Cheers,
 R0b0t1

Re: [gentoo-portage-dev] Fix all misc. bash errors.

2018-02-06 Thread Michał Górny 
W dniu pon, 05.02.2018 o godzinie 20∶33 -0600, użytkownik R0b0t1
napisał:
> On Sun, Feb 4, 2018 at 10:45 PM, Zac Medico  wrote:
> > On 02/04/2018 07:22 PM, R0b0t1 wrote:
> > > This is everything that shellcheck reported as an error. They are not
> > > as serious as the globbing issue, but it would be a good idea to
> > > change them. They are generally "type" issues (e.g. ">" instead of
> > > "-gt").
> > > 
> > > Some changes are shellcheck annotations. Very interesting was:
> > > 
> > > eval "$x=(\"\${$x[@]}\" ${QA_PREBUILT//\*/.*})"
> > > 
> > > Which looks like a bad array expansion ("$x[@]").
> > 
> > I don't see a shellcheck error for that, using shellcheck-0.4.7. Maybe a
> > false positive with an older version?
> > 
> 
> 0.4.6, I will see if I can check.
> 
> > > diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
> > > index b28e44f18..377b32d90 100644
> > > --- a/bin/isolated-functions.sh
> > > +++ b/bin/isolated-functions.sh
> > > @@ -82,7 +82,7 @@ __dump_trace() {
> > >  lineno=${BASH_LINENO[${n} - 1]}
> > >  # Display function arguments
> > >  args=
> > > -if [[ -n "${BASH_ARGV[@]}" ]]; then
> > > +if [[ -n "${BASH_ARGV[*]}" ]]; then
> > 
> > I feel like the shellcheck authors might be willing to accept [[ -n
> > ${BASH_ARGV[@]} ]] or [[ ${BASH_ARGV[@]} ]] as correct, since the
> > "Problematic code" that they cite involves an incorrect comparison:
> > 
> > https://github.com/koalaman/shellcheck/wiki/SC2199#problematic-code
> > 
> 
> This example might be more illustrative:
> 
> argc () {
> echo $#
> }
> 
> argc "${BASH_ARGV[*]}"
> argc "${BASH_ARGV[@]}"
> 
> 
> Output (./test.sh a b):
> 
> 1
> 2
> 
> 
> Which changes the semantics of the tests in which it is present. It is
> hard to know what [[ is doing; if the same is attempted with [ it
> would be an error for the same reason that globbing produces errors.
> See:
> 
> [  "${BASH_ARGV[*]}"  ]
> [  "${BASH_ARGV[@]}"  ] # Fails; [: b: unary operator expected
> [[ "${BASH_ARGV[*]}" ]]
> [[ "${BASH_ARGV[@]}" ]]
> 
> 
> It is possible [[ ignores the extra elements. I can't think of a
> reason where this would make the results of the test different. At the
> same time, it might give people the wrong impression of the operation
> of [.
> 
> In the sense that [ and [[ represent test(1), anything but the "[*]"
> expansion is incorrect, as the error message indicates. That [[ treats
> its arguments specially because it is a feature of the syntax just
> makes the situation more confusing.
> 

They don't. '[' works as a external command whose arguments are subject
to word splitting. '[[' works as a builtin that does word splitting
before expanding variables.

-- 
Best regards,
Michał Górny

Re: [gentoo-portage-dev] Fix all misc. bash errors.

2018-02-06 Thread Zac Medico 
On 02/05/2018 06:55 PM, R0b0t1 wrote:
> On Mon, Feb 5, 2018 at 8:33 PM, R0b0t1  wrote:
>> On Sun, Feb 4, 2018 at 10:45 PM, Zac Medico  wrote:
>>> On 02/04/2018 07:22 PM, R0b0t1 wrote:
 This is everything that shellcheck reported as an error. They are not
 as serious as the globbing issue, but it would be a good idea to
 change them. They are generally "type" issues (e.g. ">" instead of
 "-gt").

 Some changes are shellcheck annotations. Very interesting was:

 eval "$x=(\"\${$x[@]}\" ${QA_PREBUILT//\*/.*})"

 Which looks like a bad array expansion ("$x[@]").
>>>
>>> I don't see a shellcheck error for that, using shellcheck-0.4.7. Maybe a
>>> false positive with an older version?
>>>
>>
>> 0.4.6, I will see if I can check.
> 
> Still in 0.4.7.

Yeah, I just tried again and I'm seeing it now. I've found that adding
braces to ${x} suppresses it:

https://gitweb.gentoo.org/proj/portage.git/commit/?id=4a0a949d601969c672d9cf70ef8cf8682553f787

-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature

Re: [gentoo-portage-dev] Fix all misc. bash errors.

2018-02-06 Thread Zac Medico 
On 02/05/2018 06:33 PM, R0b0t1 wrote:
> On Sun, Feb 4, 2018 at 10:45 PM, Zac Medico  wrote:
>> On 02/04/2018 07:22 PM, R0b0t1 wrote:
>>> diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
>>> index b28e44f18..377b32d90 100644
>>> --- a/bin/isolated-functions.sh
>>> +++ b/bin/isolated-functions.sh
>>> @@ -82,7 +82,7 @@ __dump_trace() {
>>>  lineno=${BASH_LINENO[${n} - 1]}
>>>  # Display function arguments
>>>  args=
>>> -if [[ -n "${BASH_ARGV[@]}" ]]; then
>>> +if [[ -n "${BASH_ARGV[*]}" ]]; then
>>
>> I feel like the shellcheck authors might be willing to accept [[ -n
>> ${BASH_ARGV[@]} ]] or [[ ${BASH_ARGV[@]} ]] as correct, since the
>> "Problematic code" that they cite involves an incorrect comparison:
>>
>> https://github.com/koalaman/shellcheck/wiki/SC2199#problematic-code
>>
> 
> This example might be more illustrative:
> 
> argc () {
> echo $#
> }
> 
> argc "${BASH_ARGV[*]}"
> argc "${BASH_ARGV[@]}"
> 
> 
> Output (./test.sh a b):
> 
> 1
> 2
> 
> 
> Which changes the semantics of the tests in which it is present. It is
> hard to know what [[ is doing; if the same is attempted with [ it
> would be an error for the same reason that globbing produces errors.
> See:
> 
> [  "${BASH_ARGV[*]}"  ]
> [  "${BASH_ARGV[@]}"  ] # Fails; [: b: unary operator expected
> [[ "${BASH_ARGV[*]}" ]]
> [[ "${BASH_ARGV[@]}" ]]
> 
> 
> It is possible [[ ignores the extra elements. I can't think of a
> reason where this would make the results of the test different. At the
> same time, it might give people the wrong impression of the operation
> of [.
> 
> In the sense that [ and [[ represent test(1), anything but the "[*]"
> expansion is incorrect, as the error message indicates. That [[ treats
> its arguments specially because it is a feature of the syntax just
> makes the situation more confusing.

I've turned this into an optimization by testing the array length
instead of expanding the elements:

https://gitweb.gentoo.org/proj/portage.git/commit/?id=2306b8f4a2d781db87ee61707f6dea1c5f717936
-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature

Re: [gentoo-portage-dev] Fix all misc. bash errors.

2018-02-05 Thread R0b0t1 
On Mon, Feb 5, 2018 at 8:33 PM, R0b0t1  wrote:
> On Sun, Feb 4, 2018 at 10:45 PM, Zac Medico  wrote:
>> On 02/04/2018 07:22 PM, R0b0t1 wrote:
>>> This is everything that shellcheck reported as an error. They are not
>>> as serious as the globbing issue, but it would be a good idea to
>>> change them. They are generally "type" issues (e.g. ">" instead of
>>> "-gt").
>>>
>>> Some changes are shellcheck annotations. Very interesting was:
>>>
>>> eval "$x=(\"\${$x[@]}\" ${QA_PREBUILT//\*/.*})"
>>>
>>> Which looks like a bad array expansion ("$x[@]").
>>
>> I don't see a shellcheck error for that, using shellcheck-0.4.7. Maybe a
>> false positive with an older version?
>>
>
> 0.4.6, I will see if I can check.

Still in 0.4.7.

>
>>> diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
>>> index b28e44f18..377b32d90 100644
>>> --- a/bin/isolated-functions.sh
>>> +++ b/bin/isolated-functions.sh
>>> @@ -82,7 +82,7 @@ __dump_trace() {
>>>  lineno=${BASH_LINENO[${n} - 1]}
>>>  # Display function arguments
>>>  args=
>>> -if [[ -n "${BASH_ARGV[@]}" ]]; then
>>> +if [[ -n "${BASH_ARGV[*]}" ]]; then
>>
>> I feel like the shellcheck authors might be willing to accept [[ -n
>> ${BASH_ARGV[@]} ]] or [[ ${BASH_ARGV[@]} ]] as correct, since the
>> "Problematic code" that they cite involves an incorrect comparison:
>>
>> https://github.com/koalaman/shellcheck/wiki/SC2199#problematic-code
>>
>
> This example might be more illustrative:
>
> argc () {
> echo $#
> }
>
> argc "${BASH_ARGV[*]}"
> argc "${BASH_ARGV[@]}"
>
>
> Output (./test.sh a b):
>
> 1
> 2
>
>
> Which changes the semantics of the tests in which it is present. It is
> hard to know what [[ is doing; if the same is attempted with [ it
> would be an error for the same reason that globbing produces errors.
> See:
>
> [  "${BASH_ARGV[*]}"  ]
> [  "${BASH_ARGV[@]}"  ] # Fails; [: b: unary operator expected
> [[ "${BASH_ARGV[*]}" ]]
> [[ "${BASH_ARGV[@]}" ]]
>
>
> It is possible [[ ignores the extra elements. I can't think of a
> reason where this would make the results of the test different. At the
> same time, it might give people the wrong impression of the operation
> of [.
>
> In the sense that [ and [[ represent test(1), anything but the "[*]"
> expansion is incorrect, as the error message indicates. That [[ treats
> its arguments specially because it is a feature of the syntax just
> makes the situation more confusing.
>
> Cheers,
>  R0b0t1

Re: [gentoo-portage-dev] Fix all misc. bash errors.

2018-02-05 Thread R0b0t1 
On Sun, Feb 4, 2018 at 10:45 PM, Zac Medico  wrote:
> On 02/04/2018 07:22 PM, R0b0t1 wrote:
>> This is everything that shellcheck reported as an error. They are not
>> as serious as the globbing issue, but it would be a good idea to
>> change them. They are generally "type" issues (e.g. ">" instead of
>> "-gt").
>>
>> Some changes are shellcheck annotations. Very interesting was:
>>
>> eval "$x=(\"\${$x[@]}\" ${QA_PREBUILT//\*/.*})"
>>
>> Which looks like a bad array expansion ("$x[@]").
>
> I don't see a shellcheck error for that, using shellcheck-0.4.7. Maybe a
> false positive with an older version?
>

0.4.6, I will see if I can check.

>> diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
>> index b28e44f18..377b32d90 100644
>> --- a/bin/isolated-functions.sh
>> +++ b/bin/isolated-functions.sh
>> @@ -82,7 +82,7 @@ __dump_trace() {
>>  lineno=${BASH_LINENO[${n} - 1]}
>>  # Display function arguments
>>  args=
>> -if [[ -n "${BASH_ARGV[@]}" ]]; then
>> +if [[ -n "${BASH_ARGV[*]}" ]]; then
>
> I feel like the shellcheck authors might be willing to accept [[ -n
> ${BASH_ARGV[@]} ]] or [[ ${BASH_ARGV[@]} ]] as correct, since the
> "Problematic code" that they cite involves an incorrect comparison:
>
> https://github.com/koalaman/shellcheck/wiki/SC2199#problematic-code
>

This example might be more illustrative:

argc () {
echo $#
}

argc "${BASH_ARGV[*]}"
argc "${BASH_ARGV[@]}"


Output (./test.sh a b):

1
2


Which changes the semantics of the tests in which it is present. It is
hard to know what [[ is doing; if the same is attempted with [ it
would be an error for the same reason that globbing produces errors.
See:

[  "${BASH_ARGV[*]}"  ]
[  "${BASH_ARGV[@]}"  ] # Fails; [: b: unary operator expected
[[ "${BASH_ARGV[*]}" ]]
[[ "${BASH_ARGV[@]}" ]]


It is possible [[ ignores the extra elements. I can't think of a
reason where this would make the results of the test different. At the
same time, it might give people the wrong impression of the operation
of [.

In the sense that [ and [[ represent test(1), anything but the "[*]"
expansion is incorrect, as the error message indicates. That [[ treats
its arguments specially because it is a feature of the syntax just
makes the situation more confusing.

Cheers,
 R0b0t1

Re: [gentoo-portage-dev] Fix all misc. bash errors.

2018-02-04 Thread Zac Medico 
On 02/04/2018 07:22 PM, R0b0t1 wrote:
> This is everything that shellcheck reported as an error. They are not
> as serious as the globbing issue, but it would be a good idea to
> change them. They are generally "type" issues (e.g. ">" instead of
> "-gt").
> 
> Some changes are shellcheck annotations. Very interesting was:
> 
> eval "$x=(\"\${$x[@]}\" ${QA_PREBUILT//\*/.*})"
> 
> Which looks like a bad array expansion ("$x[@]").

I don't see a shellcheck error for that, using shellcheck-0.4.7. Maybe a
false positive with an older version?

> diff --git a/bin/isolated-functions.sh b/bin/isolated-functions.sh
> index b28e44f18..377b32d90 100644
> --- a/bin/isolated-functions.sh
> +++ b/bin/isolated-functions.sh
> @@ -82,7 +82,7 @@ __dump_trace() {
>  lineno=${BASH_LINENO[${n} - 1]}
>  # Display function arguments
>  args=
> -if [[ -n "${BASH_ARGV[@]}" ]]; then
> +if [[ -n "${BASH_ARGV[*]}" ]]; then

I feel like the shellcheck authors might be willing to accept [[ -n
${BASH_ARGV[@]} ]] or [[ ${BASH_ARGV[@]} ]] as correct, since the
"Problematic code" that they cite involves an incorrect comparison:

https://github.com/koalaman/shellcheck/wiki/SC2199#problematic-code

I've merged all of your other changes. Thanks!
-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature