Re: check-news target fails if NEWS file starts with a copyright header

[Jim Meyering]

Peter Simons wrote:
> Karl Berry writes:
>  > However, I also see no harm in looking at the first 20 lines instead
>  > of the first 10 lines to match the version.
>
> yes, I agree with Karl here. The current implementation enforces a
> policy that feels rather odd. A NEWS file that features the release
> heading in line 10 is considered valid, but one that features the
> heading in line 11 is not. This is really quite arbitrary and not
> particularly intuitive.

The penalty for an inadequate test would be to release without updating NEWS.
That particular file is so visible that I think it's worth making the default 
test
very strict.

As with nearly all gnulib-imported files, if you don't like
the way it's done by default, you have options:

  - disable this particular test
  - patch the file (via gnulib-tool) to test the way you'd prefer

FYI, other tools (build-aux/do-release-commit-and-tag) require
that line 3 of NEWS contain the magic line, so relaxing this test to
look at 10 lines was already throwing a bone to those who do things differently.

$ build-aux/do-release-commit-and-tag --help
Usage: do-release-commit-and-tag VERSION RELEASE_TYPE

Run this script to perform the final pre-release NEWS update
in which the date, release-type and version string are recorded.
Commit that result with a log entry marking the release, and apply
a signed tag.  Run it from your project's top-level directory.

Requirements:
- you use git for version-control
- a NEWS file, with line 3 identical to this:
* Noteworthy changes in release ?.? (-??-??) [?]
- a version-controlled .prev-version file

Options:
  --help print this help, then exit
  --version  print version number, then exit

EXAMPLE:
To update NEWS and tag the beta 8.1 release of coreutils, I would run this:

  do-release-commit-and-tag 8.1 beta

Report bugs and patches to .

Re: check-news target fails if NEWS file starts with a copyright header

[Peter Simons]

Jim Meyering wrote:

 > The penalty for an inadequate test would be to release without updating
 > NEWS.

so, according to your logic, expecting any of the top 10 lines of NEWS to
match a particular regular expression is "adequate", but expecting any of
the top, say 11 lines, of NEWS to match that expression is "inadequate"?

