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

[Akim Demaille]

Hi all,

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



0001-bootstrap-let-warn-be-like-tests-init.sh-s-warn_.patch
Description: Binary data

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

[Jim Meyering]

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_" "$@" | sed "s|^|$me_: |" ;;
> +*) printf "$me: $warnf_format_" "$@" ;;
> +  esac >&2
> +}
> +
> +warnf_ '<%s>\n' "test1" 't
> +e
> +d
> +2' t3 t54

No test artifacts, please ;-)

> +warnf_ ()
> +{
> +  warnf_format_=$1
> +  shift
> +  printf "$warnf_format_" "$@" | sed "s,^,$me: ," >&2
> +}

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

[Akim Demaille]

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

Bummer.  Thanks a lot…

Installed as follows.

From 4df9d155485595d7bae28d434a945f9c22dd1a71 Mon Sep 17 00:00:00 2001
From: Akim Demaille 
Date: Fri, 6 Jul 2012 15:01:53 +0200
Subject: [PATCH] bootstrap: let warn be like 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.
---
 ChangeLog   |  9 +
 build-aux/bootstrap | 47 ---
 2 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 892eb16..d94f7b1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2012-07-10  Akim Demaille  
+
+   bootstrap: let warn be like 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.
+
 2012-07-09  Bruno Haible  
 
getopt: Simplify after Emacs changed.
diff --git a/build-aux/bootstrap b/build-aux/bootstrap
index 1d61e5c..e00c8bbb 100755
--- a/build-aux/bootstrap
+++ b/build-aux/bootstrap
@@ -1,6 +1,6 @@
 #! /bin/sh
 # Print a version string.
-scriptversion=2012-07-06.11; # UTC
+scriptversion=2012-07-10.09; # UTC
 
 # Bootstrap this package from checked-out sources.
 
@@ -77,15 +77,32 @@ 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
+  nl='
+'
+  case $* in
+*$nl*) me_=$(printf "$me"|tr "$nl|" '??')
+   printf "$warnf_format_" "$@" | sed "s|^|$me_: |" ;;
+*) printf "$me: $warnf_format_" "$@" ;;
+  esac >&2
+}
+
+# warn_ WORD1...
+warn_ ()
+{
+  # If IFS does not start with ' ', set it and emit the warning in a subshell.
+  case $IFS in
+' '*) warnf_ '%s\n' "$*";;
+*)(IFS=' '; warn_ "$@");;
+  esac
 }
 
-die() { warn "$@"; exit 1; }
+# die WORD1...
+die() { warn_ "$@"; exit 1; }
 
 # Configuration.
 
@@ -335,8 +352,7 @@ grep '^[ ]*AC_CONFIG_AUX_DIR(\['"$build_aux"'\])' 
configure.ac \
 grep '^[]*AC_CONFIG_AUX_DIR('"$build_aux"')' configure.ac \
 >/dev/null && found_aux_dir=yes
 test $found_aux_dir = yes \
-  || die "expected line not found in configure.ac. Add the following:" \
- "  AC_CONFIG_AUX_DIR([$build_aux])"
+  || die "configure.ac lacks 'AC_CONFIG_AUX_DIR([$build_aux])'; add it"
 
 # If $build_aux doesn't exist, create it now, otherwise some bits
 # below will malfunction.  If creating it, also mark it as ignored.
@@ -442,7 +458,7 @@ check_versions() {
   automake-ng|aclocal-ng)
 app=${app%-ng}
 ($app --version | grep '(GNU automake-ng)') >/dev/null 2>&1 || {
-  warn "Error: '$app' not found or not from Automake-NG"
+  warn_ "Error: '$app' not found or not from Automake-NG"
   ret=1
   continue
 } ;;
