The GNUlib portability library bug discussion list

Akim Demaille | 6 Jul 2012 10:19

[PATCH] bootstrap: use a more consistent error reporting scheme.

Independently of the other thread we're having about set -e,
I had refactored bootstrap a bit.
-8<---

* build-aux/bootstrap (stderr, die): New.
Use them.
---
 build-aux/bootstrap | 94 ++++++++++++++++++++++++++---------------------------
 1 file changed, 47 insertions(+), 47 deletions(-)

diff --git a/build-aux/bootstrap b/build-aux/bootstrap
index e984910..ad94ef4 100755
--- a/build-aux/bootstrap
+++ b/build-aux/bootstrap
 <at>  <at>  -1,6 +1,6  <at>  <at> 
 #! /bin/sh
 # Print a version string.
-scriptversion=2012-07-03.20; # UTC
+scriptversion=2012-07-06.08; # UTC

 # Bootstrap this package from checked-out sources.

 <at>  <at>  -77,6 +77,16  <at>  <at>  Running without arguments will suffice in most cases.
 EOF
 }

+stderr()
+{
+  for i
+  do

(Continue reading)

headers

Jim Meyering | 6 Jul 2012 11:43

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Akim Demaille wrote:
> Independently of the other thread we're having about set -e,
> I had refactored bootstrap a bit.
> -8<---
>
> * build-aux/bootstrap (stderr, die): New.
> Use them.
> ---
>  build-aux/bootstrap | 94 ++++++++++++++++++++++++++---------------------------
>  1 file changed, 47 insertions(+), 47 deletions(-)
>
> diff --git a/build-aux/bootstrap b/build-aux/bootstrap
> index e984910..ad94ef4 100755
> --- a/build-aux/bootstrap
> +++ b/build-aux/bootstrap
>  <at>  <at>  -1,6 +1,6  <at>  <at> 
>  #! /bin/sh
>  # Print a version string.
> -scriptversion=2012-07-03.20; # UTC
> +scriptversion=2012-07-06.08; # UTC
>
>  # Bootstrap this package from checked-out sources.
>
>  <at>  <at>  -77,6 +77,16  <at>  <at>  Running without arguments will suffice in most cases.
>  EOF
>  }
>
> +stderr()
> +{
> +  for i

(Continue reading)

headers

Akim Demaille | 6 Jul 2012 11:55

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Hi Jim!

Le 6 juil. 2012 à 11:43, Jim Meyering a écrit :

> Thanks for the patch.
> 
> Those all look like improvements, but I'd prefer that
> you change the name s/stderr/warn/: "stderr" is not normally
> used as a verb.

OK.  I avoided "warn" because I felt it would be valid
for it to include a "warning: " prefix.

> Also, I am in the habit of writing e.g.,
> 
>    warn this does not need quotes
> 
> With your implementation, that would print the expansion of:
> 
>    $me: this
>    $me: does
>    $me: not
>    $me: need
>    $me: quotes
> 
> Something like the warn_ function in tests/init.sh (but without the
> stderr_fileno_ bit) may do what we want: it's received pretty much testing.

It is also very useful to have a single command that issues
several lines.  This is the case here for instance:

(Continue reading)

headers

Jim Meyering | 6 Jul 2012 12:01

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Akim Demaille wrote:

> Hi Jim!
>
> Le 6 juil. 2012 à 11:43, Jim Meyering a écrit :
>
>> Thanks for the patch.
>>
>> Those all look like improvements, but I'd prefer that
>> you change the name s/stderr/warn/: "stderr" is not normally
>> used as a verb.
>
> OK.  I avoided "warn" because I felt it would be valid
> for it to include a "warning: " prefix.
>
>> Also, I am in the habit of writing e.g.,
>>
>>    warn this does not need quotes
>>
>> With your implementation, that would print the expansion of:
>>
>>    $me: this
>>    $me: does
>>    $me: not
>>    $me: need
>>    $me: quotes
>>
>> Something like the warn_ function in tests/init.sh (but without the
>> stderr_fileno_ bit) may do what we want: it's received pretty much testing.
>

(Continue reading)

headers

Akim Demaille | 6 Jul 2012 13:41

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.


Le 6 juil. 2012 à 12:01, Jim Meyering a écrit :

> Akim Demaille wrote:
> 
>> It is also very useful to have a single command that issues
>> several lines.  This is the case here for instance:
> 
> I would debate the "very useful" part 
> How about "might be nice, in the rare event someone sees this message"?

Fair enough :)  It should have read "I have used this
often in several scripts I maintain".

