[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Automake-NG] [PATCH v2 2/2] dist: add back support for obsolete dis
From: |
Stefano Lattarini |
Subject: |
Re: [Automake-NG] [PATCH v2 2/2] dist: add back support for obsolete dist-* options |
Date: |
Wed, 22 Aug 2012 13:47:05 +0200 |
Hi Paolo.
Since I still have some gripes with the preparatory [PATCH 1/2], I'm
thinking about reworking this patch to make is independent from that.
Find my ideas below. Do you think they would be a good move? If
yes, would you mind re-working you patch accordingly?
On 08/22/2012 12:44 PM, Paolo Bonzini wrote:
> The old API for dist formats can be supported easily, by parsing the
> AUTOMAKE_OPTIONS and generating AM_DIST_FORMATS from it, if not defined
> otherwise.
>
> * NG-NEWS: Document that the old distribution format API is obsolete
> but available.
> * lib/Automake/Options.pm: Degrade error to warning.
> * lib/am/distcheck.mk: Generate AM_DIST_FORMATS from AUTOMAKE_OPTIONS
> * t/dist-obsolete-opts.sh: Add backwards-compatibility tests.
> ---
> NG-NEWS | 2 +-
> lib/Automake/Options.pm | 8 ++++----
> lib/am/distcheck.mk | 4 +++-
> t/dist-obsolete-opts.sh | 24 +++++++++++++++---------
> 4 file modificati, 23 inserzioni(+), 15 rimozioni(-)
>
> diff --git a/NG-NEWS b/NG-NEWS
> index 129da68..8ee1aae 100644
> --- a/NG-NEWS
> +++ b/NG-NEWS
> @@ -228,7 +228,7 @@ Distribution
> have been removed; and with them the targets 'dist-shar' and 'dist-tarZ'.
>
> * The API to specify the formats of distribution tarballs has been changed
> - completely.
> + completely; the old API is still available, but gives warnings.
>
> Instead of using the various 'dist-*' automake options, the developer is
> now expected to specify the default formats of its distribution tarballs
> diff --git a/lib/Automake/Options.pm b/lib/Automake/Options.pm
> index 8dd8176..abf488c 100644
> --- a/lib/Automake/Options.pm
> +++ b/lib/Automake/Options.pm
> @@ -322,11 +322,11 @@ sub _process_option_list (\%@)
> {
> error $where, "support for Cygnus-style trees has been removed";
> }
> - elsif (/^(?:no-)?dist-.*/)
> + elsif ($_ eq 'no-dist-gzip' || /^dist-.*/)
> {
> - error ($where,
> - "'$_' option and the like are no more supported;\n" .
> - "use AM_DIST_FORMATS in top-level Makefile.am instead");
> + msg ('obsolete', $where,
> + "'$_' option and the like are obsolete; use\n" .
> + "AM_DIST_FORMATS in top-level Makefile.am instead");
> }
>
Since we are still processing these options explicitly, we can easily know
which dist formats the user is requesting. So, instead of having to recover
them from the 'dist-*' options at make runtime, we could register them right
away in an internal make variable, say '$(am.dist.formats-from-opts)';
and then, at make runtime, default AM_DIST_FORMATS to that. Something like
this:
sub _process_option_list (\%@)
{
...
my %dist_formats = (gzip => 1);
...
elsif ($_ eq 'no-dist-gzip' || /^dist-(.*)/)
{
msg ('obsolete', $where,
"'$_' option and the like are obsolete; use\n" .
"AM_DIST_FORMATS in top-level Makefile.am instead");
if ($_ eq 'no-dist-gzip')
{
delete $dist_formats{'gzip'};
}
else
{
$dist_formats{$1} = 1;
}
}
}
Then we should find a way to pass the keys of '%dist_formats' back
to Automake, and use it to define '$(am.dist.formats-from-opts)'.
Then ...
> diff --git a/lib/am/distcheck.mk b/lib/am/distcheck.mk
> index 421b15b..cbde58b 100644
> --- a/lib/am/distcheck.mk
> +++ b/lib/am/distcheck.mk
> @@ -69,7 +69,9 @@ am.dist.extract-cmd.zip = \
>
> # This is namespace-safe, so it's OK to accept values from
> # the environment.
> -AM_DIST_FORMATS ?= gzip
> +AM_DIST_FORMATS ?= \
> + $(patsubst dist-%, %, $(filter dist-%, $(am.automake-options))) \
> + $(if $(filter no-dist-gzip, $(am.automake-options)),,gzip)
>
... here we could simply do:
AM_DIST_FORMATS ?= $(am.dist.formats-from-opts)
How does that sound?
-*-*-*-
Now, to some style/minor nits ...
> am.dist.bad-targets := \
> $(filter-out $(am.dist.all-formats),$(AM_DIST_FORMATS))
> diff --git a/t/dist-obsolete-opts.sh b/t/dist-obsolete-opts.sh
> index 06bea8d..018fc1c 100644
> --- a/t/dist-obsolete-opts.sh
> +++ b/t/dist-obsolete-opts.sh
> @@ -22,9 +22,9 @@ $ACLOCAL
>
> for fmt in gzip bzip2 xz lzip zip tarZ lzma shar; do
> echo AUTOMAKE_OPTIONS = dist-$fmt > Makefile.am
> - AUTOMAKE_fails -Wnone -Wno-error
> - grep "^Makefile\\.am:1:.* 'dist-$fmt' option.* no more supported" stderr
> - grep "^Makefile\\.am:1:.* use AM_DIST_FORMATS .*instead" stderr
> + $AUTOMAKE -Wobsolete -Wno-error 2>stderr
> + grep "^Makefile\\.am:1:.* 'dist-$fmt' option.* obsolete" stderr
> + grep "^Makefile\\.am:1:.* AM_DIST_FORMATS .*instead" stderr
>
Here is the usual idiomatic way to check for warnings:
AUTOMAKE_fails -Wnone -Wobsolete
grep "^Makefile\\.am:1:.* 'dist-$fmt' option.* obsolete" stderr
grep "^Makefile\\.am:1:.* AM_DIST_FORMATS .*instead" stderr
$AUTOMAKE -Wno-obsolete
> done
>
> rm -rf autom4te*.cache
> @@ -33,12 +33,18 @@ cat > configure.ac << 'END'
> AC_INIT([foo], [1.0])
> AM_INIT_AUTOMAKE([no-dist-gzip dist-xz])
> AC_CONFIG_FILES([Makefile])
> +AC_OUTPUT
> END
> -: > Makefile.am
> +: > foo
> +echo EXTRA_DIST = foo > Makefile.am
> $ACLOCAL
> -AUTOMAKE_fails -Wnone -Wno-error
> -grep "^configure\\.ac:2:.* 'no-dist-gzip' option.* no more supported" stderr
> -grep "^configure\\.ac:2:.* 'dist-xz' option.* no more supported" stderr
> -grep "^configure\\.ac:2:.*use AM_DIST_FORMATS .*instead" stderr
> +$AUTOMAKE -Wobsolete -Wno-error 2>stderr
>
Please use:
AUTOMAKE_run -Wobsolete -Wno-error
instead. It will do the stder r capture for you.
> +grep "^configure\\.ac:2:.* 'no-dist-gzip' option.* obsolete" stderr
> +grep "^configure\\.ac:2:.* 'dist-xz' option.* obsolete" stderr
> +grep "^configure\\.ac:2:.* AM_DIST_FORMATS .*instead" stderr
>
> -:
> +$AUTOCONF
> +./configure
> +$MAKE dist
>
This will require a working 'xz' program, so you should ensure is is present
before proceeding, to avoid spurious failures on systems lacking it.
if xz --version; then
$MAKE dist
test -f foo-1.0.tar.xz
test -f foo-1.0.tar.gz && exit 1
: For shells with busted 'set -e'.
fi
:
Thanks,
Stefano
- [Automake-NG] [PATCH v2 0/2] dist: add back support for obsolete dist-* options, Paolo Bonzini, 2012/08/22
- [Automake-NG] [PATCH v2 1/2] var: format all options in the Makefile.in output, Paolo Bonzini, 2012/08/22
- [Automake-NG] [PATCH v2 2/2] dist: add back support for obsolete dist-* options, Paolo Bonzini, 2012/08/22
- Re: [Automake-NG] [PATCH v2 2/2] dist: add back support for obsolete dist-* options,
Stefano Lattarini <=
- Re: [Automake-NG] [PATCH v2 2/2] dist: add back support for obsolete dist-* options, Paolo Bonzini, 2012/08/22
- Re: [Automake-NG] [PATCH v2 2/2] dist: add back support for obsolete dist-* options, Stefano Lattarini, 2012/08/22
- Re: [Automake-NG] [PATCH v2 2/2] dist: add back support for obsolete dist-* options, Paolo Bonzini, 2012/08/22
- Re: [Automake-NG] [PATCH v2 2/2] dist: add back support for obsolete dist-* options, Stefano Lattarini, 2012/08/22
- Re: [Automake-NG] [PATCH v2 2/2] dist: add back support for obsolete dist-* options, Paolo Bonzini, 2012/08/22
- Re: [Automake-NG] [PATCH v2 2/2] dist: add back support for obsolete dist-* options, Stefano Lattarini, 2012/08/22
- Re: [Automake-NG] [PATCH v2 2/2] dist: add back support for obsolete dist-* options, Paolo Bonzini, 2012/08/22
- Re: [Automake-NG] [PATCH v2 2/2] dist: add back support for obsolete dist-* options, Stefano Lattarini, 2012/08/22