@@ -452,20 +468,21 @@ check_versions() {
   # so we have to rely on $? rather than get_version.
   $app --version >/dev/null 2>&1
   if [ 126 -le $? ]; then
-warn "Error: '$app' not found"
+warn_ "Error: '$app' not found"
 ret=1
   fi
 else
   # Require app to produce a new enough version string.
   inst_ver=$(get_version $app)
   if [ ! "$inst_ver" ]; then
-warn "Error: '$app' not found"
+warn_ "Error: '$app' not found"
 ret=1
   else
 latest_ver=$(sort_ver $req_ver $inst_ver | cut -d' ' -f2)
 if [ ! "$latest_ver" = "$inst_ver" ]; then
-  warn "Error: '$app' version == $inst_ver is too old" \
-   "   '$app' version >= $req_ver is required"
+  warnf_ '%s\n'\
+  "Error: '$app' version == $inst_ver is too old"  \
+  "   '$app' version >= $req_ver is required"
   ret=1
 fi
   fi
@@ -761,7 +778,7 @@ version_controlled_file() {
 grep -F "/${file##*/}/" "$parent/CVS/Entries" 2>/dev/null |
  grep '^/[^/]*/[0-9]' > /dev/null
   else
-warn "no version control for $file?"
+warn_ "no version control for $file?"
 false
   fi
 }
-- 
1.7.11.1

Re: add --match argument to git-version-gen

[Andy Wingo]

On Sat 07 Jul 2012 22:42, Jim Meyering  writes:

>> -   --prefix   prefix of git tags (default 'v')
>> +   --prefix prefix of git tags to strip from version (default 'v')
>> +   --match  pattern for git tags to match
>
> Please mention that it's a "glob" pattern,
> so people don't think it's a regexp.

Done.

>> +function massage_git_version() {
>> +local v=$1
>
> Using the "function" and "local" keywords would make this script
> fail for some older shells.  I'd like to remain portable to those.

Bummer.  OK, fixed.  These functions no longer take or print values,
they just set the $v variable.

> Oh, and your patch added at least one trailing blank.

Fixed.

Thanks for the review, new patch attached.

Andy

>From f345ebb35f1510f61cc72f44fd67421ad4ff117c Mon Sep 17 00:00:00 2001
From: Andy Wingo 
Date: Fri, 6 Jul 2012 17:27:37 +0200
Subject: [PATCH] git-version-gen: add --match argument.

* build-aux/git-version-gen (Options): Add --match argument.
  (match):
  (tag_sed_script):
  (tarball_version_file): Protect against ambient values of these
  variables.
  (git_describe, massage_git_version): New helper functions.  Require a
  match in the --match case, and otherwise have our previous behavior.

Using --match can prevent a merge from e.g. a version 2 branch to a 3
branch from causing the 3 branch to show a 2 version, if the 2 version
tagged more recently than the 3 version.
---
 build-aux/git-version-gen |   69 +++--
 1 file changed, 48 insertions(+), 21 deletions(-)

diff --git a/build-aux/git-version-gen b/build-aux/git-version-gen
index 0fa9063..f80a9fb 100755
--- a/build-aux/git-version-gen
+++ b/build-aux/git-version-gen
@@ -1,6 +1,6 @@
 #!/bin/sh
 # Print a version string.
-scriptversion=2012-03-18.17; # UTC
+scriptversion=2012-07-10.09; # UTC
 
 # Copyright (C) 2007-2012 Free Software Foundation, Inc.
 #
@@ -85,20 +85,27 @@ Print a version string.
 
 Options:
 
-   --prefix   prefix of git tags (default 'v')
+   --prefix prefix of git tags to strip from version (default 'v')
+   --match  glob pattern for git tags to match
 
-   --help display this help and exit
-   --version  output version information and exit
+   --help   display this help and exit
+   --versionoutput version information and exit
 
-Running without arguments will suffice in most cases."
+Running without arguments will suffice in most cases.  If no --match
+argument is given, try to match tags that begin with the --prefix,
+falling back to the first tag that git-describe finds."
 
 prefix=v
+match=
+tag_sed_script=
+tarball_version_file=
 
 while test $# -gt 0; do
   case $1 in
 --help) echo "$usage"; exit 0;;
 --version) echo "$version"; exit 0;;
 --prefix) shift; prefix="$1";;
+--match) shift; match="$1";;
 -*)
   echo "$0: Unknown option '$1'." >&2
   echo "$0: Try '--help' for more information." >&2
@@ -144,21 +151,7 @@ then
 && echo "$0: WARNING: $tarball_version_file is missing or damaged" 1>&2
 fi
 
