From 037f7e43b6a9da6d0555449bb0db128944c24f9c Mon Sep 17 00:00:00 2001 From: Stefano Lattarini Date: Sat, 19 May 2012 19:34:06 +0200 Subject: [PATCH] [ng] vars: get rid of VAR_ASIS / VAR_PRETTY distinction It doesn't offer us any real advantage (apart from some eye-candy in the generated makefile), and complicates the code and the internal APIs. * automake.in (define_verbose_var, define_configure_variable, file_contents_internal): Adjust the 'Automake::Variable::define' call to drop the VAR_ASIS parameter. (define_pretty_variable): Likewise. Also, adjust the function description not to state that the value of a variable defined by it is "pretty printed in the output file". (handle_subdirs): Don't try to prettify the RECURSIVE_TARGETS variable anymore. (read_am_file): Adjust the 'Automake::Variable::define' calls to drop the VAR_ASIS parameter. Also, don't "prettify" the definition of variables whose value is more than 1000 characters long. That is now done directly ... * lib/Automake/Variable.pm (output): ... in here. (define): Drop the '$pretty' parameter, and adjust implementation details accordingly. Now output variable definitions are only prettified when needed, i.e., when their value is more than 1000 characters long. And the only reason this prettification is done is to ensure the generated Makefile.in won't have lines so long that could confuse tools (like sed and awk) processing it into a Makefile at config.status runtime. (Pod Documentation): Adjust. (_has_line_too_long): New internal function. (transform_variable_recursively): Drop '$pretty' parameter in a call to 'define'. * lib/Automake/VarDef.pm (VAR_ASIS, VAR_PRETTY): Delete. (@EXPORT): Don't advertise them. (new): Don't take nor store the '$pretty' parameter anymore. (pretty): Remove this accessor method. (Pod Documentation): Adjust. * t/check2.sh: Adjust. * t/distcom-subdir.sh: Likewise. * t/distcom2.sh: Likewise. * t/longline.sh: Likewise. * t/pluseq3.sh: Likewise. * t/subdir4.sh: Likewise. * t/subst-no-trailing-empty-line.sh: Remove as obsolete. Signed-off-by: Stefano Lattarini --- automake.in | 32 +++------- lib/Automake/VarDef.pm | 39 ++----------- lib/Automake/Variable.pm | 86 ++++++++++++--------------- t/check2.sh | 3 +- t/distcom-subdir.sh | 28 +++++++-- t/distcom2.sh | 19 +----- t/longline.sh | 19 ++++-- t/pluseq3.sh | 2 +- t/subdir4.sh | 9 +-- t/subst-no-trailing-empty-line.sh | 97 ------------------------------- 10 files changed, 90 insertions(+), 244 deletions(-) delete mode 100755 t/subst-no-trailing-empty-line.sh diff --git a/automake.in b/automake.in index 94a728dcd..b311c1088 100644 --- a/automake.in +++ b/automake.in @@ -1163,7 +1163,7 @@ sub define_verbose_var ($$) if (! vardef ($silent_var, TRUE)) { Automake::Variable::define ($silent_var, VAR_AUTOMAKE, '', TRUE, - $val, '', INTERNAL, VAR_ASIS); + $val, '', INTERNAL); } } @@ -3767,7 +3767,6 @@ sub handle_subdirs () if $dsubdirs; $output_rules .= &file_contents ('subdirs', new Automake::Location); - rvar ('RECURSIVE_TARGETS')->rdef (TRUE)->{'pretty'} = VAR_PRETTY; # Gross! } @@ -5906,8 +5905,7 @@ sub cond_stack_endif ($$$) # be defined conditionally. The second argument is the condition # under which the value should be defined; this should be the empty # string to define the variable unconditionally. The third argument -# is a list holding the values to use for the variable. The value is -# pretty printed in the output file. +# is a list holding the values to use for the variable. sub define_pretty_variable ($$$@) { my ($var, $cond, $where, @value) = @_; @@ -5915,7 +5913,7 @@ sub define_pretty_variable ($$$@) if (! vardef ($var, $cond)) { Automake::Variable::define ($var, VAR_AUTOMAKE, '', $cond, "@value", - '', $where, VAR_PRETTY); + '', $where); rvar ($var)->rdef ($cond)->set_seen; } } @@ -5955,7 +5953,7 @@ sub define_configure_variable ($) # substituted as `\`. return if exists $ignored_configure_vars{$var}; Automake::Variable::define ($var, VAR_CONFIGURE, '', TRUE, subst $var, - '', $configure_vars{$var}, VAR_ASIS); + '', $configure_vars{$var}); } @@ -6100,7 +6098,6 @@ sub read_am_file ($$) my $comment = ''; my $blank = 0; my $saw_bk = 0; - my $var_look = VAR_ASIS; use constant IN_VAR_DEF => 0; use constant IN_RULE_DEF => 1; @@ -6227,7 +6224,7 @@ sub read_am_file ($$) Automake::Variable::define ($last_var_name, VAR_MAKEFILE, $last_var_type, $cond, $last_var_value, $comment, - $last_where, VAR_ASIS) + $last_where) if $cond != FALSE; $comment = $spacing = ''; } @@ -6282,29 +6279,15 @@ sub read_am_file ($$) # 'sed's. $last_var_value = $3 . "\n"; } - # Normally we try to output variable definitions in the - # same format they were input. However, POSIX compliant - # systems are not required to support lines longer than - # 2048 bytes (most notably, some sed implementation are - # limited to 4000 bytes, and sed is used by config.status - # to rewrite Makefile.in into Makefile). Moreover nobody - # would really write such long lines by hand since it is - # hardly maintainable. So if a line is longer that 1000 - # bytes (an arbitrary limit), assume it has been - # automatically generated by some tools, and flatten the - # variable definition. Otherwise, keep the variable as it - # as been input. - $var_look = VAR_PRETTY if length ($last_var_value) >= 1000; if (!/\\$/) { Automake::Variable::define ($last_var_name, VAR_MAKEFILE, $last_var_type, $cond, $last_var_value, $comment, - $last_where, $var_look) + $last_where) if $cond != FALSE; $comment = $spacing = ''; - $var_look = VAR_ASIS; } } elsif (/$INCLUDE_PATTERN/o) @@ -6782,8 +6765,7 @@ sub file_contents_internal ($$$%) Automake::Variable::define ($var, $is_am ? VAR_AUTOMAKE : VAR_MAKEFILE, - $type, $cond, $val, $comment, $where, - VAR_ASIS) + $type, $cond, $val, $comment, $where) if $cond != FALSE; $comment = $spacing = ''; diff --git a/lib/Automake/VarDef.pm b/lib/Automake/VarDef.pm index be956f281..93ca49cc5 100644 --- a/lib/Automake/VarDef.pm +++ b/lib/Automake/VarDef.pm @@ -24,8 +24,7 @@ use Automake::ItemDef; require Exporter; use vars '@ISA', '@EXPORT'; @ISA = qw/Automake::ItemDef Exporter/; -@EXPORT = qw (&VAR_AUTOMAKE &VAR_CONFIGURE &VAR_MAKEFILE - &VAR_ASIS &VAR_PRETTY); +@EXPORT = qw (&VAR_AUTOMAKE &VAR_CONFIGURE &VAR_MAKEFILE); =head1 NAME @@ -43,7 +42,7 @@ Automake::VarDef - a class for variable definitions my $loc = new Automake::Location 'Makefile.am:2'; my $def = new Automake::VarDef ('foo', 'bar # more comment', '# any comment', - $loc, '', VAR_MAKEFILE, VAR_ASIS); + $loc, '', VAR_MAKEFILE); # Appending to a definition. $def->append ('value to append', 'comment to append'); @@ -56,7 +55,6 @@ Automake::VarDef - a class for variable definitions my $location = $def->location; my $type = $def->type; my $owner = $def->owner; - my $pretty = $def->pretty; # Changing owner. $def->set_owner (VAR_CONFIGURE, @@ -91,18 +89,6 @@ use constant VAR_AUTOMAKE => 0; # Variable defined by Automake. use constant VAR_CONFIGURE => 1;# Variable defined in configure.ac. use constant VAR_MAKEFILE => 2; # Variable defined in Makefile.am. -=item C, C - -Possible print styles. C variables should be output as-is. -C variables are wrapped on multiple lines if they cannot -fit on one. - -=cut - -# Possible values for pretty. -use constant VAR_ASIS => 0; # Output as-is. -use constant VAR_PRETTY => 1; # Pretty printed on output. - =back =head2 Methods @@ -112,7 +98,7 @@ from L. =over 4 -=item C +=item C Create a new Makefile-variable definition. C<$varname> is the name of the variable being defined and C<$value> its value. @@ -131,15 +117,12 @@ C<$owner> specifies who owns the variables, it can be one of C, C, or C (see these definitions). -Finally, C<$pretty> tells how the variable should be output, and can -be one of C, C. - =cut -sub new ($$$$$$$$$) +sub new ($$$$$$$$) { my ($class, $var, $value, $comment, $cond, $location, $type, - $owner, $pretty) = @_; + $owner) = @_; # A user variable must be set by either '=' or ':=', and later # promoted to '+='. @@ -151,7 +134,6 @@ sub new ($$$$$$$$$) my $self = Automake::ItemDef::new ($class, $location, $owner); $self->{'value_list'} = [ { value => $value, cond => $cond } ]; $self->{'type'} = $type; - $self->{'pretty'} = $pretty; $self->{'seen'} = 0; $self->{'comment_list'} = [ { text => $comment, cond => $cond } ]; return $self; @@ -171,9 +153,6 @@ sub append ($$$) push @{$self->{'comment_list'}}, { text => $comment, cond => $cond }; push @{$self->{'value_list'}}, { value => $value, cond => $cond }; - # Turn ASIS appended variables into PRETTY variables. This is to - # cope with 'make' implementation that cannot read very long lines. - $self->{'pretty'} = VAR_PRETTY if $self->{'pretty'} == VAR_ASIS; } =item C<$def-Evalue> @@ -182,8 +161,6 @@ sub append ($$$) =item C<$def-Etype> -=item C<$def-Epretty> - Accessors to the various constituents of a C. See the documentation of C's arguments for a description of these. @@ -230,12 +207,6 @@ sub type ($) return $self->{'type'}; } -sub pretty ($) -{ - my ($self) = @_; - return $self->{'pretty'}; -} - =item C<$def-Eset_owner ($owner, $location)> Change the owner of a definition. This usually happens because diff --git a/lib/Automake/Variable.pm b/lib/Automake/Variable.pm index 9c51c9e74..fb59f4f01 100644 --- a/lib/Automake/Variable.pm +++ b/lib/Automake/Variable.pm @@ -56,7 +56,7 @@ Automake::Variable - support for variable definitions # Defining a variable. Automake::Variable::define($varname, $owner, $type, $cond, $value, $comment, - $where, $pretty) + $where) # Looking up a variable. my $var = var $varname; @@ -483,6 +483,16 @@ sub check_defined_unconditionally ($;$$) } } +sub _has_line_too_long ($) +{ + my ($text) = @_; + foreach my $line (split "\n", $text) + { + return 1 if length ($line) >= 1000 ; + } + return 0; +} + =item C<$str = $var-Eoutput ([@conds])> Format all the definitions of C<$var> if C<@cond> is not specified, @@ -512,36 +522,27 @@ sub output ($@) my $val = $def->raw_value; my $equals = $def->type eq ':' ? ':=' : '='; my $str = $cond->subst_string; - - - if ($def->pretty == VAR_ASIS) - { - my $output_var = "$name $equals $val"; - $output_var =~ s/^/$str/meg; - $res .= "$output_var\n"; - } - elsif ($def->pretty == VAR_PRETTY) - { - # Suppress escaped new lines. &makefile_wrap will - # add them back, maybe at other places. - $val =~ s/\\$//mg; - my $wrap = makefile_wrap ("$str$name $equals", "$str\t", - split (' ', $val)); - - # If the last line of the definition is made only of - # @substitutions@, append an empty variable to make sure it - # cannot be substituted as a blank line (that would confuse - # HP-UX Make). - $wrap = makefile_wrap ("$str$name $equals", "$str\t", - split (' ', $val), '$(am__empty)') - if $wrap =~ /\n(\s*@\w+@)+\s*$/; - - $res .= $wrap; - } + my $output_var; + # Definition of variables whose value contains unescaped newlines + # (likely as a result of a "+=" appending) cannot be output as-is; + # we need to wrap their definition. We also wrap the definition if + # the length of any line is too big, since POSIX-compliant systems + # are not required to support lines longer than 2048 bytes (most + # notably, some sed implementation are limited to 4000 bytes, and + # sed is used by config.status to rewrite Makefile.in into Makefile). + if (_has_line_too_long ($val) or $val =~ /(:?\\\\)*[^\\]\n./) + { + $val =~ s/\\$//mg; + $output_var = makefile_wrap ("$str$name $equals", "$str\t", + split (' ', $val)); + } else - { - prog_error ("unknonw variable type '$def->pretty'"); - } + { + $val =~ s/^[ \t]*//; + $output_var = "$name $equals $val"; + $output_var =~ s/^/$str/meg; + } + $res .= "$output_var\n"; } return $res; } @@ -695,7 +696,7 @@ sub dump ($) =over 4 -=item C +=item C Define or append to a new variable. @@ -719,17 +720,11 @@ assignment. C<$where>: the C of the assignment. -C<$pretty>: whether C<$value> should be pretty printed (one of C -or C defined by L). -C<$pretty> applies only to real assignments. I.e., it does not apply to -a C<+=> assignment (except when part of it is being done as a conditional -C<=> assignment). - =cut -sub define ($$$$$$$$) +sub define ($$$$$$$) { - my ($var, $owner, $type, $cond, $value, $comment, $where, $pretty) = @_; + my ($var, $owner, $type, $cond, $value, $comment, $where) = @_; prog_error "$cond is not a reference" unless ref $cond; @@ -737,10 +732,6 @@ sub define ($$$$$$$$) prog_error "$where is not a reference" unless ref $where; - prog_error "pretty argument missing" - unless defined $pretty && ($pretty == VAR_ASIS - || $pretty == VAR_PRETTY); - # If there's a comment, make sure it is \n-terminated. if ($comment) { @@ -820,7 +811,7 @@ sub define ($$$$$$$$) my $num = ++$_appendvar; my $hvar = "am__append_$num"; &define ($hvar, VAR_AUTOMAKE, '+', - $cond, $value, $comment, $where, $pretty); + $cond, $value, $comment, $where); # Now HVAR is to be added to VAR. $comment = ''; $value = "\$($hvar)"; @@ -849,8 +840,7 @@ sub define ($$$$$$$$) } else { - &define ($var, $owner, '+', $vcond, $value, $comment, - $where, $pretty); + &define ($var, $owner, '+', $vcond, $value, $comment, $where); } } } @@ -873,7 +863,7 @@ sub define ($$$$$$$$) # locations for '+='. Ideally I suppose we would associate # line numbers with random bits of text. $def = new Automake::VarDef ($var, $value, $comment, $cond, - $where->clone, $type, $owner, $pretty); + $where->clone, $type, $owner); $self->set ($cond, $def); push @_var_order, $var; } @@ -1489,7 +1479,7 @@ sub transform_variable_recursively ($$$$$&;%) foreach (@conds) { define ($varname, VAR_AUTOMAKE, '', $_, "@result", - '', $where, VAR_PRETTY); + '', $where); } } } diff --git a/t/check2.sh b/t/check2.sh index 73eb0b7a5..66ea973b6 100755 --- a/t/check2.sh +++ b/t/check2.sh @@ -62,8 +62,7 @@ $EGREP '^check:.* check-recursive( |$)' Makefile.in $EGREP '^check:.* check-am( |$)' dir/Makefile.in # Make sure subrun.sh is still on its line as above. This means Automake -# hasn't rewritten the TESTS line unnecessarily (we can tell, because all -# Automake variables are reformatted by VAR_PRETTY). +# hasn't rewritten the TESTS line unnecessarily. grep '^ subrun\.sh$' Makefile.in : diff --git a/t/distcom-subdir.sh b/t/distcom-subdir.sh index 20aa8a592..ac1420fdd 100755 --- a/t/distcom-subdir.sh +++ b/t/distcom-subdir.sh @@ -17,6 +17,7 @@ # Test to make sure that if an auxfile (here depcomp) is required # by a subdir Makefile.am, it is distributed by that Makefile.am. +required=cc . ./defs || Exit 1 cat >> configure.ac << 'END' @@ -28,14 +29,18 @@ END cat > Makefile.am << 'END' SUBDIRS = subdir +test-distdir: distdir + test -f $(distdir)/depcomp +.PHONY: test-distdir +check-local: test-distdir END rm -f depcomp mkdir subdir cat > subdir/Makefile.am << 'END' -.PHONY: test-distcommon -test-distcommon: +.PHONY: test-distcom +test-distcom: echo ' ' $(am__dist_common) ' ' | $(FGREP) ' $(top_srcdir)/depcomp ' END @@ -46,15 +51,26 @@ test ! -f depcomp cat >> subdir/Makefile.am << 'END' bin_PROGRAMS = foo +.PHONY: test-distcom +test-distcom: + echo ' ' $(am__dist_common) ' ' | $(FGREP) ' $(top_srcdir)/depcomp ' +check-local: test-distcom END -: > subdir/foo.c +cat > subdir/foo.c <<'END' +int main (void) +{ + return 0; +} +END $AUTOMAKE -a subdir/Makefile test -f depcomp + ./configure -(cd subdir && $MAKE test-distcommon) -$MAKE distdir -test -f $distdir/depcomp +(cd subdir && $MAKE test-distcom) +$MAKE test-distdir + +$MAKE distcheck : diff --git a/t/distcom2.sh b/t/distcom2.sh index 76ded068b..08d26798c 100755 --- a/t/distcom2.sh +++ b/t/distcom2.sh @@ -50,23 +50,8 @@ for opt in '' --no-force; do test -f depcomp for dir in . subdir; do - # FIXME: the logic of this check and other similar ones in other - # FIXME: 'distcom*.test' files should be factored out in a common - # FIXME: subroutine in 'defs'... - sed -n -e " - /^am__dist_common =.*\\\\$/ { - :loop - p - n - t clear - :clear - s/\\\\$/\\\\/ - t loop - s/$/ / - s/[$tab ][$tab ]*/ /g - p - n - }" $dir/Makefile.in > $dir/dc.txt + sed -n 's/^am__dist_common = *\(.*\)$/ \1 /p' \ + <$dir/Makefile.in >$dir/dc.txt done cat dc.txt # For debugging. diff --git a/t/longline.sh b/t/longline.sh index dd5664d31..4900da42c 100755 --- a/t/longline.sh +++ b/t/longline.sh @@ -14,17 +14,24 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -# Long lines of += should be wrapped. +# Long lines of = and += should be wrapped. # Report from Simon Josefsson. . ./defs || Exit 1 -(echo DUMMY = some_long_filename_1; -for i in 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; -do - echo DUMMY += some_long_filename_$i -done) > Makefile.am +i=0 +while test $i -lt 30; do + echo some_very_very_long_variable_content_$i + i=$((i + 1)) +done > t + +{ echo "DUMMY =" && sed 's/^/DUMMY +=/' t; } > Makefile.am +{ echo "ZARDOZ =" && cat t; } | tr '\012\015' ' ' >> Makefile.am $ACLOCAL $AUTOMAKE +grep long_variable Makefile.in # For debugging. test 80 -ge `grep DUMMY Makefile.in | wc -c` +test 80 -ge `grep ZARDOZ Makefile.in | wc -c` + +: diff --git a/t/pluseq3.sh b/t/pluseq3.sh index fc7a496a0..a5fdacacb 100755 --- a/t/pluseq3.sh +++ b/t/pluseq3.sh @@ -43,7 +43,7 @@ $ACLOCAL $AUTOMAKE grep '^@CHECK_TRUE@data_DATA = zarrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr \\$' Makefile.in -grep "^@CHECK_TRUE@${tab}doz$" Makefile.in +grep "^@CHECK_TRUE@[ $tab]*doz$" Makefile.in grep '^@CHECK_FALSE@data_DATA = dog$' Makefile.in diff --git a/t/subdir4.sh b/t/subdir4.sh index 8bf5823f5..7c20a6aa3 100755 --- a/t/subdir4.sh +++ b/t/subdir4.sh @@ -48,7 +48,7 @@ libfoo_a_SOURCES = foo.c END cat > lib/foo.c << 'END' -int foo () {} +int foo (void) { return 0; } END cat > src/Makefile.am << 'END' @@ -60,11 +60,4 @@ END $ACLOCAL $AUTOMAKE --gnu -# Make sure that depcomp is *not* included in the definition -# of am__dist_common in lib/Makefile.in. If you change this test -# so that more files are included in lib's am__dist_common definition, -# then you must handle the case in which depcomp is listed on a -# continued line. -grep '^am__dist_common.*depcomp' lib/Makefile.in && Exit 1 - : diff --git a/t/subst-no-trailing-empty-line.sh b/t/subst-no-trailing-empty-line.sh deleted file mode 100755 index 82b4e5585..000000000 --- a/t/subst-no-trailing-empty-line.sh +++ /dev/null @@ -1,97 +0,0 @@ -#! /bin/sh -# Copyright (C) 2003-2012 Free Software Foundation, Inc. -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2, or (at your option) -# any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . - -# If the last line of a automake-rewritten definition is made only of -# @substitutions@, automake should take care of appending an empty -# variable to make sure that line cannot end up substituted as a blank -# line (that would confuse HP-UX Make). -# These checks have been introduced in commit 'Release-1-9-254-g9d0eaef' -# into the former test 'subst2.test'. - -. ./defs || Exit 1 - -# These are deliberately quite long, so that the xxx_PROGRAMS definition -# in Makefile.am below will be split on multiple lines, with the last -# line containing only @substituted@ stuff that expands to empty (this is -# required to expose the bug we are testing). -v1=ABCDEFGHIJKLMNOPQRSTUVWX -v2=ABCDEFGHIJKLMNOPQRSTUVWXY -v3=ABCDEFGHIJKLMNOPQRSTUVWXYZ - -# Literal backslash for use by grep. -bs='\\' - -cat >> configure.ac <Makefile.am <t-programs -cat t-programs -grep '^ *$' t-programs && Exit 1 - -$MAKE print-programs >stdout || { cat stdout; Exit 1; } -cat stdout -grep '^BEG1: x :END1$' stdout -grep '^BEG2: :END2$' stdout -grep '^BEG3: zardoz x :END3$' stdout - -$MAKE am__empty=X print-programs >stdout || { cat stdout; Exit 1; } -cat stdout -grep '^BEG1: x X :END1$' stdout -grep '^BEG2: X :END2$' stdout -grep '^BEG3: zardoz x X :END3$' stdout - -: -- 2.47.2