This reasoning doesn't make much sense to me. I think it's pretty obvious
that the choice of the 10 line limit is perfectly random in this context.
That number is inherently meaningless. Any other number, say 17 or 32, would
work just as well for all practical purposes. My impression is that your
opinion is based on irrational fears ("if we change the code, bad things
will happen") rather than on rational thought.

Take care,
Peter

Re: check-news target fails if NEWS file starts with a copyright header

[Jim Meyering]

Peter Simons wrote:
> Jim Meyering wrote:
>  > The penalty for an inadequate test would be to release without updating
>  > NEWS.
>
> so, according to your logic, expecting any of the top 10 lines of NEWS to
> match a particular regular expression is "adequate", but expecting any of
> the top, say 11 lines, of NEWS to match that expression is "inadequate"?
>
> This reasoning doesn't make much sense to me. I think it's pretty obvious
> that the choice of the 10 line limit is perfectly random in this context.
> That number is inherently meaningless. Any other number, say 17 or 32, would
> work just as well for all practical purposes. My impression is that your
> opinion is based on irrational fears ("if we change the code, bad things
> will happen") rather than on rational thought.

My wanting to minimize risk is now "irrational fears"?
If you'd ever been bitten by a false-positive match (I have) and
nearly released with out-of-date NEWS, you wouldn't call it irrational.

>From your tone, I wonder if you read this message:

  http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/22023/focus=22035

If it were just for my projects (as it was for years), it'd match
only on line 3, because all of my NEWS files are consistent that way.
The choice of 10 is somewhat arbitrary, but I don't want to use 15 or 20.
10 is large enough to accommodate every project that uses it so far,
while searching only 7 additional lines of change descriptions for most
(each of which represents a small risk of a false-positive match --
it is not uncommon to mention "fixed in version M.N"), but typically
no more than 6 of those additional lines are non-empty, and the odds of
getting an FP match in that range seems acceptable.

Is there some reason you'd rather not move the copyright notice
to the end of that file?

If you're motivated, propose a patch.
Make it search only lines 1..10 by default, but
allow that to be overridden via cfg.mk to search say,
only line 3 or lines 20..22.

Re: check-news target fails if NEWS file starts with a copyright header

[Peter Simons]

Jim Meyering wrote:

 > My wanting to minimize risk is now "irrational fears"?

No, of course not. The desire to minimize risks is perfectly reasonable.
However, your perception of the risks involved in this change strikes me
as somewhat exaggerated.


 > Is there some reason you'd rather not move the copyright notice to
 > the end of that file?

No, there is no particular reason, except that I'd have to change
several other small files, too, to keep them consistent within the
project. Since there are plenty of simple ways to avoid making that
change, I'd rather not make it.


 > If you're motivated, propose a patch.

My suggested solution is attached.

Thank you for your time and effort,
Peter

>From 34abe8ff8a000f650b22935f36240d9acda66439 Mon Sep 17 00:00:00 2001
From: Peter Simons 
Date: Mon, 7 Jun 2010 15:24:21 +0200
Subject: [PATCH] Added news-check-lines-limit variable to determine the number 
of lines that the
 'news-check' target inspects in order to find a heading for the current
 release's version number. The default value is "10". The variable can be
 overidden in cfg.mk.

---
 ChangeLog|7 +++
 top/maint.mk |5 +++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 3eb9e3a..f8bcd47 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2010-06-07  Peter Simons  
+
+* top/maint.mk: Added news-check-lines-limit variable to determine
+the number of lines that the 'news-check' target inspects in order
+to find a heading for the current release's version number. The
+default value is "10". The variable can be overidden in cfg.mk.
+
 2010-06-07  Jim Meyering  
 
do-release-commit-and-tag: fix typo in --help
diff --git a/top/maint.mk b/top/maint.mk
index 644fbb6..68c3407 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -104,6 +104,7 @@ endif
 # NEWS file.
 today = $(shell date +%Y-%m-%d)
 news-check-regexp ?= '^\*.* $(VERSION_REGEXP) \($(today)\)'
+news-check-lines-limit ?= 10
 
 # Prevent programs like 'sort' from considering distinct strings to be equal.
 # Doing it here saves us from having to set LC_ALL elsewhere in this file.
@@ -874,8 +875,8 @@ sc_makefile_at_at_check:
  && { echo '$(ME): use $$(...), not @...@' 1>&2; exit 1; } || :
 
 news-check: NEWS
-   if head $(srcdir)/NEWS | grep -E $(news-check-regexp)   \
-   >/dev/null; then\
+   if head -n $(news-check-lines-limit) $(srcdir)/NEWS \
+   | grep -E $(news-check-regexp) >/dev/null; then \
  :;\
else\
  echo 'NEWS: $$(news-check-regexp) failed to match' 1>&2;  \
-- 
1.7.1

Re: check-news target fails if NEWS file starts with a copyright header

[Jim Meyering]

Peter Simons wrote:
...
>  > If you're motivated, propose a patch.
>
> My suggested solution is attached.

Thanks, but that does not allow us to select ranges like
"just line 3" or "lines 20-22".
How about removing the use of "head" and using this as the default:

sed -n 1,10p

[PATCH] describe --local-dir option in documentation

[Ben Pfaff]

I couldn't find a description of how or why to use --local-dir,
so I wrote this up.  OK to push it?

Thanks,

Ben.

--8<--cut here-->8--

>From e5c8b4f4b93bef75db1680d4473958337ebfbd07 Mon Sep 17 00:00:00 2001
From: Ben Pfaff 
Date: Mon, 7 Jun 2010 22:35:00 -0700
Subject: [PATCH] Update 'gnulib-tool' documentation.

* doc/gnulib-tool.texi (Local changes): New section.
* doc/gnulib-intro.texi (Openness): Add cross-reference to new section.
---
 ChangeLog |7 +++
 doc/gnulib-intro.texi |4 ++--
 doc/gnulib-tool.texi  |   32 
 3 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 40bd312..e723303 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2010-06-07  Ben Pfaff  
+
+   Update 'gnulib-tool' documentation.
+   * doc/gnulib-tool.texi (Local changes): New section.
+   * doc/gnulib-intro.texi (Openness): Add cross-reference to new
+   section.
+
 2010-05-23  Ben Pfaff  
 
Update maintainer documentation for 'relocatable-prog' module.
diff --git a/doc/gnulib-intro.texi b/doc/gnulib-intro.texi
index 4cb86b8..aa5f9ca 100644
--- a/doc/gnulib-intro.texi
+++ b/doc/gnulib-intro.texi
@@ -346,5 +346,5 @@ locally add modules that are treated like Gnulib modules by
 @code{gnulib-tool}.
 @end enumerate
 
-This is achieved by the @samp{--local-dir} option of @code{gnulib-tool}.
-
+This is achieved by the @samp{--local-dir} option of @code{gnulib-tool}
+(@pxref{Local changes}).
diff --git a/doc/gnulib-tool.texi b/doc/gnulib-tool.texi
index 02bb89a..09415be 100644
--- a/doc/gnulib-tool.texi
+++ b/doc/gnulib-tool.texi
@@ -48,6 +48,7 @@ a real run without changing anything.
 * Localization::Handling Gnulib's own message translations.
 * VCS Issues::  Integration with Version Control Systems.
 * Unit tests::  Bundling the unit tests of the Gnulib modules.
+* Local changes::   Patching Gnulib to suit your project.
 @end menu
 
 
@@ -672,3 +673,34 @@ Note: In packages which use more than one invocation of 
@code{gnulib-tool}
 in the scope of the same @code{configure.ac}, you cannot use
 @samp{--with-tests}.  You will have to use a separate @code{configure.ac}
 in this case.
+
+
+...@node Local changes
+...@section Patching Gnulib to suit your project
+
+Gnulib modules are intended to be suitable for widespread use.  Most
+problems with Gnulib can and should be fixed in a generic way, so that all
+of Gnulib's users can benefit from the change.  But occasionally a problem
+arises that is difficult or undesirable to fix generically, or a project
+that uses Gnulib may need to work around an issue before the Gnulib
+maintainers commit a final fix.
+
+Whatever the reason, the @samp{--local-d...@var{directory}} option to
+...@command{gnulib-tool} may be used to apply and maintain local changes to
+Gnulib modules.  If this option is specified, then @command{gnulib-tool}
+looks in @fi...@var{directory}} whenever it reads a file from the Gnulib
+directory.  Suppose @command{gnulib-tool} is looking for @var{file}.  Then:
+
+...@itemize @bullet
+...@item
+If @fi...@var{directory}/@var{file}} exists, then @samp{gnulib-tool} uses
+it instead of the file included in Gnulib.
+
+...@item
+Otherwise, if @fi...@var{directory}/@var{file}.diff} exists, then
+...@command{gnulib-tool} uses the file from Gnulib after applying the diff
+using the @command{patch} program.
+
+...@item
+Otherwise, @command{gnulib-tool} uses the file included in Gnulib.
+...@end itemize
-- 
1.7.1