> No change.  The existing code uses echo and prints it all on one line.
> I think it's fine to leave it that way.
> It's only a diagnostic, after all.

Installed!

headers

Jim Meyering | 6 Jul 2012 13:56

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Akim Demaille wrote:
> Le 6 juil. 2012 à 12:01, Jim Meyering a écrit :
>
>> Akim Demaille wrote:
>>
>>> It is also very useful to have a single command that issues
>>> several lines.  This is the case here for instance:
>>
>> I would debate the "very useful" part 
>> How about "might be nice, in the rare event someone sees this message"?
>
> Fair enough :)  It should have read "I have used this
> often in several scripts I maintain".
>
>> No change.  The existing code uses echo and prints it all on one line.
>> I think it's fine to leave it that way.
>> It's only a diagnostic, after all.
>
> Installed!

Um... this is a good reason to post an adjusted patch.
What I meant was that it is fine to continue to print that diagnostic
on one line, but not using echo; using the new "warn".

It should use warn, and warn should be defined as in init.sh.

headers

Akim Demaille | 6 Jul 2012 14:08

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.


Le 6 juil. 2012 à 13:56, Jim Meyering a écrit :

> Um... this is a good reason to post an adjusted patch.
> What I meant was that it is fine to continue to print that diagnostic
> on one line, but not using echo; using the new "warn".
> 
> It should use warn, and warn should be defined as in init.sh.

Ah, ok.  But the messages were already on several lines before,
so you actually prefer to make the diagnostic less formatted?

-if test $found_aux_dir = no; then
-  echo "$0: expected line not found in configure.ac. Add the following:" >&2
-  echo "  AC_CONFIG_AUX_DIR([$build_aux])" >&2
-  exit 1
-fi
+test $found_aux_dir = yes \
+  || die "expected line not found in configure.ac. Add the following:" \
+         "  AC_CONFIG_AUX_DIR([$build_aux])"

-          echo "$me: Error: '$app' version == $inst_ver is too old" >&2
-          echo "       '$app' version >= $req_ver is required" >&2
+          warn "Error: '$app' version == $inst_ver is too old" \
+               "       '$app' version >= $req_ver is required"

Or do you prefer reintroducing braces and several commands for a single,
but multi-line, message?

(Continue reading)

headers

Jim Meyering | 6 Jul 2012 14:24

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Akim Demaille wrote:
> Le 6 juil. 2012 à 13:56, Jim Meyering a écrit :
>
>> Um... this is a good reason to post an adjusted patch.
>> What I meant was that it is fine to continue to print that diagnostic
>> on one line, but not using echo; using the new "warn".
>>
>> It should use warn, and warn should be defined as in init.sh.
>
> Ah, ok.  But the messages were already on several lines before,
> so you actually prefer to make the diagnostic less formatted?
>
> -if test $found_aux_dir = no; then
> -  echo "$0: expected line not found in configure.ac. Add the following:" >&2
> -  echo "  AC_CONFIG_AUX_DIR([$build_aux])" >&2
> -  exit 1
> -fi
> +test $found_aux_dir = yes \
> +  || die "expected line not found in configure.ac. Add the following:" \
> +         "  AC_CONFIG_AUX_DIR([$build_aux])"

Actually, using die above is fine.
However, I would like to keep bootstrap's warn and init.sh's warn_ compatible.

> -          echo "$me: Error: '$app' version == $inst_ver is too old" >&2
> -          echo "       '$app' version >= $req_ver is required" >&2
> +          warn "Error: '$app' version == $inst_ver is too old" \
> +               "       '$app' version >= $req_ver is required"
>
> Or do you prefer reintroducing braces and several commands for a single,

(Continue reading)

headers

Akim Demaille | 6 Jul 2012 14:37

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.


