Re: [PATCH] maint: add a rule to use gnulib's update-copyright (was: Re: [PATCH] tests: avoid spurious new makefile-deps failure)

[Stefano Lattarini]

On 02/12/2012 05:29 PM, Jim Meyering wrote:
> Stefano Lattarini wrote:
> 
>> Yes, I've seen this new failure.  But the best fix is IMHO to improve the
>> new 'am__make_dryrun' instead (which is proving a little to much brittle
>> in other contexts).  This spurious failure will probably disappear by
>> itself after such a fix, so I'd rather avoid tweaking the 'makefile-deps'
>> test at all.
> 
> Hi Stefano,
> 
> That's fine by me.
>
> However, if you know of such test failures, it'd be nice to mark
> them as XFAIL as soon as you can -- even if just for a few days --
> to avoid wasted effort.
>
But by doing so we would risk to let such failures "slips through the
cracks"...  Still, having the affected tests printing a proper message
"I'm temporary failing due to commit ..." might help saving wasted
work.  I'll try to be more careful next time.  Sorry for the confusion
and sloppiness.

>>> I hesitated to update copyright (what's automake's policy?)
>>> since I'm used to performing a mass-update in early January so
>>> that patches aren't constantly polluted with a distracting
>>> copyright-updating hunk throughout each year.
>>>
>> I think automake should begin to follow this simpler and more rational
>> policy as well.  Care to write a patch?
>
> I would propose a rule like this one from maint.mk:
>
> .PHONY: update-copyright
> update-copyright:
>       grep -l -w Copyright                                             \
>         $$(export VC_LIST_EXCEPT_DEFAULT=COPYING && $(VC_LIST_EXCEPT)) \
>         | $(update-copyright-env) xargs $(srcdir)/$(_build-aux)/$@
>
'update-copyright-env' is not portable as the name of a macro name ...  Oh,
but I see you have already addressed that in your the patch below.  Nevermind
then.

> with these envvar settings:
> 
> update-copyright-env = \
>   UPDATE_COPYRIGHT_FORCE=1 \
>   UPDATE_COPYRIGHT_USE_INTERVALS=2 \
>
Why the "2" here?  The documentation in update-copyright reads:

  If UPDATE_COPYRIGHT_USE_INTERVALS=1, every series of consecutive
  copyright years (such as 90, 1991, 1992-2007, 2008) in a
  reformatted FSF copyright statement is collapsed to a single
  interval (such as 1990-2008).  If unset or set to 0, all existing
  copyright year intervals in a reformatted FSF copyright statement
  are expanded instead.

So we should just export UPDATE_COPYRIGHT_USE_INTERVALS to "1",
right?

But then, a more serious issue: are we truly and legally allowed to transform
a copyright notice like:

  Copyright (C) 1998, 2002, 2010, 2011 Free Software Foundation, Inc.

into this:

  Copyright (C) 1998-2012 Free Software Foundation, Inc.

without problems?  (I hope so, because I'd love the gain in compactness and
consistency).

>   UPDATE_COPYRIGHT_MAX_LINE_LENGTH=79
>
I'd rather keep all the lines shorter than 76 chars if possible (this leaves
room for leading line numbers in a vim buffer, even for terminals with only
80 chars per line).  Maybe leaving the default of 72 here would be even
better.  And BTW, if we go on with collapsing the ranges of copyright years,
all of this becomes moot, because then all the lines containing the copyright
years will simply be 54 characters long, no?

> Then I would propose the large, boring change induced
> by running "make update-copyright".
>
I agree it's a good idea to keep this change separate.

> Here's the first patch:
> (I don't particularly like to copy update-copyright from gnulib and
> VC it here in automake, but that's the current method.  Addressing that,
> if desired, would be a separate issue. )
>
Agreed.  What would you propose to address that?  (I'm assuming you'll agree
with me that having gnulib as an automake git submodule would be a *gross*
overkill).

> From fd93630e5c6aa9a9775ae945787b9903bab2f6c6 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <address@hidden>
> Date: Sun, 12 Feb 2012 16:57:42 +0100
> Subject: [PATCH] maint: add a rule to use gnulib's update-copyright
>
I assume this patch is for the 'maint' branch, right?

> * lib/update-copyright: New file, from gnulib.
> * Makefile.am (FETCHFILES): Add update-copyright to the list.
> (fetch): Fetch it.
> (update-copyright): New rule.
> ---
>  Makefile.am          |   14 +++-
>  lib/update-copyright |  274 
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 287 insertions(+), 1 deletions(-)
>  create mode 100755 lib/update-copyright
> 
> diff --git a/Makefile.am b/Makefile.am
> index a493e3f..6a6631f 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -340,7 +340,8 @@ config.guess \
>  config.sub \
>  gnupload \
>  gitlog-to-changelog \
> -texinfo.tex
> +texinfo.tex \
> +update-copyright
> 
>  ## Fetch the latest versions of files we care about.
>  fetch:
> @@ -353,6 +354,7 @@ fetch:
>       $(WGET_SV_CVS)texinfo/texinfo/doc/texinfo.tex -O texinfo.tex && \
>       $(WGET_SV_GIT_GL)doc/INSTALL -O INSTALL && \
>       $(WGET_SV_GIT_GL)build-aux/gnupload -O gnupload && \
> +     $(WGET_SV_GIT_GL)build-aux/update-copyright -O update-copyright && \
>       $(WGET_SV_GIT_GL)build-aux/gitlog-to-changelog -O gitlog-to-changelog)
>  ## Don't exit after test because we want to give as many errors as
>  ## possible.
> @@ -400,3 +402,13 @@ release-stats: ps
>       printf '@item %s @tab %-6s @tab %4d @tab %4d @tab %4d @tab %4d %-4s 
> @tab %4d %-4s @tab %3d @tab %d %-4s\n' \
>                     $$today $(VERSION) $$am    $$acl    $$pm   $$aml 
> "($$amf)" $$m4l "($$m4f)" $$doc $$t "($$tgen)"
>  .PHONY: release-stats
> +
> +update_copyright_env = \
> +  UPDATE_COPYRIGHT_FORCE=1 \
> +  UPDATE_COPYRIGHT_USE_INTERVALS=2 \
> +  UPDATE_COPYRIGHT_MAX_LINE_LENGTH=79
>
See comments above.

> +
> +.PHONY: update-copyright
> +update-copyright:
> +     git ls-files | grep -Ev 'COPYING|INSTALL' \
> +       | $(update_copyright_env) xargs $(srcdir)/lib/$@

> diff --git a/lib/update-copyright b/lib/update-copyright
> new file mode 100755
> index 0000000..082b749
> --- /dev/null
> +++ b/lib/update-copyright
>
> [SNIP]
>
Other than those little nits, the patch looks good.

Thank you very much for having tackled this,
  Stefano