public inbox for gentoo-dev@lists.gentoo.org
 help / color / mirror / Atom feed
* [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
@ 2015-12-19 17:18 Ulrich Mueller
  2015-12-19 17:32 ` Michał Górny
  0 siblings, 1 reply; 18+ messages in thread
From: Ulrich Mueller @ 2015-12-19 17:18 UTC (permalink / raw
  To: gentoo-dev; +Cc: Pacho Ramos

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

Here is a change to readme.gentoo-r1.eclass, as discussed with pacho.
The eclass currently inherits eutils for shell flag saving, which
seems like overkill (especially in EAPI 6). Replace this by a more
lightweight approach.

Ulrich


From b1f6de1e005d1cbcc9f2cfff281ad7d1d176fd31 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
Date: Sat, 19 Dec 2015 15:15:05 +0100
Subject: [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.

This was only needed in readme.gentoo_create_doc() for a single call
of eshopts_{push,pop}. Replace it by saving the set of options in a
variable. Die if writing the temp file in readme.gentoo_create_doc()
fails.
---
 eclass/readme.gentoo-r1.eclass | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/eclass/readme.gentoo-r1.eclass b/eclass/readme.gentoo-r1.eclass
index c076650..07320c0 100644
--- a/eclass/readme.gentoo-r1.eclass
+++ b/eclass/readme.gentoo-r1.eclass
@@ -21,8 +21,6 @@
 if [[ -z ${_README_GENTOO_ECLASS} ]]; then
 _README_GENTOO_ECLASS=1
 
-inherit eutils
-
 case "${EAPI:-0}" in
 	0|1|2|3)
 		die "Unsupported EAPI=${EAPI:-0} (too old) for ${ECLASS}"
@@ -61,15 +59,16 @@ readme.gentoo_create_doc() {
 	debug-print-function ${FUNCNAME} "${@}"
 
 	if [[ -n "${DOC_CONTENTS}" ]]; then
-		eshopts_push
-		set -f
 		if [[ -n "${DISABLE_AUTOFORMATTING}" ]]; then
-			echo "${DOC_CONTENTS}" > "${T}"/README.gentoo
+			echo "${DOC_CONTENTS}" > "${T}"/README.gentoo || die
 		else
+			local saved_flags=$-
+			set -f				# disable filename expansion in echo arguments
 			echo -e ${DOC_CONTENTS} | fold -s -w 70 \
 				| sed 's/[[:space:]]*$//' > "${T}"/README.gentoo
+			assert
+			set +f -${saved_flags}
 		fi
-		eshopts_pop
 	elif [[ -f "${FILESDIR}/README.gentoo-${SLOT%/*}" ]]; then
 		cp "${FILESDIR}/README.gentoo-${SLOT%/*}" "${T}"/README.gentoo || die
 	elif [[ -f "${FILESDIR}/README.gentoo${README_GENTOO_SUFFIX}" ]]; then
-- 
2.6.4

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

* Re: [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-19 17:18 [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils Ulrich Mueller
@ 2015-12-19 17:32 ` Michał Górny
  2015-12-19 18:05   ` [gentoo-dev] " Martin Vaeth
  2015-12-19 18:24   ` [gentoo-dev] " Ulrich Mueller
  0 siblings, 2 replies; 18+ messages in thread
From: Michał Górny @ 2015-12-19 17:32 UTC (permalink / raw
  To: Ulrich Mueller; +Cc: gentoo-dev, Pacho Ramos

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

On Sat, 19 Dec 2015 18:18:43 +0100
Ulrich Mueller <ulm@gentoo.org> wrote:

> Here is a change to readme.gentoo-r1.eclass, as discussed with pacho.
> The eclass currently inherits eutils for shell flag saving, which
> seems like overkill (especially in EAPI 6). Replace this by a more
> lightweight approach.
> 
> Ulrich
> 
> 
> From b1f6de1e005d1cbcc9f2cfff281ad7d1d176fd31 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
> Date: Sat, 19 Dec 2015 15:15:05 +0100
> Subject: [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
> 
> This was only needed in readme.gentoo_create_doc() for a single call
> of eshopts_{push,pop}. Replace it by saving the set of options in a
> variable. Die if writing the temp file in readme.gentoo_create_doc()
> fails.
> ---
>  eclass/readme.gentoo-r1.eclass | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/eclass/readme.gentoo-r1.eclass b/eclass/readme.gentoo-r1.eclass
> index c076650..07320c0 100644
> --- a/eclass/readme.gentoo-r1.eclass
> +++ b/eclass/readme.gentoo-r1.eclass
> @@ -21,8 +21,6 @@
>  if [[ -z ${_README_GENTOO_ECLASS} ]]; then
>  _README_GENTOO_ECLASS=1
>  
> -inherit eutils
> -
>  case "${EAPI:-0}" in
>  	0|1|2|3)
>  		die "Unsupported EAPI=${EAPI:-0} (too old) for ${ECLASS}"
> @@ -61,15 +59,16 @@ readme.gentoo_create_doc() {
>  	debug-print-function ${FUNCNAME} "${@}"
>  
>  	if [[ -n "${DOC_CONTENTS}" ]]; then
> -		eshopts_push
> -		set -f
>  		if [[ -n "${DISABLE_AUTOFORMATTING}" ]]; then
> -			echo "${DOC_CONTENTS}" > "${T}"/README.gentoo
> +			echo "${DOC_CONTENTS}" > "${T}"/README.gentoo || die
>  		else
> +			local saved_flags=$-
> +			set -f				# disable filename expansion in echo arguments
>  			echo -e ${DOC_CONTENTS} | fold -s -w 70 \
>  				| sed 's/[[:space:]]*$//' > "${T}"/README.gentoo

Maybe I'm missing something but is there any reason not to quote
"${DOC_CONTENTS}" instead of working around the results of not quoting
it?

> +			assert
> +			set +f -${saved_flags}
>  		fi
> -		eshopts_pop
>  	elif [[ -f "${FILESDIR}/README.gentoo-${SLOT%/*}" ]]; then
>  		cp "${FILESDIR}/README.gentoo-${SLOT%/*}" "${T}"/README.gentoo || die
>  	elif [[ -f "${FILESDIR}/README.gentoo${README_GENTOO_SUFFIX}" ]]; then



-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

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

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

* [gentoo-dev] Re: [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-19 17:32 ` Michał Górny
@ 2015-12-19 18:05   ` Martin Vaeth
  2015-12-19 18:24   ` [gentoo-dev] " Ulrich Mueller
  1 sibling, 0 replies; 18+ messages in thread
From: Martin Vaeth @ 2015-12-19 18:05 UTC (permalink / raw
  To: gentoo-dev

Michał Górny <mgorny@gentoo.org> wrote:
>>	if [[ -n "${DISABLE_AUTOFORMATTING}" ]]; then
>>		echo "${DOC_CONTENTS}" > "${T}"/README.gentoo || die
>>	else
>>		echo -e ${DOC_CONTENTS} | fold -s -w 70 \
>>		| sed 's/[[:space:]]*$//' > "${T}"/README.gentoo

(omitted some lines/space for better context):

> Maybe I'm missing something but is there any reason not to quote
> "${DOC_CONTENTS}" instead of working around the results of not quoting
> it?

With DISABLE_AUTOFORMATTING=true, it is correctly quoted,
and there is no "workaround".
Without DISABLE_AUTOFORMATTING, the autowrapping (and ignoring
of "manual" format information by not quoting) is obviously
intentional.
Maybe, it would be more natural to have the opposite as the default,
but this is how the eclass works from its very beginning.



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

* Re: [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-19 17:32 ` Michał Górny
  2015-12-19 18:05   ` [gentoo-dev] " Martin Vaeth
@ 2015-12-19 18:24   ` Ulrich Mueller
  2015-12-19 18:34     ` Michał Górny
  1 sibling, 1 reply; 18+ messages in thread
From: Ulrich Mueller @ 2015-12-19 18:24 UTC (permalink / raw
  To: Michał Górny; +Cc: gentoo-dev, Pacho Ramos

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

>>>>> On Sat, 19 Dec 2015, Michał Górny wrote:

>> +			set -f				# disable filename expansion in echo arguments
>>  			echo -e ${DOC_CONTENTS} | fold -s -w 70 \
>>  				| sed 's/[[:space:]]*$//' > "${T}"/README.gentoo

> Maybe I'm missing something but is there any reason not to quote
> "${DOC_CONTENTS}" instead of working around the results of not
> quoting it?

IIUC the intention of not quoting it is to normalise whitespace,
especially embedded newlines. Unfortunately, I don't see a good way to
do this differently. Adding another processing step after echo -e is
too late, because newlines expanded from \n must be kept.

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

* Re: [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-19 18:24   ` [gentoo-dev] " Ulrich Mueller
@ 2015-12-19 18:34     ` Michał Górny
  2015-12-19 20:17       ` Ulrich Mueller
  0 siblings, 1 reply; 18+ messages in thread
From: Michał Górny @ 2015-12-19 18:34 UTC (permalink / raw
  To: Ulrich Mueller; +Cc: gentoo-dev, Pacho Ramos

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

On Sat, 19 Dec 2015 19:24:17 +0100
Ulrich Mueller <ulm@gentoo.org> wrote:

> >>>>> On Sat, 19 Dec 2015, Michał Górny wrote:  
> 
> >> +			set -f				# disable filename expansion in echo arguments
> >>  			echo -e ${DOC_CONTENTS} | fold -s -w 70 \
> >>  				| sed 's/[[:space:]]*$//' > "${T}"/README.gentoo  
> 
> > Maybe I'm missing something but is there any reason not to quote
> > "${DOC_CONTENTS}" instead of working around the results of not
> > quoting it?  
> 
> IIUC the intention of not quoting it is to normalise whitespace,
> especially embedded newlines. Unfortunately, I don't see a good way to
> do this differently. Adding another processing step after echo -e is
> too late, because newlines expanded from \n must be kept.

  read -d '' -r -a DOC_CONTENTS <<<"${DOC_CONTENTS}"

This performs word splitting into an array. Then you can do:

  echo -e "${DOC_CONTENTS[*]}" | ...

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

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

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

* Re: [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-19 18:34     ` Michał Górny
@ 2015-12-19 20:17       ` Ulrich Mueller
  2015-12-19 20:52         ` Michał Górny
  0 siblings, 1 reply; 18+ messages in thread
From: Ulrich Mueller @ 2015-12-19 20:17 UTC (permalink / raw
  To: Michał Górny; +Cc: gentoo-dev, Pacho Ramos

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

>>>>> On Sat, 19 Dec 2015, Michał Górny wrote:

>   read -d '' -r -a DOC_CONTENTS <<<"${DOC_CONTENTS}"

> This performs word splitting into an array. Then you can do:

>   echo -e "${DOC_CONTENTS[*]}" | ...

The first line fails for me:

$ DOC_CONTENTS="test"
$ read -d '' -r -a DOC_CONTENTS <<<"${DOC_CONTENTS}"
$ echo $?
1

(I guess it is encountering EOF on the temporary file created by the
string redirection, but how would one distinguish this from other
errors?)

Besides, it is hard to understand what this code does, as compared to
the "set -f" solution.

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

* Re: [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-19 20:17       ` Ulrich Mueller
@ 2015-12-19 20:52         ` Michał Górny
  2015-12-19 22:51           ` Ulrich Mueller
  0 siblings, 1 reply; 18+ messages in thread
From: Michał Górny @ 2015-12-19 20:52 UTC (permalink / raw
  To: Ulrich Mueller; +Cc: gentoo-dev, Pacho Ramos

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

On Sat, 19 Dec 2015 21:17:26 +0100
Ulrich Mueller <ulm@gentoo.org> wrote:

> >>>>> On Sat, 19 Dec 2015, Michał Górny wrote:  
> 
> >   read -d '' -r -a DOC_CONTENTS <<<"${DOC_CONTENTS}"  
> 
> > This performs word splitting into an array. Then you can do:  
> 
> >   echo -e "${DOC_CONTENTS[*]}" | ...  
> 
> The first line fails for me:
> 
> $ DOC_CONTENTS="test"
> $ read -d '' -r -a DOC_CONTENTS <<<"${DOC_CONTENTS}"
> $ echo $?
> 1
> 
> (I guess it is encountering EOF on the temporary file created by the
> string redirection, but how would one distinguish this from other
> errors?)

read's return code indicates whether if found a full line (with
a newline). read can't really fail.

> Besides, it is hard to understand what this code does, as compared to
> the "set -f" solution.

Many pieces of good code are harder to understand than cheap, ugly
hacks. That's why those hacks are so common, and people meet them all
the time and never learn good code.

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

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

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

* Re: [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-19 20:52         ` Michał Górny
@ 2015-12-19 22:51           ` Ulrich Mueller
  2015-12-19 22:56             ` Michał Górny
  0 siblings, 1 reply; 18+ messages in thread
From: Ulrich Mueller @ 2015-12-19 22:51 UTC (permalink / raw
  To: gentoo-dev; +Cc: Pacho Ramos

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

>>>>> On Sat, 19 Dec 2015, Michał Górny wrote:

>> (I guess it is encountering EOF on the temporary file created by
>> the string redirection, but how would one distinguish this from
>> other errors?)

> read's return code indicates whether if found a full line (with a
> newline). read can't really fail.

Certainly writing or reading the temp file can fail?

>> Besides, it is hard to understand what this code does, as compared
>> to the "set -f" solution.

> Many pieces of good code are harder to understand than cheap, ugly
> hacks. That's why those hacks are so common, and people meet them
> all the time and never learn good code.

Well, compare:

    set -f
    echo -e ${DOC_CONTENTS} | ...

versus:

    read -d '' -r -a DOC_CONTENTS <<<"${DOC_CONTENTS}"
    echo -e "${DOC_CONTENTS[*]}" | ...

The second one is (IMHO) harder to understand, less efficient, and
relies on undocumented behaviour.

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

* Re: [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-19 22:51           ` Ulrich Mueller
@ 2015-12-19 22:56             ` Michał Górny
  2015-12-19 23:47               ` [gentoo-dev] " Jonathan Callen
  2015-12-19 23:56               ` [gentoo-dev] " Ulrich Mueller
  0 siblings, 2 replies; 18+ messages in thread
From: Michał Górny @ 2015-12-19 22:56 UTC (permalink / raw
  To: Ulrich Mueller; +Cc: gentoo-dev, Pacho Ramos

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

On Sat, 19 Dec 2015 23:51:47 +0100
Ulrich Mueller <ulm@gentoo.org> wrote:

> >>>>> On Sat, 19 Dec 2015, Michał Górny wrote:  
> 
> >> (I guess it is encountering EOF on the temporary file created by
> >> the string redirection, but how would one distinguish this from
> >> other errors?)  
> 
> > read's return code indicates whether if found a full line (with a
> > newline). read can't really fail.  
> 
> Certainly writing or reading the temp file can fail?
> 
> >> Besides, it is hard to understand what this code does, as compared
> >> to the "set -f" solution.  
> 
> > Many pieces of good code are harder to understand than cheap, ugly
> > hacks. That's why those hacks are so common, and people meet them
> > all the time and never learn good code.  
> 
> Well, compare:
> 
>     set -f
>     echo -e ${DOC_CONTENTS} | ...
> 
> versus:
> 
>     read -d '' -r -a DOC_CONTENTS <<<"${DOC_CONTENTS}"
>     echo -e "${DOC_CONTENTS[*]}" | ...
> 
> The second one is (IMHO) harder to understand, less efficient, and
> relies on undocumented behaviour.

On WHAT?!

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

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

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

* [gentoo-dev] Re: [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-19 22:56             ` Michał Górny
@ 2015-12-19 23:47               ` Jonathan Callen
  2015-12-20 10:17                 ` Andrew Savchenko
  2015-12-19 23:56               ` [gentoo-dev] " Ulrich Mueller
  1 sibling, 1 reply; 18+ messages in thread
From: Jonathan Callen @ 2015-12-19 23:47 UTC (permalink / raw
  To: gentoo-dev

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 12/19/2015 05:56 PM, Michał Górny wrote:
> On Sat, 19 Dec 2015 23:51:47 +0100 Ulrich Mueller <ulm@gentoo.org>
> wrote:
> 
>>>>>>> On Sat, 19 Dec 2015, Michał Górny wrote:
>> 
>>>> (I guess it is encountering EOF on the temporary file created
>>>> by the string redirection, but how would one distinguish this
>>>> from other errors?)
>> 
>>> read's return code indicates whether if found a full line (with
>>> a newline). read can't really fail.
>> 
>> Certainly writing or reading the temp file can fail?
>> 
>>>> Besides, it is hard to understand what this code does, as
>>>> compared to the "set -f" solution.
>> 
>>> Many pieces of good code are harder to understand than cheap,
>>> ugly hacks. That's why those hacks are so common, and people
>>> meet them all the time and never learn good code.
>> 
>> Well, compare:
>> 
>> set -f echo -e ${DOC_CONTENTS} | ...
>> 
>> versus:
>> 
>> read -d '' -r -a DOC_CONTENTS <<<"${DOC_CONTENTS}" echo -e
>> "${DOC_CONTENTS[*]}" | ...
>> 
>> The second one is (IMHO) harder to understand, less efficient,
>> and relies on undocumented behaviour.
> 
> On WHAT?!
> 

Apparently, bash(1) and bash.info don't document what it means to pass
an empty string as the argument to the -d option of the read builtin
(what actually happens is that in all cases, bash uses the first
character of the C string that the option gets translated to, which is
'\0' for the empty string; the documentation just refers to "the first
character of the string passed").

- -- 
Jonathan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCgAGBQJWdeydAAoJEEIQbvYRB3mgnQ4P+gP6WHxcyb6PZaBo8EG3nj/i
I5mbIs4XSsp4G13SVm54nTQelPzDlDLtMsRqdAj5Oh+oXrCyLLFcUKSpUf/1laNG
z7pzY6FStgFm1MZWAajtw16xr1+tAbkmGbGy/6pYvSVu7yNiI1n/P+2tBij6e6rN
cOeLtqRGfb1M0Ew3Py2qKVmWYf+/YxH6AkwFZGBZRcb+q3lCSZ9Ycbk3GRBStZwG
wi3PAf6nnuEaouuk7GA33Fi35cmHs8MISVIC70ULakOTu51xHm7aVJCzZQQIFl9N
WvH04+IMCcX+BleU6mP4LK0NB647WwKR8JWgcl8qj2415RvvImCtE35bhnpJbf92
PcVP1l4NJzbUUyfXMDZBpvxzXMHTz2DA4b9PS5Mq8jhiA1jKqAWFD5BBG05SHyrA
IpD0CKipLUFsXyJ5z/MS0gmUFFwymhB+dBVtKOqa4a4s40H2qahY3ffyn/MzIx52
imAYb7IK4MqCPQrzr9nM7qf7UApyLEaQDjN5n+bJ4ZEEd+p3kdf2sVrAFdTom0SU
hXmF1+7TCW4pVB6kL35YbtJkKQbHPfXTHVAJ/ctsLMMNNzXSiKUdnlCV+CwWfrgC
ptMy/Rk7WoHssyQZgl83JsQBdz396j0M6M3+kUNSGFThCz3OF9YBAccrKy7gqeHr
cyg+QQQN4yNyFaWl7S9a
=5O6Z
-----END PGP SIGNATURE-----


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

* Re: [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-19 22:56             ` Michał Górny
  2015-12-19 23:47               ` [gentoo-dev] " Jonathan Callen
@ 2015-12-19 23:56               ` Ulrich Mueller
  2015-12-20  9:16                 ` Michał Górny
  1 sibling, 1 reply; 18+ messages in thread
From: Ulrich Mueller @ 2015-12-19 23:56 UTC (permalink / raw
  To: Michał Górny; +Cc: gentoo-dev, Pacho Ramos

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

>>>>> On Sat, 19 Dec 2015, Michał Górny wrote:

>> set -f
>> echo -e ${DOC_CONTENTS} | ...
>> 
>> versus:
>> 
>> read -d '' -r -a DOC_CONTENTS <<<"${DOC_CONTENTS}"
>> echo -e "${DOC_CONTENTS[*]}" | ...
>> 
>> The second one is (IMHO) harder to understand, less efficient, and
>> relies on undocumented behaviour.

> On WHAT?!

Documentation of the read command doesn't specify behaviour for an
empty argument of the -d option:

    `-d DELIM'
          The first character of DELIM is used to terminate the input
          line, rather than newline.

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

* Re: [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-19 23:56               ` [gentoo-dev] " Ulrich Mueller
@ 2015-12-20  9:16                 ` Michał Górny
  2015-12-20 10:31                   ` Ulrich Mueller
  0 siblings, 1 reply; 18+ messages in thread
From: Michał Górny @ 2015-12-20  9:16 UTC (permalink / raw
  To: Ulrich Mueller; +Cc: gentoo-dev, Pacho Ramos

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

On Sun, 20 Dec 2015 00:56:39 +0100
Ulrich Mueller <ulm@gentoo.org> wrote:

> >>>>> On Sat, 19 Dec 2015, Michał Górny wrote:  
> 
> >> set -f
> >> echo -e ${DOC_CONTENTS} | ...
> >> 
> >> versus:
> >> 
> >> read -d '' -r -a DOC_CONTENTS <<<"${DOC_CONTENTS}"
> >> echo -e "${DOC_CONTENTS[*]}" | ...
> >> 
> >> The second one is (IMHO) harder to understand, less efficient, and
> >> relies on undocumented behaviour.  
> 
> > On WHAT?!  
> 
> Documentation of the read command doesn't specify behaviour for an
> empty argument of the -d option:
> 
>     `-d DELIM'
>           The first character of DELIM is used to terminate the input
>           line, rather than newline.

Feel free to use $'\0' if that makes it more explicit for you.

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

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

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

* Re: [gentoo-dev] Re: [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-19 23:47               ` [gentoo-dev] " Jonathan Callen
@ 2015-12-20 10:17                 ` Andrew Savchenko
  0 siblings, 0 replies; 18+ messages in thread
From: Andrew Savchenko @ 2015-12-20 10:17 UTC (permalink / raw
  To: gentoo-dev

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

On Sat, 19 Dec 2015 18:47:45 -0500 Jonathan Callen wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
> 
> On 12/19/2015 05:56 PM, Michał Górny wrote:
> > On Sat, 19 Dec 2015 23:51:47 +0100 Ulrich Mueller <ulm@gentoo.org>
> > wrote:
> > 
> >>>>>>> On Sat, 19 Dec 2015, Michał Górny wrote:
> >> 
> >>>> (I guess it is encountering EOF on the temporary file created
> >>>> by the string redirection, but how would one distinguish this
> >>>> from other errors?)
> >> 
> >>> read's return code indicates whether if found a full line (with
> >>> a newline). read can't really fail.
> >> 
> >> Certainly writing or reading the temp file can fail?
> >> 
> >>>> Besides, it is hard to understand what this code does, as
> >>>> compared to the "set -f" solution.
> >> 
> >>> Many pieces of good code are harder to understand than cheap,
> >>> ugly hacks. That's why those hacks are so common, and people
> >>> meet them all the time and never learn good code.
> >> 
> >> Well, compare:
> >> 
> >> set -f echo -e ${DOC_CONTENTS} | ...
> >> 
> >> versus:
> >> 
> >> read -d '' -r -a DOC_CONTENTS <<<"${DOC_CONTENTS}" echo -e
> >> "${DOC_CONTENTS[*]}" | ...
> >> 
> >> The second one is (IMHO) harder to understand, less efficient,
> >> and relies on undocumented behaviour.
> > 
> > On WHAT?!
> > 
> 
> Apparently, bash(1) and bash.info don't document what it means to pass
> an empty string as the argument to the -d option of the read builtin
> (what actually happens is that in all cases, bash uses the first
> character of the C string that the option gets translated to, which is
> '\0' for the empty string; the documentation just refers to "the first
> character of the string passed").

And the first and the last character of an empty string is '\0'.
So this behaviour is perfectly defined.
 

Best regards,
Andrew Savchenko

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-20  9:16                 ` Michał Górny
@ 2015-12-20 10:31                   ` Ulrich Mueller
  2015-12-20 10:56                     ` Michał Górny
  0 siblings, 1 reply; 18+ messages in thread
From: Ulrich Mueller @ 2015-12-20 10:31 UTC (permalink / raw
  To: Michał Górny; +Cc: gentoo-dev, Pacho Ramos

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

>>>>> On Sun, 20 Dec 2015, Michał Górny wrote:

> Feel free to use $'\0' if that makes it more explicit for you.

This is even worse. The null character cannot be passed as an argument
because it is the string terminator. No, I won't use the "read"
variant at all, because it is an unreadable and gross hack.

Besides, the "set -f" code in the eclass works just fine and has been
tested for several years by now.

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

* Re: [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-20 10:31                   ` Ulrich Mueller
@ 2015-12-20 10:56                     ` Michał Górny
  2015-12-20 12:13                       ` Ulrich Mueller
  0 siblings, 1 reply; 18+ messages in thread
From: Michał Górny @ 2015-12-20 10:56 UTC (permalink / raw
  To: gentoo-dev, Ulrich Mueller; +Cc: Pacho Ramos

Dnia 20 grudnia 2015 11:31:58 CET, Ulrich Mueller <ulm@gentoo.org> napisał(a):
>>>>>> On Sun, 20 Dec 2015, Michał Górny wrote:
>
>> Feel free to use $'\0' if that makes it more explicit for you.
>
>This is even worse. The null character cannot be passed as an argument
>because it is the string terminator. No, I won't use the "read"
>variant at all, because it is an unreadable and gross hack.
>
>Besides, the "set -f" code in the eclass works just fine and has been
>tested for several years by now.

Sure. Relying on implicit word splitting which is a side effect of unquoted variable use, along with disabling side effects of that use is not a hack. On the other hand, using built-in dedicated to word splitting is. Of course.

While at it, please make sure to remove all uses of 'find -print0' hack. I'm pretty sure you can find some implicit ugly side effect that could allow us to do the same after mangling a few shopts.

>
>Ulrich


-- 
Best regards,
Michał Górny (by phone)


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

* Re: [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-20 10:56                     ` Michał Górny
@ 2015-12-20 12:13                       ` Ulrich Mueller
  2015-12-21  7:38                         ` Pacho Ramos
  0 siblings, 1 reply; 18+ messages in thread
From: Ulrich Mueller @ 2015-12-20 12:13 UTC (permalink / raw
  To: Michał Górny; +Cc: gentoo-dev, Pacho Ramos

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

>>>>> On Sun, 20 Dec 2015, Michał Górny wrote:

>> Besides, the "set -f" code in the eclass works just fine and has
>> been tested for several years by now.

> Sure. Relying on implicit word splitting which is a side effect of
> unquoted variable use, along with disabling side effects of that use
> is not a hack. On the other hand, using built-in dedicated to word
> splitting is. Of course.

> While at it, please make sure to remove all uses of 'find -print0'
> hack. I'm pretty sure you can find some implicit ugly side effect
> that could allow us to do the same after mangling a few shopts.

So, you don't get things exactly in your style, and then other dev's
code is an "ugly hack" and uses "side effects". Even if the code in
question causes no problems whatsoever and uses standard well
documented shell features.

I think we should end this discussion here. Let the eclass maintainer
decide what approach (if any) he wants to adopt.

Ulrich

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

* Re: [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-20 12:13                       ` Ulrich Mueller
@ 2015-12-21  7:38                         ` Pacho Ramos
  2015-12-21  7:55                           ` Ulrich Mueller
  0 siblings, 1 reply; 18+ messages in thread
From: Pacho Ramos @ 2015-12-21  7:38 UTC (permalink / raw
  To: Ulrich Mueller, Michał Górny; +Cc: gentoo-dev

El dom, 20-12-2015 a las 13:13 +0100, Ulrich Mueller escribió:
> > > > > > On Sun, 20 Dec 2015, Michał Górny wrote:
> 
> > > Besides, the "set -f" code in the eclass works just fine and has
> > > been tested for several years by now.
> 
> > Sure. Relying on implicit word splitting which is a side effect of
> > unquoted variable use, along with disabling side effects of that
> > use
> > is not a hack. On the other hand, using built-in dedicated to word
> > splitting is. Of course.
> 
> > While at it, please make sure to remove all uses of 'find -print0'
> > hack. I'm pretty sure you can find some implicit ugly side effect
> > that could allow us to do the same after mangling a few shopts.
> 
> So, you don't get things exactly in your style, and then other dev's
> code is an "ugly hack" and uses "side effects". Even if the code in
> question causes no problems whatsoever and uses standard well
> documented shell features.
> 
> I think we should end this discussion here. Let the eclass maintainer
> decide what approach (if any) he wants to adopt.
> 
> Ulrich

I prefer to keep the original behavior as ulm patch did :|


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

* Re: [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils.
  2015-12-21  7:38                         ` Pacho Ramos
@ 2015-12-21  7:55                           ` Ulrich Mueller
  0 siblings, 0 replies; 18+ messages in thread
From: Ulrich Mueller @ 2015-12-21  7:55 UTC (permalink / raw
  To: gentoo-dev; +Cc: Pacho Ramos, Michał Górny

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

>>>>> On Mon, 21 Dec 2015, Pacho Ramos wrote:

> El dom, 20-12-2015 a las 13:13 +0100, Ulrich Mueller escribió:
>> I think we should end this discussion here. Let the eclass
>> maintainer decide what approach (if any) he wants to adopt.

> I prefer to keep the original behavior as ulm patch did :|

Pushed.

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

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

end of thread, other threads:[~2015-12-21  7:55 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-19 17:18 [gentoo-dev] [PATCH] readme.gentoo-r1.eclass: Do not inherit eutils Ulrich Mueller
2015-12-19 17:32 ` Michał Górny
2015-12-19 18:05   ` [gentoo-dev] " Martin Vaeth
2015-12-19 18:24   ` [gentoo-dev] " Ulrich Mueller
2015-12-19 18:34     ` Michał Górny
2015-12-19 20:17       ` Ulrich Mueller
2015-12-19 20:52         ` Michał Górny
2015-12-19 22:51           ` Ulrich Mueller
2015-12-19 22:56             ` Michał Górny
2015-12-19 23:47               ` [gentoo-dev] " Jonathan Callen
2015-12-20 10:17                 ` Andrew Savchenko
2015-12-19 23:56               ` [gentoo-dev] " Ulrich Mueller
2015-12-20  9:16                 ` Michał Górny
2015-12-20 10:31                   ` Ulrich Mueller
2015-12-20 10:56                     ` Michał Górny
2015-12-20 12:13                       ` Ulrich Mueller
2015-12-21  7:38                         ` Pacho Ramos
2015-12-21  7:55                           ` Ulrich Mueller

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