Le 6 juil. 2012 à 14:24, Jim Meyering a écrit :

> Akim Demaille wrote:
>> Le 6 juil. 2012 à 13:56, Jim Meyering a écrit :
>> 
>>> Um... this is a good reason to post an adjusted patch.
>>> What I meant was that it is fine to continue to print that diagnostic
>>> on one line, but not using echo; using the new "warn".
>>> 
>>> It should use warn, and warn should be defined as in init.sh.
>> 
>> Ah, ok.  But the messages were already on several lines before,
>> so you actually prefer to make the diagnostic less formatted?
>> 
>> -if test $found_aux_dir = no; then
>> -  echo "$0: expected line not found in configure.ac. Add the following:" >&2
>> -  echo "  AC_CONFIG_AUX_DIR([$build_aux])" >&2
>> -  exit 1
>> -fi
>> +test $found_aux_dir = yes \
>> +  || die "expected line not found in configure.ac. Add the following:" \
>> +         "  AC_CONFIG_AUX_DIR([$build_aux])"
> 
> Actually, using die above is fine.
> However, I would like to keep bootstrap's warn and init.sh's warn_ compatible.

So if I understand correctly die would take lines as arguments
and warn would take words?  Or maybe I don't understand what you
mean with "Actually, using die above is fine".  Maybe you mean

(Continue reading)

headers

Jim Meyering | 6 Jul 2012 14:44

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Akim Demaille wrote:
> Le 6 juil. 2012 à 14:24, Jim Meyering a écrit :
>
>> Akim Demaille wrote:
>>> Le 6 juil. 2012 à 13:56, Jim Meyering a écrit :
>>>
>>>> Um... this is a good reason to post an adjusted patch.
>>>> What I meant was that it is fine to continue to print that diagnostic
>>>> on one line, but not using echo; using the new "warn".
>>>>
>>>> It should use warn, and warn should be defined as in init.sh.
>>>
>>> Ah, ok.  But the messages were already on several lines before,
>>> so you actually prefer to make the diagnostic less formatted?
>>>
>>> -if test $found_aux_dir = no; then
>>> -  echo "$0: expected line not found in configure.ac. Add the following:" >&2
>>> -  echo "  AC_CONFIG_AUX_DIR([$build_aux])" >&2
>>> -  exit 1
>>> -fi
>>> +test $found_aux_dir = yes \
>>> +  || die "expected line not found in configure.ac. Add the following:" \
>>> +         "  AC_CONFIG_AUX_DIR([$build_aux])"
>>
>> Actually, using die above is fine.
>> However, I would like to keep bootstrap's warn and init.sh's warn_ compatible.
>
> So if I understand correctly die would take lines as arguments
> and warn would take words?  Or maybe I don't understand what you
> mean with "Actually, using die above is fine".  Maybe you mean

(Continue reading)

headers

Akim Demaille | 6 Jul 2012 15:11

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.


Le 6 juil. 2012 à 14:44, Jim Meyering a écrit :

> No, but if you think it's better, somehow...
> I meant that warnf would be a very thin wrapper around printf:
> 
>    $ printf '%s\n' a b c
>    a
>    b
>    c
> 
>> Or should warnf expect a single %s
> 
> or multiple %-directives:
> 
>    $ printf '%s - %s\n' a b c d
>    a - b
>    c - d

Ah, I was not aware of this feature, thanks!
Now I see what you mean (thanks to Stefano too :).

How about this?

From 96b829fb08217060f20a6a56eddb1a952324fa35 Mon Sep 17 00:00:00 2001
From: Akim Demaille <akim <at> lrde.epita.fr>
Date: Fri, 6 Jul 2012 15:01:53 +0200
Subject: [PATCH] bootstrap: let warn be alike tests/init.sh's warn_

Reported by Jim Meyering.

(Continue reading)

headers