-- 
RMS on DRM: "This ought to be a crime. And, if we had governments of the
people, by the people, for the people, then the executives of those companies
would be in prison.  But they're not in prison, and the reason is that we have
government of the people, by the sell-outs, for the corporations."

relocwrapper interaction with replacement functions

[Ben Pfaff]

On OpenBSD 4.7, GNU PSPP from current "master" configured with
--enable-relocatable fails linking relocwrapper.c at "make
install" time:

/tmp//ccy8gNb6.o(.text+0x64): In function `add_dotbin':
gl/relocwrapper.c:105: undefined reference to `rpl_fprintf'
/tmp//ccy8gNb6.o(.text+0x172): In function `activate_libdirs':
gl/relocwrapper.c:165: undefined reference to `rpl_fprintf'
/tmp//ccy8gNb6.o(.text+0x1f9): In function `main':
gl/relocwrapper.c:189: undefined reference to `rpl_fprintf'
/tmp//ccHe4GuE.o(.text+0xe9): In function `set_relocation_prefix':
gl/relocatable.c:154: undefined reference to `libintl_set_relocation_prefix'
/tmp//ccIOlqzo.o(.text+0x3c): In function `rpl_strerror':
gl/strerror.c:339: undefined reference to `rpl_sprintf'
collect2: ld returned 1 exit status

One solution for the problems with the rpl_* symbols would be to
build the relocwrapper program in the usual way as part of a
Makefile.am, and then link it against the Gnulib library, so that
relocwrapper could take advantage of the wrappers.  I don't
actually know why it is not done that way already; I assume that
there must be good reason.  Maybe it is because the relocwrapper
passes special -D options when it compiles the source files, but
I think that Automake can handle that with per-target compilation
flags.

Another way would be to make the substitutions conditional on
!IN_RELOCWRAPPER, but that seems fragile for two reasons: first,
because Gnulib developers would need to be aware of the need to
check for IN_RELOCWRAPPER as they add more wrappers, and second,
because Gnulib developers would need to remember to avoid using
the features provided by the wrappers in contexts where the
wrappers might not always be present.

For libintl_set_relocation_prefix, I guess PSPP should just stop
using DEPENDS_ON_LIBINTL.  There is not much benefit to it
anyhow.

What do you think?

Thanks,

Ben.
-- 
Ben Pfaff 
http://benpfaff.org