[PATCH] einstalldocs: Fix test for DOCS being unset.

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] einstalldocs: Fix test for DOCS being unset.

Ulrich Müller
The current test does not exactly test for unset DOCS, because it also
evaluates as true if the variable has attributes. Such attributes can
be defined even for an unset variable.

Therefore test the output of declare -p for presence of an = sign
instead, which indicates that a value has been assigned to the
variable (bug 710076 comment #2).

PMS reference: Algorithm 12.4, line 7:
https://projects.gentoo.org/pms/7/pms.html#x1-135011r183

See also bash upstream discussion:
https://lists.gnu.org/archive/html/bug-bash/2020-02/msg00045.html

Closes: https://bugs.gentoo.org/710076
Signed-off-by: Ulrich Müller <[hidden email]>
---
 bin/phase-helpers.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bin/phase-helpers.sh b/bin/phase-helpers.sh
index 3deb28c68..9495465f9 100644
--- a/bin/phase-helpers.sh
+++ b/bin/phase-helpers.sh
@@ -1,5 +1,5 @@
 #!/bin/bash
-# Copyright 1999-2019 Gentoo Authors
+# Copyright 1999-2020 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
 if ___eapi_has_DESTTREE_INSDESTTREE; then
@@ -953,7 +953,7 @@ fi
 if ___eapi_has_einstalldocs; then
  einstalldocs() {
  (
- if ! declare -p DOCS &>/dev/null ; then
+ if [[ $(declare -p DOCS 2>/dev/null) != *=* ]]; then
  local d
  for d in README* ChangeLog AUTHORS NEWS TODO CHANGES \
  THANKS BUGS FAQ CREDITS CHANGELOG ; do
--
2.25.1

signature.asc (517 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] einstalldocs: Fix test for DOCS being unset.

Zac Medico-2
On 2/20/20 5:04 AM, Ulrich Müller wrote:

> The current test does not exactly test for unset DOCS, because it also
> evaluates as true if the variable has attributes. Such attributes can
> be defined even for an unset variable.
>
> Therefore test the output of declare -p for presence of an = sign
> instead, which indicates that a value has been assigned to the
> variable (bug 710076 comment #2).
>
> PMS reference: Algorithm 12.4, line 7:
> https://projects.gentoo.org/pms/7/pms.html#x1-135011r183
>
> See also bash upstream discussion:
> https://lists.gnu.org/archive/html/bug-bash/2020-02/msg00045.html
>
> Closes: https://bugs.gentoo.org/710076
> Signed-off-by: Ulrich Müller <[hidden email]>
> ---
>   bin/phase-helpers.sh | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/bin/phase-helpers.sh b/bin/phase-helpers.sh
> index 3deb28c68..9495465f9 100644
> --- a/bin/phase-helpers.sh
> +++ b/bin/phase-helpers.sh
> @@ -1,5 +1,5 @@
>   #!/bin/bash
> -# Copyright 1999-2019 Gentoo Authors
> +# Copyright 1999-2020 Gentoo Authors
>   # Distributed under the terms of the GNU General Public License v2
>  
>   if ___eapi_has_DESTTREE_INSDESTTREE; then
> @@ -953,7 +953,7 @@ fi
>   if ___eapi_has_einstalldocs; then
>   einstalldocs() {
>   (
> - if ! declare -p DOCS &>/dev/null ; then
> + if [[ $(declare -p DOCS 2>/dev/null) != *=* ]]; then
>   local d
>   for d in README* ChangeLog AUTHORS NEWS TODO CHANGES \
>   THANKS BUGS FAQ CREDITS CHANGELOG ; do
>
Looks good. Even though earlier EAPIs are deprecated, would there be a
problem with updating __eapi4_src_install for consistency?
--
Thanks,
Zac


signature.asc (1000 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] einstalldocs: Fix test for DOCS being unset.

Ulrich Müller
>>>>> On Thu, 20 Feb 2020, Zac Medico wrote:

> Looks good. Even though earlier EAPIs are deprecated, would there be a
> problem with updating __eapi4_src_install for consistency?

I'd rather not, because for EAPIs 4 and 5 PMS specifies that the return
status of declare -p should be tested [1].

Presumably it wouldn't make much of a difference, because we don't
account for the "assigned, but empty" case in these EAPIs.
src_install format 4 will just fail for DOCS="" or DOCS=().

[1] https://projects.gentoo.org/pms/7/pms.html#x1-94001r7

signature.asc (517 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] einstalldocs: Fix test for DOCS being unset.

Zac Medico-2
On 2/20/20 10:51 AM, Ulrich Mueller wrote:

>>>>>> On Thu, 20 Feb 2020, Zac Medico wrote:
>
>> Looks good. Even though earlier EAPIs are deprecated, would there be a
>> problem with updating __eapi4_src_install for consistency?
>
> I'd rather not, because for EAPIs 4 and 5 PMS specifies that the return
> status of declare -p should be tested [1].
>
> Presumably it wouldn't make much of a difference, because we don't
> account for the "assigned, but empty" case in these EAPIs.
> src_install format 4 will just fail for DOCS="" or DOCS=().
>
> [1] https://projects.gentoo.org/pms/7/pms.html#x1-94001r7
>
Ok, that sounds reasonable. Thanks for the explanation.

Please go ahead and merge.
--
Thanks,
Zac


signature.asc (1000 bytes) Download Attachment