Jim Meyering | 6 Jul 2012 15:25

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Akim Demaille wrote:
> Le 6 juil. 2012 à 14:44, Jim Meyering a écrit :
>
>> No, but if you think it's better, somehow...
>> I meant that warnf would be a very thin wrapper around printf:
>>
>>    $ printf '%s\n' a b c
>>    a
>>    b
>>    c
>>
>>> Or should warnf expect a single %s
>>
>> or multiple %-directives:
>>
>>    $ printf '%s - %s\n' a b c d
>>    a - b
>>    c - d
>
> Ah, I was not aware of this feature, thanks!
> Now I see what you mean (thanks to Stefano too :).
>
> How about this?
>
> Subject: [PATCH] bootstrap: let warn be alike tests/init.sh's warn_

Thanks.

Nit #1:

(Continue reading)

headers

Bruce Korb | 6 Jul 2012 18:04

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

On 07/06/12 06:11, Akim Demaille wrote:

One other tiny nit that is helpful in a die() function:
It isn't necessarily true that "die" always gets invoked
from the main process.  If it gets invoked from a subprocess
for any of a variety of reasons, this:

+die() { warn_ "$ <at> "; exit 1; }

won't work.  I use this (more or less) as a standard template:

readonly progpid=$$
die() {
     warn_ "$ <at> "
     kill -9 $progpid
     exit 1
} 1>&2

headers

Eric Blake | 10 Jul 2012 01:04

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

On 07/06/2012 07:11 AM, Akim Demaille wrote:

> How about this?
> 
> From 96b829fb08217060f20a6a56eddb1a952324fa35 Mon Sep 17 00:00:00 2001
> From: Akim Demaille <akim <at> lrde.epita.fr>
> Date: Fri, 6 Jul 2012 15:01:53 +0200
> Subject: [PATCH] bootstrap: let warn be alike tests/init.sh's warn_
> 
> Reported by Jim Meyering.
> * build-aux/bootstrap (warn): Remove, replaced by...
> (warnf_, warn_): these.
> Adjust callers.
> Shorten messages that no longer fit in 80 columns.

> -warn()
> +# warnf_ FORMAT-STRING ARG1...
> +warnf_ ()
>  {
> -  for i
> -  do
> -    echo "$i"
> -  done | sed -e "s/^/$me: /" >&2

This is broken if you invoke './bootstrap', since then $me contains /
and bootstrap complains:

autoreconf: Leaving directory `.'
sed: -e expression #1, char 7: unknown option to `s'

(Continue reading)

headers

Eric Blake | 10 Jul 2012 02:00

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

On 07/09/2012 05:04 PM, Eric Blake wrote:

>> -warn()
>> +# warnf_ FORMAT-STRING ARG1...
>> +warnf_ ()
>>  {
>> -  for i
>> -  do
>> -    echo "$i"
>> -  done | sed -e "s/^/$me: /" >&2
> 
> This is broken if you invoke './bootstrap', since then $me contains /
> and bootstrap complains:
> 
> autoreconf: Leaving directory `.'
> sed: -e expression #1, char 7: unknown option to `s'

More precisely, this is broken only if bootstrap encounters a failure,
as the real failure message is lost by the sed failure message, but a
useless error message is indeed pretty lousy for debugging purposes.  I
traced my particular problem to a case of libvirt not being prepared for
the fact that the 'missing' script is no longer shipped in gnulib.

+ for file in '$gnulib_extra_files'
+ case $file in
+ dst=build-aux/missing
+ symlink_to_dir .gnulib build-aux/missing build-aux/missing
+ src=.gnulib/build-aux/missing
+ dst=build-aux/missing
+ test -f .gnulib/build-aux/missing

(Continue reading)

headers

Jim Meyering | 10 Jul 2012 07:21

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Akim Demaille wrote:
> Le 6 juil. 2012 à 14:44, Jim Meyering a écrit :
>> No, but if you think it's better, somehow...
>> I meant that warnf would be a very thin wrapper around printf:
>>
>>    $ printf '%s\n' a b c
>>    a
>>    b
>>    c
>>
>>> Or should warnf expect a single %s
>>
>> or multiple %-directives:
>>
>>    $ printf '%s - %s\n' a b c d
>>    a - b
>>    c - d
>
> Ah, I was not aware of this feature, thanks!
> Now I see what you mean (thanks to Stefano too :).
>
> How about this?
>
> From 96b829fb08217060f20a6a56eddb1a952324fa35 Mon Sep 17 00:00:00 2001
> From: Akim Demaille <akim <at> lrde.epita.fr>
> Date: Fri, 6 Jul 2012 15:01:53 +0200
> Subject: [PATCH] bootstrap: let warn be alike tests/init.sh's warn_

s/alike/like/

(Continue reading)

headers

Akim Demaille | 10 Jul 2012 10:07

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Hi all,

Here is an updated proposal, that was taking into account
several comments made last week.

Attachment (0001-bootstrap-let-warn-be-like-tests-init.sh-s-warn_.patch): application/octet-stream, 4462 bytes

headers

Jim Meyering | 10 Jul 2012 10:14

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Akim Demaille wrote:
> Here is an updated proposal, that was taking into account
> several comments made last week.

Looks fine, modulo a question and test artifacts:
Thanks!

...
> -warn()
> +# warnf_ FORMAT-STRING ARG1...
> +warnf_ ()
>  {
> -  for i
> -  do
> -    echo "$i"
> -  done | sed -e "s/^/$me: /" >&2
> +  warnf_format_=$1
> +  shift
> +  nl='
> +'
> +  case $* in
> +    *$nl*) me_=$(printf "$me"|tr "$nl|" '??')

Is it worth testing for both $nl and '|' ?

> +       printf "$warnf_format_" "$ <at> " | sed "s|^|$me_: |" ;;
> +    *) printf "$me: $warnf_format_" "$ <at> " ;;
> +  esac >&2
> +}
> +