-if test -n "$v"
-then
-: # use $v
-# Otherwise, if there is at least one git commit involving the working
-# directory, and "git describe" output looks sensible, use that to
-# derive a version string.
-elif test "`git log -1 --pretty=format:x . 2>&1`" = x \
-&& v=`git describe --abbrev=4 --match="$prefix*" HEAD 2>/dev/null \
-  || git describe --abbrev=4 HEAD 2>/dev/null` \
-&& v=`printf '%s\n' "$v" | sed "$tag_sed_script"` \
-&& case $v in
- $prefix[0-9]*) ;;
- *) (exit 1) ;;
-   esac
-then
+massage_git_version() {
 # Is this a new git that lists number of commits since the last
 # tag or the previous older version that did not?
 #   Newer: v6.10-77-g0f8faeb
@@ -182,7 +175,41 @@ then
 
 # Change the first '-' to a '.', so version-comparing tools work properly.
 # Remove the "g" in git describe's output string, to save a byte.
-v=`echo "$v" | sed 's/-/./;s/\(.*\)-g/\1-/'`;
+v=`echo "$v" | sed 's/-/./;s/\(.*\)-g/\1-/'`
+}
+
+# If successful, sets 'v'.
+git_describe() {
+# Make sure this working directory has commits.
+test "`git log -1 --pretty=format:x . 2>&1`" = x || exit 1
+
+# If the user specified --match, look for a matching tag.
+# Otherwise, look for a tag starting with --prefix, or the first tag
+# we find.
+if test -n "$match"
+then
+v=`git describe --abbrev=4 --match="$match" HEAD 2>/dev/null` || exit 1
+else
+v=`git describe --abbrev=4 --match="$prefix*" HEAD 2>/dev/null \
+|| git describe --abbrev=4 HEAD 2>/dev/null` || exit 1
+fi
+
+v=`printf '%s\n' "$v" | sed "$tag_sed_script"` || exit 1
+case $v in
+$prefix[0-9]*)
+massage_git_version
+;;
+*)
+exit 1
+;;
+esac
+}
+
+if test -n "$v"
+then
+: # use $v
+elif git_describe
+then
 v_from_git=1
 else
 v=UNKNOWN

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

[Jim Meyering]

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.

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

[Akim Demaille]

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.

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

[Jim Meyering]

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.
>
> 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.

request to relax license of 'stat-time'

[Eric Blake]

Libvirt would like to use lib/stat-time.h to portably get at nanosecond
timestamps from an LGPLv2+ license; but this file is currently licensed
LGPLv3+.  Any objections to relaxing the license, particularly since
POSIX 2008 guarantees that stat() should be returning this information
and the stat module is already LGPLv2+?

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org




signature.asc
Description: OpenPGP digital signature

Re: request to relax license of 'stat-time'

[Jim Meyering]

Eric Blake wrote:
> Libvirt would like to use lib/stat-time.h to portably get at nanosecond
> timestamps from an LGPLv2+ license; but this file is currently licensed
> LGPLv3+.  Any objections to relaxing the license, particularly since
> POSIX 2008 guarantees that stat() should be returning this information
> and the stat module is already LGPLv2+?

That is fine with me.
(Bruno and Paul should also sign off)
Note that st_mtim.m4 (predecessor of stat-time.m4) had an
all-permissive copyright notice:

commit 44d7c1c1f004fa9bfb8ec3948de2671a7958cf5b
Author: Paul Eggert 
Date:   Sun Jan 23 08:06:57 2005 +

Use an all-permissive copyright notice, recommended by RMS.

Re: request to relax license of 'stat-time'

[Paul Eggert]

On 07/10/2012 06:29 AM, Jim Meyering wrote:
> That is fine with me.
> (Bruno and Paul should also sign off)

Fine with me too.

Re: request to relax license of 'stat-time'

[Bruno Haible]

Eric Blake wrote:
> > Libvirt would like to use lib/stat-time.h to portably get at nanosecond
> > timestamps from an LGPLv2+ license; but this file is currently licensed
> > LGPLv3+.  Any objections to relaxing the license, particularly since
> > POSIX 2008 guarantees that stat() should be returning this information
> > and the stat module is already LGPLv2+?

