public inbox for gentoo-portage-dev@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-portage-dev] Fix all misc. bash errors.
@ 2018-02-05  3:22 R0b0t1
  2018-02-05  4:45 ` Zac Medico
  0 siblings, 1 reply; 8+ messages in thread
From: R0b0t1 @ 2018-02-05  3:22 UTC (permalink / raw
  To: gentoo-portage-dev

[-- Attachment #1: Type: text/plain, Size: 4223 bytes --]

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[@]").

Cheers,
     R0b0t1


---
 bin/ebuild-helpers/newins | 2 +-
 bin/ebuild.sh             | 2 +-
 bin/etc-update            | 6 +++---
 bin/isolated-functions.sh | 6 +++---
 bin/phase-functions.sh    | 1 +
 5 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/bin/ebuild-helpers/newins b/bin/ebuild-helpers/newins
index 8032a8f2f..30e54b7e5 100755
--- a/bin/ebuild-helpers/newins
+++ b/bin/ebuild-helpers/newins
@@ -12,7 +12,7 @@ if [[ -z ${T} ]] || [[ -z ${2} ]] ; then
 fi

 (($#>2)) && \
-    eqawarn "QA Notice: ${helper} called with more than 2 arguments: ${@:3}"
+    eqawarn "QA Notice: ${helper} called with more than 2 arguments: ${*:3}"

 stdin=
 if ___eapi_newins_supports_reading_from_standard_input && [[ $1 == "-" ]]; then
diff --git a/bin/ebuild.sh b/bin/ebuild.sh
index 94a44d534..4a80fdd06 100755
--- a/bin/ebuild.sh
+++ b/bin/ebuild.sh
@@ -232,7 +232,7 @@ debug-print-section() {
 declare -ix ECLASS_DEPTH=0
 inherit() {
     ECLASS_DEPTH=$(($ECLASS_DEPTH + 1))
-    if [[ ${ECLASS_DEPTH} > 1 ]]; then
+    if [[ ${ECLASS_DEPTH} -gt 1 ]]; then
         debug-print "*** Multiple Inheritence (Level: ${ECLASS_DEPTH})"
     fi

diff --git a/bin/etc-update b/bin/etc-update
index ea69f1478..cbca8217e 100755
--- a/bin/etc-update
+++ b/bin/etc-update
@@ -767,7 +767,7 @@ portage_vars=(
 )

 if type -P portageq > /dev/null; then
-    eval $(${PORTAGE_PYTHON:+"${PORTAGE_PYTHON}"} "$(type -P
portageq)" envvar -v ${portage_vars[@]})
+    eval $(${PORTAGE_PYTHON:+"${PORTAGE_PYTHON}"} "$(type -P
portageq)" envvar -v "${portage_vars[@]}")
 else
     [[ $OS_FAMILY == 'gentoo' ]] && die "missing portageq"
 fi
@@ -801,7 +801,7 @@ cfg_vars=(
     mode
 )
 # default them all to ""
-eval ${cfg_vars[@]/%/=}
+eval "${cfg_vars[@]/%/=}"
 # then extract them all from the conf in one shot
 # (ugly var at end is due to printf appending a '|' to last item)
 get_config "($(printf '%s|' "${cfg_vars[@]}")NOVARFOROLDMEN)"
@@ -846,7 +846,7 @@ if ${NONINTERACTIVE_MV} ; then
 fi

 if ${VERBOSE} ; then
-    for v in ${portage_vars[@]} ${cfg_vars[@]} TMP SCAN_PATHS ; do
+    for v in "${portage_vars[@]} ${cfg_vars[@]} TMP SCAN_PATHS" ; do
         echo "${v}=${!v}"
     done
 fi
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
             for (( j = 1 ; j <= ${BASH_ARGC[${n} - 1]} ; ++j )); do
                 newarg=${BASH_ARGV[$(( p - j - 1 ))]}
                 args="${args:+${args} }'${newarg}'"
@@ -550,13 +550,13 @@ __eqatag() {

     (
         echo "- tag: ${tag}"
-        if [[ ${data[@]} ]]; then
+        if [[ ${data[*]} ]]; then
             echo "  data:"
             for i in "${data[@]}"; do
                 echo "    ${i%%=*}: \"$(__eqaquote "${i#*=}")\""
             done
         fi
-        if [[ ${filenames[@]} ]]; then
+        if [[ ${filenames[*]} ]]; then
             echo "  files:"
             for i in "${filenames[@]}"; do
                 echo "    - \"$(__eqaquote "${i}")\""
diff --git a/bin/phase-functions.sh b/bin/phase-functions.sh
index 10d54ca74..469715f7f 100644
--- a/bin/phase-functions.sh
+++ b/bin/phase-functions.sh
@@ -541,6 +541,7 @@ __dyn_install() {
         # fnmatch patterns to regular expressions
         for x in QA_DT_NEEDED QA_FLAGS_IGNORED QA_PRESTRIPPED QA_SONAME ; do
             if [[ $(declare -p $x 2>/dev/null) = declare\ -a* ]] ; then
+                # shellcheck disable=1087
                 eval "$x=(\"\${$x[@]}\" ${QA_PREBUILT//\*/.*})"
             else
                 eval "$x+=\" ${QA_PREBUILT//\*/.*}\""
-- 
2.14.1

[-- Attachment #2: fix-misc-bash.patch --]
[-- Type: text/x-patch, Size: 3732 bytes --]

From 2111073648d3e33f49ceaaca971f8cff9e5e6b0e Mon Sep 17 00:00:00 2001
From: Sid Spry <R030t1@gmail.com>
Date: Sun, 4 Feb 2018 19:11:32 -0800
Subject: [PATCH] Fix all misc. bash errors.

---
 bin/ebuild-helpers/newins | 2 +-
 bin/ebuild.sh             | 2 +-
 bin/etc-update            | 6 +++---
 bin/isolated-functions.sh | 6 +++---
 bin/phase-functions.sh    | 1 +
 5 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/bin/ebuild-helpers/newins b/bin/ebuild-helpers/newins
index 8032a8f2f..30e54b7e5 100755
--- a/bin/ebuild-helpers/newins
+++ b/bin/ebuild-helpers/newins
@@ -12,7 +12,7 @@ if [[ -z ${T} ]] || [[ -z ${2} ]] ; then
 fi
 
 (($#>2)) && \
-	eqawarn "QA Notice: ${helper} called with more than 2 arguments: ${@:3}"
+	eqawarn "QA Notice: ${helper} called with more than 2 arguments: ${*:3}"
 
 stdin=
 if ___eapi_newins_supports_reading_from_standard_input && [[ $1 == "-" ]]; then
diff --git a/bin/ebuild.sh b/bin/ebuild.sh
index 94a44d534..4a80fdd06 100755
--- a/bin/ebuild.sh
+++ b/bin/ebuild.sh
@@ -232,7 +232,7 @@ debug-print-section() {
 declare -ix ECLASS_DEPTH=0
 inherit() {
 	ECLASS_DEPTH=$(($ECLASS_DEPTH + 1))
-	if [[ ${ECLASS_DEPTH} > 1 ]]; then
+	if [[ ${ECLASS_DEPTH} -gt 1 ]]; then
 		debug-print "*** Multiple Inheritence (Level: ${ECLASS_DEPTH})"
 	fi
 
diff --git a/bin/etc-update b/bin/etc-update
index ea69f1478..cbca8217e 100755
--- a/bin/etc-update
+++ b/bin/etc-update
@@ -767,7 +767,7 @@ portage_vars=(
 )
 
 if type -P portageq > /dev/null; then
-	eval $(${PORTAGE_PYTHON:+"${PORTAGE_PYTHON}"} "$(type -P portageq)" envvar -v ${portage_vars[@]})
+	eval $(${PORTAGE_PYTHON:+"${PORTAGE_PYTHON}"} "$(type -P portageq)" envvar -v "${portage_vars[@]}")
 else
 	[[ $OS_FAMILY == 'gentoo' ]] && die "missing portageq"
 fi
@@ -801,7 +801,7 @@ cfg_vars=(
 	mode
 )
 # default them all to ""
-eval ${cfg_vars[@]/%/=}
+eval "${cfg_vars[@]/%/=}"
 # then extract them all from the conf in one shot
 # (ugly var at end is due to printf appending a '|' to last item)
 get_config "($(printf '%s|' "${cfg_vars[@]}")NOVARFOROLDMEN)"
@@ -846,7 +846,7 @@ if ${NONINTERACTIVE_MV} ; then
 fi
 
 if ${VERBOSE} ; then
-	for v in ${portage_vars[@]} ${cfg_vars[@]} TMP SCAN_PATHS ; do
+	for v in "${portage_vars[@]} ${cfg_vars[@]} TMP SCAN_PATHS" ; do
 		echo "${v}=${!v}"
 	done
 fi
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
 			for (( j = 1 ; j <= ${BASH_ARGC[${n} - 1]} ; ++j )); do
 				newarg=${BASH_ARGV[$(( p - j - 1 ))]}
 				args="${args:+${args} }'${newarg}'"
@@ -550,13 +550,13 @@ __eqatag() {
 
 	(
 		echo "- tag: ${tag}"
-		if [[ ${data[@]} ]]; then
+		if [[ ${data[*]} ]]; then
 			echo "  data:"
 			for i in "${data[@]}"; do
 				echo "    ${i%%=*}: \"$(__eqaquote "${i#*=}")\""
 			done
 		fi
-		if [[ ${filenames[@]} ]]; then
+		if [[ ${filenames[*]} ]]; then
 			echo "  files:"
 			for i in "${filenames[@]}"; do
 				echo "    - \"$(__eqaquote "${i}")\""
diff --git a/bin/phase-functions.sh b/bin/phase-functions.sh
index 10d54ca74..469715f7f 100644
--- a/bin/phase-functions.sh
+++ b/bin/phase-functions.sh
@@ -541,6 +541,7 @@ __dyn_install() {
 		# fnmatch patterns to regular expressions
 		for x in QA_DT_NEEDED QA_FLAGS_IGNORED QA_PRESTRIPPED QA_SONAME ; do
 			if [[ $(declare -p $x 2>/dev/null) = declare\ -a* ]] ; then
+				# shellcheck disable=1087
 				eval "$x=(\"\${$x[@]}\" ${QA_PREBUILT//\*/.*})"
 			else
 				eval "$x+=\" ${QA_PREBUILT//\*/.*}\""
-- 
2.14.1


[-- Attachment #3: portcheck.tar.xz --]
[-- Type: application/x-xz, Size: 11748 bytes --]

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [gentoo-portage-dev] Fix all misc. bash errors.
  2018-02-05  3:22 [gentoo-portage-dev] Fix all misc. bash errors R0b0t1
@ 2018-02-05  4:45 ` Zac Medico
  2018-02-06  2:33   ` R0b0t1
  0 siblings, 1 reply; 8+ messages in thread
From: Zac Medico @ 2018-02-05  4:45 UTC (permalink / raw
  To: gentoo-portage-dev, R0b0t1


[-- Attachment #1.1: Type: text/plain, Size: 1323 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 224 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [gentoo-portage-dev] Fix all misc. bash errors.
  2018-02-05  4:45 ` Zac Medico