(Continue reading)

headers

Akim Demaille | 10 Jul 2012 11:19

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.


Le 10 juil. 2012 à 10:14, Jim Meyering a écrit :

>> +  case $* in
>> +    *$nl*) me_=$(printf "$me"|tr "$nl|" '??')
> 
> Is it worth testing for both $nl and '|' ?

I tried to keep what you did about $nl in $me (it would break
the sed command).  I used printf, since the \n from echo gave
a spurious ? at the end of $me.

And | is the separator for sed.

>> +       printf "$warnf_format_" "$ <at> " | sed "s|^|$me_: |" ;;
>> +    *) printf "$me: $warnf_format_" "$ <at> " ;;
>> +  esac >&2
>> +}
>> +
>> +warnf_ '<%s>\n' "test1" 't
>> +e
>> +d
>> +2' t3 t54
> 
> No test artifacts, please 

Bummer.  Thanks a lot…

Installed as follows.

(Continue reading)

headers

Jim Meyering | 10 Jul 2012 12:05

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Akim Demaille wrote:

> Le 10 juil. 2012 à 10:14, Jim Meyering a écrit :
>
>>> +  case $* in
>>> +    *$nl*) me_=$(printf "$me"|tr "$nl|" '??')
>>
>> Is it worth testing for both $nl and '|' ?
>
> I tried to keep what you did about $nl in $me (it would break
> the sed command).  I used printf, since the \n from echo gave
> a spurious ? at the end of $me.

I meant "is it worth *also* testing for '|' there"?
E.g., if $me contains '|', but no newline.
Definitely not a big issue.

headers

Akim Demaille | 10 Jul 2012 12:42

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Le 10 juil. 2012 à 12:05, Jim Meyering a écrit :

> Akim Demaille wrote:
> 
>> Le 10 juil. 2012 à 10:14, Jim Meyering a écrit :
>> 
>>>> +  case $* in
>>>> +    *$nl*) me_=$(printf "$me"|tr "$nl|" '??')
>>> 
>>> Is it worth testing for both $nl and '|' ?
>> 
>> I tried to keep what you did about $nl in $me (it would break
>> the sed command).  I used printf, since the \n from echo gave
>> a spurious ? at the end of $me.
> 
> I meant "is it worth *also* testing for '|' there"?
> E.g., if $me contains '|', but no newline.
> Definitely not a big issue.

Sorry, there must be something I am missing :/.
I use sed when there are $nl in the messages (not
in $me, just in $*).  And when I use sed, I avoid characters
that can be troublesome for sed.

But when there is no need to call sed, I just use
$me as is, including with | and \n, as it should not
be a problem.

headers

Jim Meyering | 10 Jul 2012 13:08

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Akim Demaille wrote:

