On 05/25/2012 01:38 PM, Stefano Lattarini wrote: > Far from the sweeping simplifications I was hoping for when I started > writing this series, but they are good to go IMHO. A better cleanup > (which I still hope I'll be able to do eventually) will probably require > a rewrite of the Automake::Variable and Automake::VarDef modules :-/ > > OK for ng/master? I will push in a couple of days if there is no > objection. > > Regards, > Stefano > > -*-*- > I've now pushed this series, after dropping ...
> Stefano Lattarini (15): > [ng] cleanup: remove unused private variables in Automake::Variable > [ng] vars: get rid of VAR_SORTED type > [ng] vars: get rid of VAR_SILENT type > [ng] tests: spy behaviour of '+=' with GNU make > [ng] tests: Automake should let us append to undefined variables (someday) > [ng] refactor: support comments only for VarDef, not for ItemDef too > [ng] VarDef: store comments and values as a perl array > [ng] vars: simplify logic for appending conditionally > [ng] vars: keep track of conditionals in appended values and comments > [ng] vars: get rid of VAR_ASIS / VAR_PRETTY distinction > [ng] refactor: change signature of 'define_variable()' > [ng] cleanup: prefer 'define_variable' over 'define_pretty_variable' > [ng] rename: define_pretty_variable -> define_cond_variable() > [ng] cosmetics: avoid redundant use of '&' in subroutine calls > > [ng] vars: remove some safety checks in Automake::Variable::define > ... this last patch, because its only purpose would be to simplify future refactorings, which are still not ready nor truly planned. I've also pushed the attached patch as a follow-up, to fix a regression revealed by the more comprehensive tests in 'ng/master'. Regards, Stefano
>From 763cf97ba788c950389aedc9818322761ef7c40a Mon Sep 17 00:00:00 2001 Message-Id: <763cf97ba788c950389aedc9818322761ef7c40a.1338621325.git.stefano.lattar...@gmail.com> From: Stefano Lattarini <[email protected]> Date: Thu, 31 May 2012 17:52:51 +0200 Subject: [PATCH] [ng] vars: recognize escaped '#' correctly in a variable definition Regression revealed by a failure in test 't/backslash-tricks.sh', and caused by the recent merge of the 'ng/var-simplify' branch. It is worth noting that this change has a collateral effect: comments placed after the variable definition *must* now be separated with one or more spaces from said definition: # This won't work, and will produce a subtly broken Makefile foo = val# comment foo += val2 # Please do this instead foo = val # comment foo += val2 We believe that supporting the use of escaped '#' characters in variable definitions is more important than supporting the fringe case above. * lib/Automake/VarDef.pm (raw_value): Fix processing of stored value to account for escaped '#' characters. (value): Add a FIXME comment about a statement that is not strictly true anymore now that we assume GNU make. * t/backslash-tricks.sh: Extend a bit. * t/comment8.sh: Relax a bit, by placing spaces between the variable's definitions and the comments on the same line. Extend in another respect, to watch against a regression introduced by the first draft of this commit. Signed-off-by: Stefano Lattarini <[email protected]> --- lib/Automake/VarDef.pm | 9 ++++++++- t/backslash-tricks.sh | 6 ++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lib/Automake/VarDef.pm b/lib/Automake/VarDef.pm index 93ca49c..bc3ca82 100644 --- a/lib/Automake/VarDef.pm +++ b/lib/Automake/VarDef.pm @@ -173,6 +173,8 @@ sub value ($) # Strip anything past '#'. '#' characters cannot be escaped # in Makefiles, so we don't have to be smart. + # FIXME: Actually, '#' *can* be escaped in GNU make ... + # FIXME: Should we adapt our code? $val =~ s/#.*$//s; # Strip backslashes. $val =~ s/\\$/ /mg; @@ -197,7 +199,12 @@ sub raw_value ($) # VAR = foo # com bar # Furthermore keeping '#' would not be portable if the variable is # output on multiple lines. - map { s/ ?#.*// } @values; + # But we have to preserve escaped '#', so that a definition line: + # hash = \# + # remains possible. To make our life easier, we just assume that + # any tailed comment must be separated with whitespace from the + # actual variable value. + map { s/[ \t]+#.*// } @values; return join (' ', @values); } diff --git a/t/backslash-tricks.sh b/t/backslash-tricks.sh index dea9e39..e236183 100755 --- a/t/backslash-tricks.sh +++ b/t/backslash-tricks.sh @@ -48,6 +48,11 @@ var4 = $(var3) var5 = ok \ # ko +var6 = \# \ +\#\\\\\# seen # not seen + +var6 += \# \# # again not seen + .PHONY: test test: test -z '$(var1)' @@ -57,6 +62,7 @@ test: # Use '[', not 'test', here, so that spurious comments # are ensured to cause syntax errors. [ $(var5) = ok ] + test '$(var6)' = '# #\\# seen # #' # Yes, this file ends with a backslash-newline. So what? \ -- 1.7.9.5