Jim Meyering wrote:
> That is fine with me.
> (Bruno and Paul should also sign off)

Fine with me as well.
We also need James' signoff, for the 2007-03-27 commit.

Bruno

Re: bug#11905: Unnecessary '#define unix' in src/s/aix4-2.h

[Paul Eggert]

On 07/10/2012 04:25 PM, Glenn Morris wrote:
> I suspect that Gnulib already has the AIX
> case covered (via "__unix"?), so that src/s/aix4-2.h no longer needs to
> define "unix".

Yes, that sounds right.  I have started to fix this by
installing the following patch into gnulib, and will
propagate this into Emacs shortly.

---
 ChangeLog|9 +
 lib/getloadavg.c |   28 +++-
 m4/getloadavg.m4 |2 ++
 3 files changed, 14 insertions(+), 25 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d94f7b1..b27ef8d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2012-07-10  Paul Eggert  
+
+   getloadavg: clean out old Emacs and Autoconf cruft
+   See Glenn Morris in .
+   * lib/getloadavg.c: Include ,  always.
+   Include  if HAVE_SYS_PARAM_H, not if unix or __unix.
+   (LDAV_CVT): Remove no-longer-used LOAD_AVE_CVT hook.
+   * m4/getloadavg.m4 (gl_PREREQ_GETLOADAVG): Check for .
+
 2012-07-10  Akim Demaille  
 
bootstrap: let warn be like tests/init.sh's warn_
diff --git a/lib/getloadavg.c b/lib/getloadavg.c
index 79ea717..a8ffefe 100644
--- a/lib/getloadavg.c
+++ b/lib/getloadavg.c
@@ -80,45 +80,23 @@
We also #define LDAV_PRIVILEGED if a program will require
special installation to be able to call getloadavg.  */
 
-/* "configure" defines CONFIGURING_GETLOADAVG to sidestep problems
-   with partially-configured source directories.  */
-
-#ifndef CONFIGURING_GETLOADAVG
-# include 
-# include 
-#endif
+#include 
 
 /* Specification.  */
 #include 
 
 #include 
+#include 
 #include 
 
 # include 
 
-/* Both the Emacs and non-Emacs sections want this.  Some
-   configuration files' definitions for the LOAD_AVE_CVT macro (like
-   sparc.h's) use macros like FSCALE, defined here.  */
-# if defined (unix) || defined (__unix)
+# if HAVE_SYS_PARAM_H
 #  include 
 # endif
 
 # include "intprops.h"
 
-/* The existing Emacs configuration files define a macro called
-   LOAD_AVE_CVT, which accepts a value of type LOAD_AVE_TYPE, and
-   returns the load average multiplied by 100.  What we actually want
-   is a macro called LDAV_CVT, which returns the load average as an
-   unmultiplied double.
-
-   For backwards compatibility, we'll define LDAV_CVT in terms of
-   LOAD_AVE_CVT, but future machine config files should just define
-   LDAV_CVT directly.  */
-
-# if !defined (LDAV_CVT) && defined (LOAD_AVE_CVT)
-#  define LDAV_CVT(n) (LOAD_AVE_CVT (n) / 100.0)
-# endif
-
 # if !defined (BSD) && defined (ultrix)
 /* Ultrix behaves like BSD on Vaxen.  */
 #  define BSD
diff --git a/m4/getloadavg.m4 b/m4/getloadavg.m4
index 029da44..7738d2f 100644
--- a/m4/getloadavg.m4
+++ b/m4/getloadavg.m4
@@ -105,6 +105,8 @@ AC_DEFUN([gl_PREREQ_GETLOADAVG],
 [
 # Figure out what our getloadavg.c needs.
 
+AC_CHECK_HEADERS_ONCE([sys/param.h])
+
 # On HPUX9, an unprivileged user can get load averages this way.
 if test $gl_func_getloadavg_done = no; then
   AC_CHECK_FUNCS([pstat_getdynamic], [gl_func_getloadavg_done=yes])
-- 
1.7.6.5