> Le 10 juil. 2012 à 12:05, Jim Meyering a écrit :
>
>> Akim Demaille wrote:
>>
>>> Le 10 juil. 2012 à 10:14, Jim Meyering a écrit :
>>>
>>>>> +  case $* in
>>>>> +    *$nl*) me_=$(printf "$me"|tr "$nl|" '??')
>>>>
>>>> Is it worth testing for both $nl and '|' ?
>>>
>>> I tried to keep what you did about $nl in $me (it would break
>>> the sed command).  I used printf, since the \n from echo gave
>>> a spurious ? at the end of $me.
>>
>> I meant "is it worth *also* testing for '|' there"?
>> E.g., if $me contains '|', but no newline.
>> Definitely not a big issue.
>
> Sorry, there must be something I am missing :/.

No, 'twas I.
If $me contains '|' there's no need for sed, as you say.

> I use sed when there are $nl in the messages (not
> in $me, just in $*).  And when I use sed, I avoid characters
> that can be troublesome for sed.
>

(Continue reading)

headers

Akim Demaille | 6 Jul 2012 15:11

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.


Le 6 juil. 2012 à 14:44, Jim Meyering a écrit :

> No, but if you think it's better, somehow...
> I meant that warnf would be a very thin wrapper around printf:
> 
>    $ printf '%s\n' a b c
>    a
>    b
>    c
> 
>> Or should warnf expect a single %s
> 
> or multiple %-directives:
> 
>    $ printf '%s - %s\n' a b c d
>    a - b
>    c - d

Ah, I was not aware of this feature, thanks!
Now I see what you mean (thanks to Stefano too :).

How about this?

From 96b829fb08217060f20a6a56eddb1a952324fa35 Mon Sep 17 00:00:00 2001
From: Akim Demaille <akim <at> lrde.epita.fr>
Date: Fri, 6 Jul 2012 15:01:53 +0200
Subject: [PATCH] bootstrap: let warn be alike tests/init.sh's warn_

Reported by Jim Meyering.

(Continue reading)

headers

Stefano Lattarini | 6 Jul 2012 15:21

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

On 07/06/2012 03:11 PM, Akim Demaille wrote:
>
> --- a/build-aux/bootstrap
> +++ b/build-aux/bootstrap
>  <at>  <at>  -1,6 +1,6  <at>  <at> 
>  #! /bin/sh
>  # Print a version string.
> -scriptversion=2012-07-06.11; # UTC
> +scriptversion=2012-07-06.13; # UTC
>  
>  # Bootstrap this package from checked-out sources.
>  
>  <at>  <at>  -77,15 +77,26  <at>  <at>  Running without arguments will suffice in most cases.
>  EOF
>  }
>  
> -warn()
> +# warnf_ FORMAT-STRING ARG1...
> +warnf_ ()
>  {
> -  for i
> -  do
> -    echo "$i"
> -  done | sed -e "s/^/$me: /" >&2
> +  warnf_format_=$1
> +  shift
> +  printf "$warnf_format_" "$ <at> " | sed "s,^,$me: ," >&2
>
Why not shave off the extra forks here?

(Continue reading)

headers

Akim Demaille | 6 Jul 2012 15:28

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.


Le 6 juil. 2012 à 15:21, Stefano Lattarini a écrit :

> Why not shave off the extra forks here?
> 
>    printf "$me: $warnf_format_" "$ <at> " >&2
> 
> This shouldn't cause problems, unless '$me' contains '%' or '\' characters.

Or end of line.  Elsewhere, where I use similar routines,
it is useful to be able to output an error/log message from
a tool for instance, which may be multiline.

In that case I prefer to keep the original formatting, but
prepend the name of the tool on each line.

That's why I did it this way, but I'll do whatever people here
tell me to.

headers

Stefano Lattarini | 6 Jul 2012 15:54

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

On 07/06/2012 03:28 PM, Akim Demaille wrote:
> 
> Le 6 juil. 2012 à 15:21, Stefano Lattarini a écrit :
> 
>> Why not shave off the extra forks here?
>>
>>    printf "$me: $warnf_format_" "$ <at> " >&2
>>
>> This shouldn't cause problems, unless '$me' contains '%' or '\' characters.
> 
> Or end of line.
>
You mean in one "$ <at> " entry, not in "$me", right?  If yes, point taken.

  Elsewhere, where I use similar routines,
> it is useful to be able to output an error/log message from
> a tool for instance, which may be multiline.
> 
> In that case I prefer to keep the original formatting, but
> prepend the name of the tool on each line.
> 
> That's why I did it this way, but I'll do whatever people here
> tell me to.
> 
I didn't think about the use cases you've described, and they wouldn't
have seemed important *to me*; but if they are to you, please use your
original formulation.  I don't think it's really worth worrying about
performance in this case anyway.  Sure not worth holding this patch
back!

(Continue reading)

headers

Akim Demaille | 6 Jul 2012 16:08

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.


Le 6 juil. 2012 à 15:54, Stefano Lattarini a écrit :

> On 07/06/2012 03:28 PM, Akim Demaille wrote:
>> 
>> Le 6 juil. 2012 à 15:21, Stefano Lattarini a écrit :
>> 
>>> Why not shave off the extra forks here?
>>> 
>>>   printf "$me: $warnf_format_" "$ <at> " >&2
>>> 
>>> This shouldn't cause problems, unless '$me' contains '%' or '\' characters.
>> 
>> Or end of line.
>> 
> You mean in one "$ <at> " entry, not in "$me", right?  If yes, point taken.

Yes, I do.

> I didn't think about the use cases you've described, and they wouldn't
> have seemed important *to me*; but if they are to you, please use your
> original formulation.  I don't think it's really worth worrying about
> performance in this case anyway.  Sure not worth holding this patch
> back!

I'm waiting for Jim's call.

headers

Jim Meyering | 6 Jul 2012 16:09

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Akim Demaille wrote:
> Le 6 juil. 2012 à 15:21, Stefano Lattarini a écrit :
>
>> Why not shave off the extra forks here?
>>
>>    printf "$me: $warnf_format_" "$ <at> " >&2
>>
>> This shouldn't cause problems, unless '$me' contains '%' or '\' characters.
>
> Or end of line.  Elsewhere, where I use similar routines,
> it is useful to be able to output an error/log message from
> a tool for instance, which may be multiline.
>
> In that case I prefer to keep the original formatting, but
> prepend the name of the tool on each line.

You're welcome to leave the pipe-to-sed.
Worrying about an extra process when processing a diagnostic
is probably a pre-optimization anyhow.  Besides, then
we don't have to worry about whether $me is printf-safe.

Thanks.

headers

Paul Eggert | 6 Jul 2012 16:39

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

On 07/06/2012 07:09 AM, Jim Meyering wrote:
> You're welcome to leave the pipe-to-sed.
> Worrying about an extra process when processing a diagnostic
> is probably a pre-optimization anyhow.

... unless the diagnostic is something like "program failed"
due to lack of enough system resources to fork.

Perhaps the function could use a 'case' to see whether the
format contains '\n' or a newline, and use the pipe-to-sed
only in that case?  It is a bit of a hack, admittedly.

headers

Akim Demaille | 6 Jul 2012 16:40

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.


Le 6 juil. 2012 à 16:09, Jim Meyering a écrit :

> You're welcome to leave the pipe-to-sed.
> Worrying about an extra process when processing a diagnostic
> is probably a pre-optimization anyhow.  Besides, then
> we don't have to worry about whether $me is printf-safe.

But we have to worry about ',' in $me, which is $0, not
its basename.  Do you want me (Akim, not the variable)
to change me (the variable, not Akim; not that I am
immutable) to basename and use / in the sed command,
instead of ','?

# warnf_ FORMAT-STRING ARG1...
warnf_ ()
{
  warnf_format_=$1
  shift
  printf "$warnf_format_" "$ <at> " | sed "s,^,$me: ," >&2
}

headers

Jim Meyering | 6 Jul 2012 17:18

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Akim Demaille wrote:
> Le 6 juil. 2012 à 16:09, Jim Meyering a écrit :
>
>> You're welcome to leave the pipe-to-sed.
>> Worrying about an extra process when processing a diagnostic
>> is probably a pre-optimization anyhow.  Besides, then
>> we don't have to worry about whether $me is printf-safe.
>
> But we have to worry about ',' in $me, which is $0, not
> its basename.  Do you want me (Akim, not the variable)
> to change me (the variable, not Akim; not that I am
> immutable) to basename and use / in the sed command,
> instead of ','?
>
> # warnf_ FORMAT-STRING ARG1...
> warnf_ ()
> {
>   warnf_format_=$1
>   shift
>   printf "$warnf_format_" "$ <at> " | sed "s,^,$me: ," >&2
> }

Good point.
In that case, we might as well do it right, along the lines Paul
suggested, maybe like this:

warnf_ ()
{
  warnf_format_=$1
  shift

(Continue reading)

headers

Thien-Thi Nguyen | 6 Jul 2012 17:43

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

() Akim Demaille <akim <at> lrde.epita.fr>
() Fri, 6 Jul 2012 16:40:37 +0200

   But we have to worry about ',' in $me, which is $0, not
   its basename.  Do you want me (Akim, not the variable)
   to change me (the variable, not Akim; not that I am
   immutable) to basename and use / in the sed command,
   instead of ','?

   # warnf_ FORMAT-STRING ARG1...
   warnf_ ()
   {
     warnf_format_=$1
     shift
     printf "$warnf_format_" "$ <at> " | sed "s,^,$me: ," >&2
   }

Instead of these contortions, what about:

  printf "%s: $warnf_format" "$me" "$ <at> "

?

headers

Jim Meyering | 6 Jul 2012 17:49

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

Thien-Thi Nguyen wrote:

> () Akim Demaille <akim <at> lrde.epita.fr>
> () Fri, 6 Jul 2012 16:40:37 +0200
>
>    But we have to worry about ',' in $me, which is $0, not
>    its basename.  Do you want me (Akim, not the variable)
>    to change me (the variable, not Akim; not that I am
>    immutable) to basename and use / in the sed command,
>    instead of ','?
>
>    # warnf_ FORMAT-STRING ARG1...
>    warnf_ ()
>    {
>      warnf_format_=$1
>      shift
>      printf "$warnf_format_" "$ <at> " | sed "s,^,$me: ," >&2
>    }
>
> Instead of these contortions, what about:
>
>   printf "%s: $warnf_format" "$me" "$ <at> "
>
> ?

Hi Thien-Thi,

We want warnf '%s\n' a b c d
to print this (as printf would do):

(Continue reading)

headers

Thien-Thi Nguyen | 6 Jul 2012 18:29

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

() Jim Meyering <jim <at> meyering.net>
() Fri, 06 Jul 2012 17:49:03 +0200

   We want warnf '%s\n' a b c d
   to print this (as printf would do):

       $me: a
       $me: b
       $me: c
       $me: d

   With that added "%s: ", and with "$me" inserted into the
   list of arguments, we'd get this:

       $me: a
       b: c
       d: $me

Thanks for explaining it; i understand the contortions now.

headers

Stefano Lattarini | 6 Jul 2012 14:46

Re: [PATCH] bootstrap: use a more consistent error reporting scheme.

On 07/06/2012 02:37 PM, Akim Demaille wrote:
> Le 6 juil. 2012 à 14:24, Jim Meyering a écrit :
>
>>    warnf '%s\n' "Error: '$app' version == $inst_ver is too old" \
>>                 "       '$app' version >= $req_ver is required"
> 
> Don't you mean warnf '%s\n%s\n'?  Or should warnf expect a single %s
> in $1 and use $1 repeatedly on the remaining arguments?
> 
I think POSIX mandates that printf(1) applies the format string to
the arguments in a sort of loop:

  $ printf '%s\n' a b c d
  a
  b
  c
  d

  $ printf '%s-%s\n' a b c d
  a-b
  c-d

In fact, quoting from
<http://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html>:

   The format operand shall be reused as often as necessary to satisfy the
   argument operands.  Any extra c or s conversion specifiers shall be
   evaluated as if a null string argument were supplied; other extra
   conversion specifications shall be evaluated as if a zero argument were
   supplied. If the format operand contains no conversion specifications

(Continue reading)