@ 2018-02-06  2:33   ` R0b0t1
  2018-02-06  2:55     ` R0b0t1
                       ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: R0b0t1 @ 2018-02-06  2:33 UTC (permalink / raw
  To: Zac Medico; +Cc: gentoo-portage-dev

On Sun, Feb 4, 2018 at 10:45 PM, Zac Medico <zmedico@gentoo.org> 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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [gentoo-portage-dev] Fix all misc. bash errors.
  2018-02-06  2:33   ` R0b0t1
@ 2018-02-06  2:55     ` R0b0t1
  2018-02-07  5:26       ` Zac Medico
  2018-02-07  5:13     ` Zac Medico
  2018-02-07  7:41     ` Michał Górny
  2 siblings, 1 reply; 8+ messages in thread
From: R0b0t1 @ 2018-02-06  2:55 UTC (permalink / raw
  To: Zac Medico; +Cc: gentoo-portage-dev

On Mon, Feb 5, 2018 at 8:33 PM, R0b0t1 <r030t1@gmail.com> wrote:
> On Sun, Feb 4, 2018 at 10:45 PM, Zac Medico <zmedico@gentoo.org> 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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [gentoo-portage-dev] Fix all misc. bash errors.
  2018-02-06  2:33   ` R0b0t1
  2018-02-06  2:55     ` R0b0t1
@ 2018-02-07  5:13     ` Zac Medico
  2018-02-07  7:41     ` Michał Górny
  2 siblings, 0 replies; 8+ messages in thread
From: Zac Medico @ 2018-02-07  5:13 UTC (permalink / raw
  To: gentoo-portage-dev, R0b0t1, Zac Medico


[-- Attachment #1.1: Type: text/plain, Size: 2175 bytes --]

On 02/05/2018 06:33 PM, R0b0t1 wrote:
> On Sun, Feb 4, 2018 at 10:45 PM, Zac Medico <zmedico@gentoo.org> 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 224 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [gentoo-portage-dev] Fix all misc. bash errors.
  2018-02-06  2:55     ` R0b0t1
@ 2018-02-07  5:26       ` Zac Medico
  0 siblings, 0 replies; 8+ messages in thread
From: Zac Medico @ 2018-02-07  5:26 UTC (permalink / raw
  To: gentoo-portage-dev, R0b0t1, Zac Medico


[-- Attachment #1.1: Type: text/plain, Size: 1079 bytes --]

On 02/05/2018 06:55 PM, R0b0t1 wrote:
> On Mon, Feb 5, 2018 at 8:33 PM, R0b0t1 <r030t1@gmail.com> wrote:
>> On Sun, Feb 4, 2018 at 10:45 PM, Zac Medico <zmedico@gentoo.org> 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 224 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [gentoo-portage-dev] Fix all misc. bash errors.
  2018-02-06  2:33   ` R0b0t1
  2018-02-06  2:55     ` R0b0t1
  2018-02-07  5:13     ` Zac Medico
@ 2018-02-07  7:41     ` Michał Górny
  2018-02-08  4:33       ` R0b0t1
  2 siblings, 1 reply; 8+ messages in thread
From: Michał Górny @ 2018-02-07  7:41 UTC (permalink / raw
  To: gentoo-portage-dev, Zac Medico

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 <zmedico@gentoo.org> 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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [gentoo-portage-dev] Fix all misc. bash errors.
  2018-02-07  7:41     ` Michał Górny
@ 2018-02-08  4:33       ` R0b0t1
  0 siblings, 0 replies; 8+ messages in thread
From: R0b0t1 @ 2018-02-08  4:33 UTC (permalink / raw
  To: gentoo-portage-dev; +Cc: Zac Medico

On Wed, Feb 7, 2018 at 1:41 AM, Michał Górny <mgorny@gentoo.org> 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 <zmedico@gentoo.org> 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


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-02-08  4:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-05  3:22 [gentoo-portage-dev] Fix all misc. bash errors R0b0t1
2018-02-05  4:45 ` Zac Medico
2018-02-06  2:33   ` R0b0t1
2018-02-06  2:55     ` R0b0t1
2018-02-07  5:26       ` Zac Medico
2018-02-07  5:13     ` Zac Medico
2018-02-07  7:41     ` Michał Górny
2018-02-08  4:33       ` R0b0t1

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox