Re: PATCH: patsubst support
> "Derek" == Derek R Price <[EMAIL PROTECTED]> writes: >> if FOO >> var = a b >> endif >> derived = $(var:%=%.c) >> if BAR >> var = c d >> endif Derek> Isn't the order irrelevant here since derived won't be Derek> evaluated until it's used? No, because we're talking about having automake itself expand the patsubst. It is an automake implementation issue, not related to what make does. Tom
Re: PATCH: patsubst support
Tom Tromey wrote: > if FOO > var = a b > endif > derived = $(var:%=%.c) > if BAR > var = c d > endif Isn't the order irrelevant here since derived won't be evaluated until it's used? Um, the gmake manual calls this "expanded when read, except for the shell commands in rules, the right-hand sides of variable definitions using `=', and the bodies of variable definitions using the `define' directive". Derek -- Derek Price CVS Solutions Architect ( http://CVSHome.org ) mailto:[EMAIL PROTECTED] OpenAvenue ( http://OpenAvenue.com ) -- Before you criticize someone, walk a mile in his shoes. That way, if he gets angry, he'll be a mile away - and barefoot.
Re: PATCH: patsubst support
> "Tom" == Tom Tromey <[EMAIL PROTECTED]> writes: Following up my own post... Tom> if FOO Tom> var = a b c Tom> else Tom> var = d e f Tom> endif Tom> derived = $(var:%=%.c) Tom> Will this work correctly? In this situation we have to give Tom> `derived' the same conditions as `var'. Last night I realized we could solve this by introducing a new variable in Makefile.in to represent each patsubst expansion. Such a variable would be given the same conditions as what it depended on and would be expanded by automake. The `derived' variable would be rewritten to refer to this new variable. I think that solves all our problems and is fairly easy to implement. Well, it is easy if you do patsubst expansion in a separate pass, to handle cases like this: if FOO var = a b endif derived = $(var:%=%.c) if BAR var = c d endif Tom
Re: PATCH: patsubst support
> "Alex" == Alex Hornby <[EMAIL PROTECTED]> writes: Alex> + (read_main_am_file): call expand_contents to output Alex> + variables. I'm concerned about this (referenced) part of the patch. Suppose we have a Makefile.am like this: if FOO var = a b c else var = d e f endif derived = $(var:%=%.c) Will this work correctly? In this situation we have to give `derived' the same conditions as `var'. This quickly gets very hairy if you consider that `var' might depend on things with conditions, or that `derived' might be derived from several conditional variables, or that `var' might be undefined in some condition (whereas we must always unconditionally define `derived' in the above example). I'm also concerned that using &expand_contents from &read_main_am_file will prematurely expand variables into Makefile.in. For instance this: a = b foo = $(a) b ... shouldn't end up in the Makefile.in like so: a = b foo = b b Probably we'd need to do a separate expansion that handles only patsubst. (But is that even possible? The mind boggles at some of the oddities that might encounter.) This is all fairly hideous, probably more so than you were anticipating. Sorry. I do like the idea of letting automake handle patsubst somehow. Finally, the doc fix and NEWS entry seem to have been dropped from the patch. Could you restore them? Tom
Re: PATCH: patsubst support
> "Alex" == Alex Hornby <[EMAIL PROTECTED]> writes: Alex> Here is a new version of the patsubst patch against cvs HEAD. Sorry I haven't been following this thread until now. Only in the last few weeks have I really started reading automake email again. If your message is buried too deep in the mailbox it might take a long time for me to get there. Alex> + # standard substitution reference style FYI, comments should start with a capital letter and end with a period. This is from the GNU coding standards, which we sort of follow (we'd follow them more closely but I failed to get Perl mode to indent GNU-style). Tom
Re: PATCH: patsubst support
> "Pavel" == Pavel Roskin <[EMAIL PROTECTED]> writes: >> -($from = $2) =~ s/(\W)/\\$1/g; >> +($from = $2) =~ s/(\W)/$1/g; Pavel> I don't understrand this. This change will affect the Pavel> traditional rules as well. It should probably be a separate Pavel> patch if it fixes a separate issue. You may even need a test Pavel> case. I agree. I don't follow this change either. Unfortunately neither do I follow the original code :-(. So if you know what it does feel free to add a comment explaining it ... Pavel> &am_error ("\`$from' cannot be expanded"); Pavel> Note that am_error() doesn't stop the execution, but causes Pavel> automake to exit with code 1. In this case it is probably better to use &am_line_error. The first argument to &am_line_error can be either a variable name or a line number. If it is a variable name the automake will pick the correct line number. Pavel> How about this: Pavel> join(" ", @curval) To pick a nit, we usually use single quotes and a space before open parens: join (' ', @curval) Pavel> The Automake code should be of the highest quality. Hah, hah! Good one! :-) Tom
Re: PATCH: patsubst support
> "Akim" == Akim Demaille <[EMAIL PROTECTED]> writes: Akim> Yep, by default Automake must not let the users do nonportable Akim> things. I'm sorry about that, but I believe it's a strong Akim> requirement. I'm finally following up to this -- it was buried in my overly large automake mailbox. Automake isn't clearly on one side or the other of this issue. On the one hand, any code we generate must be portable. On the other, we don't go out of our way to disallow nonportable constructs. This isn't a very helpful observation :-(. Tom
Re: PATCH: patsubst support
> "Kevin" == Kevin Ryde <[EMAIL PROTECTED]> writes: Kevin> It'd be nice to be able to embed little fragments of perl to do Kevin> things like that, for the "static" case that is. But perhaps Kevin> that idea has come up before. I've long resisted letting the user extend Makefile.am with Perl code. I want to preserve the barrier between the implemented "language" (automake input) and automake's implementation language. This is important because we want the freedom to randomly change the automake implementation. For instance we're supposed to rewrite automake in guile eventually. Tom
Re: PATCH: patsubst support
Pavel Roskin writes: > > > - ($from = $2) =~ s/(\W)/\\$1/g; > > + ($from = $2) =~ s/(\W)/$1/g; > > I don't understrand this. This change will affect the traditional rules as > well. It should probably be a separate patch if it fixes a separate issue. > You may even need a test case. > Hi Pavel, Its a bug fix, the reason escapes me at the moment... > Everything else is fine. I'm sorry that I'm keeping bouncing your work, Perhaps I can return the favor one day I will attempt the "one true patch" later in the week. Cheers, Alex.
Re: PATCH: patsubst support
Hello, Alex! > Here is a new version of the patsubst patch against cvs HEAD. Thanks! Were are getting closer. > + * automake.in (expand_contents): add new function to perform > + the patsubst expansion > + (value_to_list): add support for patsubst style variable > + substitution. > + (read_main_am_file): call expand_contents to output > + variables. > + > + * tests/patsubst.test: add test for patsubst expansion > + > + * tests/patsubst2.test: add test for conditional patsubst > + expansion > + > + * tests/Makefile.am: reference patsubst.test and patsubst2.test Sorry, I'll be pedantic. Capitalize the first word (s/add/Add/). Don't leave blank lines - they are used to distinguish separate commits. Don't omit a full stop. > - ($from = $2) =~ s/(\W)/\\$1/g; > + ($from = $2) =~ s/(\W)/$1/g; I don't understrand this. This change will affect the traditional rules as well. It should probably be a separate patch if it fixes a separate issue. You may even need a test case. > - if ($from) > + if ($from =~ '^([^%]*)%([^%]*)') > { > - grep (s/$from$/$to/, @temp_list); > + # patsubst style substitution > + my ($prefrom, $suffrom, $preto, $sufto); > + $prefrom = $1; > + $suffrom = $2; When I first saw "preto" and "suffrom" I thought it's probably in Latin :-) Bytes are cheap, especially when they don't end up in every Makefile.in for every package. Use suffix_from or whatever is appropriate. ... > + } > + elsif ($from) > + { > + # standard substitution reference style > + grep (s/$from$/$to/, @temp_list); > } what I don't see here is a case for $from containing '%' but not matching our rule. It should be something like: elsif ($from) { # standard substitution reference style if ($from =~ '%') { &am_error ("\`$from' cannot be expanded"); } grep (s/$from$/$to/, @temp_list); } Note that am_error() doesn't stop the execution, but causes automake to exit with code 1. > +sub expand_contents > +{ > +my ($var, $value, $cond) = @_; > +my ($ret) = $value; > + > +if ( $value =~ m/([^%]*)%([^%]*)%/ ) > +{ > + my @curval = &variable_value_as_list ($var, $cond); > + my ( $val ); > + $ret = ''; > + foreach $val ( @curval ) > + { > + $ret .= $val . " "; > + } How about this: join(" ", @curval) Everything else is fine. I'm sorry that I'm keeping bouncing your work, but the Automake code should be of the highest quality. Regards, Pavel Roskin
Re: PATCH: patsubst support
Here is a new version of the patsubst patch against cvs HEAD. It is now smaller due to the removal of a superflous option, and has my instead of local etc. Also the conditional test is improved. After applying the patch remember to make the .test files executable. That has caught me out on more than one occasion :) Cheers, Alex. diff -r -P -u --exclude-from=/tmp/diff_exclude.1207 automake-cvs/ChangeLog.patsubst automake-patsubst/ChangeLog.patsubst --- automake-cvs/ChangeLog.patsubst Thu Jan 1 01:00:00 1970 +++ automake-patsubst/ChangeLog.patsubstSun Feb 18 17:07:23 2001 @@ -0,0 +1,15 @@ +2001-02-18 Alex Hornby <[EMAIL PROTECTED]> + + * automake.in (expand_contents): add new function to perform + the patsubst expansion + (value_to_list): add support for patsubst style variable + substitution. + (read_main_am_file): call expand_contents to output + variables. + + * tests/patsubst.test: add test for patsubst expansion + + * tests/patsubst2.test: add test for conditional patsubst + expansion + + * tests/Makefile.am: reference patsubst.test and patsubst2.test diff -r -P -u --exclude-from=/tmp/diff_exclude.1207 automake-cvs/automake.in automake-patsubst/automake.in --- automake-cvs/automake.inSun Feb 18 16:22:53 2001 +++ automake-patsubst/automake.in Sun Feb 18 17:07:23 2001 @@ -5826,16 +5826,34 @@ { $varname = $1; $to = $3; - ($from = $2) =~ s/(\W)/\\$1/g; + ($from = $2) =~ s/(\W)/$1/g; } # Find the value. @temp_list = &variable_value_as_list_worker ($1, $cond, $var); # Now rewrite the value if appropriate. - if ($from) + if ($from =~ '^([^%]*)%([^%]*)') { - grep (s/$from$/$to/, @temp_list); + # patsubst style substitution + my ($prefrom, $suffrom, $preto, $sufto); + $prefrom = $1; + $suffrom = $2; + + if ( $to =~ '^([^%]*)%([^%]*)') + { + $preto = $1; + $sufto = $2; + } + grep { + s/^$prefrom/$preto/; + s/$suffrom$/$sufto/; + } @temp_list; + } + elsif ($from) + { + # standard substitution reference style + grep (s/$from$/$to/, @temp_list); } push (@result, @temp_list); @@ -6453,6 +6471,24 @@ } } +sub expand_contents +{ +my ($var, $value, $cond) = @_; +my ($ret) = $value; + +if ( $value =~ m/([^%]*)%([^%]*)%/ ) +{ + my @curval = &variable_value_as_list ($var, $cond); + my ( $val ); + $ret = ''; + foreach $val ( @curval ) + { + $ret .= $val . " "; + } +} +return $ret; +} + # Read main am file. sub read_main_am_file { @@ -6501,6 +6537,7 @@ { local ($vcond) = shift (@cond_vals); local ($val) = &unquote_cond_val (shift (@cond_vals)); + $val = expand_contents ($curs, $val, $vcond); $output_vars .= ($vcond . $curs . ' ' . $def_type{$curs} . "= "); local ($line); @@ -6514,8 +6551,9 @@ } else { + my ($val) = expand_contents($curs, $contents{$curs}, ''); $output_vars .= ($curs . ' ' . $def_type{$curs} . '= ' -. $contents{$curs} . "\n"); +. $val . "\n"); } } diff -r -P -u --exclude-from=/tmp/diff_exclude.1207 automake-cvs/tests/Makefile.am automake-patsubst/tests/Makefile.am --- automake-cvs/tests/Makefile.am Sun Feb 18 16:23:02 2001 +++ automake-patsubst/tests/Makefile.am Sun Feb 18 17:07:23 2001 @@ -188,6 +188,8 @@ output5.test \ package.test \ parse.test \ +patsubst.test \ +patsubst2.test \ pluseq.test \ pluseq2.test \ pluseq3.test \ diff -r -P -u --exclude-from=/tmp/diff_exclude.1207 automake-cvs/tests/patsubst.test automake-patsubst/tests/patsubst.test --- automake-cvs/tests/patsubst.testThu Jan 1 01:00:00 1970 +++ automake-patsubst/tests/patsubst.test Sun Feb 18 17:07:23 2001 @@ -0,0 +1,25 @@ +#! /bin/sh + +# Test `patsubst expansion' functionality. +# There should be no patsubst constructs in the Makefile.in + +. $srcdir/defs || exit 1 + +cat >> configure.in << 'END' +AC_PROG_CC +END + +cat > Makefile.am << 'END' +bin_PROGRAMS = zardoz +BASENAMES = zar doz +zardoz_SOURCES = ${BASENAMES:%=%.c} +END + +: > zar.c +: > doz.c + +$AUTOMAKE || exit 1 +fgrep 'zar.o doz.o' Makefile.in && +if fgrep '${BASENAMES:%=%.c}' Makefile.in; then + exit 1 +fi diff -r -P -u --exclude-from=/tmp/diff_exclude.1207 automake-cvs/tests/patsubst2.test automake-patsubst/tests/patsubst2.test --- automake-cvs/tests/patsubst2.test Thu Jan 1 01:00:00 1970 +++ a
Re: PATCH: patsubst support
Hi Pavel, Thanks for your comments, I'll put together a new patch taking them into account. Removing the new command line option will simplify things and get rid of the need for the second call to handle_options. Regards, Alex.
Re: PATCH: patsubst support
Hello, Alex! Sorry for another delay. Your patch is very important, but unfortunately I'm have been very busy recently. > Here is an updated patsubst patch against CVS automake. Any patsubst > style variables are now staticly expanded by automake, thus avoiding > make portability problems. We now have a single ChangeLog in the top-level directory. > I have included tests for both the normal and conditional cases of > variable expansion. The test for the conditional case doesn't really test how the pattern is expanded. And for me it's expanded incorrectly: @TEST_TRUE@VAR = @TEST_TRUE@zar doz Note missing ".c" extentions. The first test just fails. > Please consider this for checkin. Only when it's done correctly. Few notes about the implementation. > + (handle_options): add new option "no-expand-patsubst" I don't like adding unsafe options. I cannot imagine any situation when anybody would need it for a legitimate reason. Using patterns with the older versions of Automake is incorrect, so I'm not concerned if we break it. It's better to have the same makefiles for users and developers unless there are really serious reasons for them to be different. But if you insist, the new options should be documented. > + (expand_contents): add new function to perform the patsubst expansion > + (value_to_list): add support for patsubst style variable > + substitution. > + (read_main_am_file): call expand_contents to output > + variables. Add extra call to handle_options, otherwise options > + are set after they have effect. Very dirty. Now we are calling handle_options twice. > +sub expand_contents > +{ > +local ($var, $value, $cond) = @_; We are using "my", not "local" for the new code. Eventually Automake will get rid of "local" variables. > +local ($ret) = $value; > + > +if ( $value =~ m/([^%]*)%([^%]*)%/ ) > +{ > + if ( $expand_patsubst ) > + { > + local @curval = &variable_value_as_list ($var, $cond); Maybe I'm missing something, but I don't see any error protection here. You are working with user input. What if the expression is invalid? > +$AUTOMAKE || exit 1 > + > +grep '^@TEST_TRUE@' Makefile.in || exit 1 > +exit 0 As I said before, this is not a test for pattern expansion. Regards, Pavel Roskin
Re: PATCH: patsubst support
Hello, Here is an updated patsubst patch against CVS automake. Any patsubst style variables are now staticly expanded by automake, thus avoiding make portability problems. I have included tests for both the normal and conditional cases of variable expansion. Please consider this for checkin. Regards, Alex Hornby diff -r -P -u --exclude-from=/tmp/diff_exclude.9118 automake-cvs/ChangeLog.patsubst automake-patsubst/ChangeLog.patsubst --- automake-cvs/ChangeLog.patsubst Thu Jan 1 01:00:00 1970 +++ automake-patsubst/ChangeLog.patsubstSun Feb 11 19:55:33 2001 @@ -0,0 +1,11 @@ +2000-11-17 Alex Hornby <[EMAIL PROTECTED]> + + * automake.in ($expand_patsubst): add new global to determine + patsubst expansion + (handle_options): add new option "no-expand-patsubst" + (expand_contents): add new function to perform the patsubst expansion + (value_to_list): add support for patsubst style variable + substitution. + (read_main_am_file): call expand_contents to output + variables. Add extra call to handle_options, otherwise options + are set after they have effect. diff -r -P -u --exclude-from=/tmp/diff_exclude.9118 automake-cvs/automake.in automake-patsubst/automake.in --- automake-cvs/automake.inSun Feb 11 17:12:31 2001 +++ automake-patsubst/automake.in Sun Feb 11 19:44:31 2001 @@ -101,6 +101,9 @@ # included in generated Makefile.in. $cmdline_use_dependencies = 1; +# This is TRUE if variables are to have patsubst expressions expanded +$expand_patsubst = 1; + # TRUE if in verbose mode. $verbose = 0; @@ -775,6 +778,10 @@ { $use_dependencies = 0; } + elsif ($_ eq 'no-expand-patsubst' ) + { + $expand_patsubst = 0; + } elsif (/([0-9]+)\.([0-9]+)([a-z])?/) { # Got a version number. @@ -6432,6 +6439,27 @@ } } +sub expand_contents +{ +local ($var, $value, $cond) = @_; +local ($ret) = $value; + +if ( $value =~ m/([^%]*)%([^%]*)%/ ) +{ + if ( $expand_patsubst ) + { + local @curval = &variable_value_as_list ($var, $cond); + local ( $val ); + $ret = ''; + foreach $val ( @curval ) + { + $ret .= $val . " "; + } + } +} +return $ret; +} + # Read main am file. sub read_main_am_file { @@ -6464,6 +6492,12 @@ $output_vars = ''; &read_am_file ($amfile); +if (&handle_options) +{ + # Fatal error. Just return, so we can continue with next file. + return; +} + # Now dump the variables that were defined. We do it in the same # order in which they were defined (skipping duplicates). local (%done); @@ -6480,6 +6514,7 @@ { local ($vcond) = shift (@cond_vals); local ($val) = &unquote_cond_val (shift (@cond_vals)); + $val = expand_contents ($curs, $val, $vcond); $output_vars .= ($vcond . $curs . ' ' . $def_type{$curs} . "= "); local ($line); @@ -6493,8 +6528,9 @@ } else { + local ($val) = expand_contents($curs, $contents{$curs}, ''); $output_vars .= ($curs . ' ' . $def_type{$curs} . '= ' -. $contents{$curs} . "\n"); +. $val . "\n"); } } diff -r -P -u --exclude-from=/tmp/diff_exclude.9118 automake-cvs/tests/ChangeLog.patsubst automake-patsubst/tests/ChangeLog.patsubst --- automake-cvs/tests/ChangeLog.patsubst Thu Jan 1 01:00:00 1970 +++ automake-patsubst/tests/ChangeLog.patsubst Sun Feb 11 19:44:31 2001 @@ -0,0 +1,7 @@ +2000-10-25 Alex Hornby <[EMAIL PROTECTED]> + + * patsubst.test: add test for new patsubst expansion + + * patsubst2.test: add test for conditional patsubst expansion + + * Makefile.am: reference patsubst.test and patsubst2.test diff -r -P -u --exclude-from=/tmp/diff_exclude.9118 automake-cvs/tests/Makefile.am automake-patsubst/tests/Makefile.am --- automake-cvs/tests/Makefile.am Sun Feb 11 17:12:52 2001 +++ automake-patsubst/tests/Makefile.am Sun Feb 11 19:44:31 2001 @@ -188,6 +188,8 @@ output5.test \ package.test \ parse.test \ +patsubst.test \ +patsubst2.test \ pluseq.test \ pluseq2.test \ pluseq3.test \ diff -r -P -u --exclude-from=/tmp/diff_exclude.9118 automake-cvs/tests/patsubst.test automake-patsubst/tests/patsubst.test --- automake-cvs/tests/patsubst.testThu Jan 1 01:00:00 1970 +++ automake-patsubst/tests/patsubst.test Sun Feb 11 19:44:31 2001 @@ -0,0 +1,25 @@ +#! /bin/sh + +# Test `patsubst expansion' functionality. +# There should be no patsubst constructs in the Makefile.in + +. $srcdir/defs || exit 1 + +cat >> configure.in << 'END' +AC_PROG_CC +END + +cat > Makefile.am << 'END' +bin_PROGRAMS = zardoz +BASENAMES = zar doz +zardoz_SOURCES =
Re: PATCH: patsubst support
> "Pavel" == Pavel Roskin <[EMAIL PROTECTED]> writes: Pavel> Hello! Trying to catch up with the mailing lists :-) Pavel> I'm surprised that this patch has not been applied since Pavel> October. I believe it's very valuable. I even considered doing Pavel> it myself. We ended stuck with a portability problem: it would have let go into Makefile.in nonportable constructs. With a safety belt, it'd been too dangerous for users. We're waiting for a good soul to write the safety net.
Re: PATCH: patsubst support
Hello! Trying to catch up with the mailing lists :-) I'm surprised that this patch has not been applied since October. I believe it's very valuable. I even considered doing it myself. > b) default static expansion to off, avoids surprising anyone depending >on dynamic expansion by make, retains the same non-portable >default behavour as current CVS. > > # Makefile.in fragment > FOO = foo bar > BAR = ${FOO:%=%.c} This would be wrong. This is only acceptable in the developer's environment, i.e. not in the makefiles created by "make dist". But I see no serious reason to generate different makefiles in this case. Regards, Pavel Roskin
Re: PATCH: patsubst support
Alex Hornby <[EMAIL PROTECTED]> writes: > > # Makefile.am fragment > FOO = foo bar > BAR = ${FOO:%=%.c} > ... > > What do people think? It'd be nice to be able to embed little fragments of perl to do things like that, for the "static" case that is. But perhaps that idea has come up before. Or naturally the whole Makefile.am could be a generated file if desired.
Re: PATCH: patsubst support
On Oct 28, 2000, Alex Hornby <[EMAIL PROTECTED]> wrote: > What is the policy regarding changes to non-portable behavour? automake is supposed to generate portable Makefiles, so I think the default should be static expansion. -- Alexandre Oliva Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/ Red Hat GCC Developer aoliva@{cygnus.com, redhat.com} CS PhD student at IC-Unicampoliva@{lsd.ic.unicamp.br, gnu.org} Free Software Evangelist*Please* write to mailing lists, not to me
Re: PATCH: patsubst support
Alexandre Oliva writes: > On Oct 27, 2000, Akim Demaille <[EMAIL PROTECTED]> wrote: > > > Yep, by default Automake must not let the users do nonportable > > things. > > I tend to agree. But I wouldn't say `must not', I'd say `should not'. What is the policy regarding changes to non-portable behavour? Current CVS autoconf will pass through patsubst style variables to to Makefile for dynamic expansion via make. I have now got a patch that expands these statically which I need to decide to the default for. I have illustrated the possible default behavours with a Makefile.am expansion; # Makefile.am fragment FOO = foo bar BAR = ${FOO:%=%.c} a) default static expansion to on, this is good for portability # Makefile.in fragment FOO = foo bar BAR = foo.c bar.c b) default static expansion to off, avoids surprising anyone depending on dynamic expansion by make, retains the same non-portable default behavour as current CVS. # Makefile.in fragment FOO = foo bar BAR = ${FOO:%=%.c} What do people think? Regards, Alex.
Re: PATCH: patsubst support
On Oct 27, 2000, Akim Demaille <[EMAIL PROTECTED]> wrote: > Yep, by default Automake must not let the users do nonportable > things. I tend to agree. But I wouldn't say `must not', I'd say `should not'. -- Alexandre Oliva Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/ Red Hat GCC Developer aoliva@{cygnus.com, redhat.com} CS PhD student at IC-Unicampoliva@{lsd.ic.unicamp.br, gnu.org} Free Software Evangelist*Please* write to mailing lists, not to me
Re: PATCH: patsubst support
| Akim Demaille writes: | > | > Sorry, I'm confused, and the documentation snippet didn't really | > enlighten me :( | > | | Hi Akim, | | The reasoning was fairly tortured :) | | To document the patsubst internal change I had to invent a contrived | example so that the user could see the expansion. That example has | problems... | | There are two possible solutions: | | * document patsubst in the context of the user dep files patch, where | the behavour is truly static. | * patsubst expand variables before they hit Makefile.in. (enabled by | an option to protect people (e.g. myself) who already abuse Automake | by passing through patsubst expressions). | | From your reaction, I suspect the second might be the better | solution. Unfortunately it is more work... Yep, by default Automake must not let the users do nonportable things. I'm sorry about that, but I believe it's a strong requirement. I'd like to hear from Alexandre and Tom though.
Re: PATCH: patsubst support
Akim Demaille writes: > > Sorry, I'm confused, and the documentation snippet didn't really > enlighten me :( > Hi Akim, The reasoning was fairly tortured :) To document the patsubst internal change I had to invent a contrived example so that the user could see the expansion. That example has problems... There are two possible solutions: * document patsubst in the context of the user dep files patch, where the behavour is truly static. * patsubst expand variables before they hit Makefile.in. (enabled by an option to protect people (e.g. myself) who already abuse Automake by passing through patsubst expressions). >From your reaction, I suspect the second might be the better solution. Unfortunately it is more work... Cheers, Alex.
Re: PATCH: patsubst support
> "Alex" == Alex Hornby <[EMAIL PROTECTED]> writes: Alex> Akim, Alex> Okay, here is patsubst patch v2. Thanks! Alex> The _PROGRAMS based example in the documentation needs a Alex> patsubst supporting make (e.g. GNU and Solaris work). This is Alex> because the program target writes prog_SOURCES to the Alex> Makefile.in without any variable expansions. Alex> The real reason I want patsubst (user specified dependency Alex> files) doesn't impose any new make requirements when used, as Alex> the patsubst expressions are expanded before being written to Alex> Makefile.in. Alex> Until then, the PROGRAM targets are the only obvious route to Alex> Automake's variable expansion, so thats what I used to give an Alex> example. Sorry, I'm confused, and the documentation snippet didn't really enlighten me :( I really thought that your patch was purely static, i.e., only from .am to .in, requiring no special feature from Make. You seem to say in the first paragraph above that it's only the case of *this* example, but I'm now failing to see in which cases you would not depend upon the underlying Make. We are hitting a limit (assumptions over Make) which is out of my competence. Only Tom and Alexandre may make such decision...
Re: PATCH: patsubst support
Akim, Okay, here is patsubst patch v2. New since last time: * ChangeLog entry formatting * NEWS entry * Documentation (first texinfo usage, please beware!) The _PROGRAMS based example in the documentation needs a patsubst supporting make (e.g. GNU and Solaris work). This is because the program target writes prog_SOURCES to the Makefile.in without any variable expansions. The real reason I want patsubst (user specified dependency files) doesn't impose any new make requirements when used, as the patsubst expressions are expanded before being written to Makefile.in. Until then, the PROGRAM targets are the only obvious route to Automake's variable expansion, so thats what I used to give an example. Alex. diff -r -P -u automake/ChangeLog.entry automake-patsubst/ChangeLog.entry --- automake/ChangeLog.entryThu Jan 1 01:00:00 1970 +++ automake-patsubst/ChangeLog.entry Wed Oct 25 15:41:28 2000 @@ -0,0 +1,5 @@ +2000-10-25 Alex Hornby <[EMAIL PROTECTED]> + + * automake.in (value_to_list): Add support for patsubst + style variable substitution. + diff -r -P -u automake/NEWS.entry automake-patsubst/NEWS.entry --- automake/NEWS.entry Thu Jan 1 01:00:00 1970 +++ automake-patsubst/NEWS.entryWed Oct 25 15:44:28 2000 @@ -0,0 +1 @@ +* Patsubst style variable expansion is now available. diff -r -P -u automake/automake.in automake-patsubst/automake.in --- automake/automake.inWed Oct 25 11:25:01 2000 +++ automake-patsubst/automake.in Wed Oct 25 14:20:07 2000 @@ -5902,16 +5902,34 @@ { $varname = $1; $to = $3; - ($from = $2) =~ s/(\W)/\\$1/g; + ($from = $2) =~ s/(\W)/$1/g; } # Find the value. @temp_list = &variable_value_as_list_worker ($1, $cond, $var); # Now rewrite the value if appropriate. - if ($from) + if ($from =~ '^([^%]*)%([^%]*)') { - grep (s/$from$/$to/, @temp_list); + # patsubst style substitution + local ($prefrom, $suffrom, $preto, $sufto); + $prefrom = $1; + $suffrom = $2; + + if ( $to =~ '^([^%]*)%([^%]*)') + { + $preto = $1; + $sufto = $2; + } + grep { + s/^$prefrom/$preto/; + s/$suffrom$/$sufto/; + } @temp_list; + } + elsif ($from) + { + # standard substitution reference style + grep (s/$from$/$to/, @temp_list); } push (@result, @temp_list); diff -r -P -u automake/automake.texi automake-patsubst/automake.texi --- automake/automake.texi Tue Oct 17 09:49:13 2000 +++ automake-patsubst/automake.texi Wed Oct 25 16:42:13 2000 @@ -1605,6 +1605,23 @@ cause an invalid value for @samp{@var{prog}_DEPENDENCIES} to be generated. +@cindex Variables, patsubst expansion + +Sometimes it can be useful to derive @samp{@var{prog}_SOURCES} from +another variable. This can be done using patsubst style expansion to +rewrite a variable: + +@example +MODULES = Account Bank Currency @dots{} +bin_PROGRAMS = account_server +account_server_SOURCES = $@{MODULES:%=%.c@} +@end example + +Currently this feature requires the use of a @code{make} with patsubst +support (such as GNU @code{make}). It may become possible in the future +to expand all patsubst variable definitions before they are written to +@file{ Makefile.in}. In the meantime if you use this feature you may get +a error from non-GNU make. @node A Library, LIBOBJS, A Program, Programs @section Building a library diff -r -P -u automake/tests/ChangeLog.entry automake-patsubst/tests/ChangeLog.entry --- automake/tests/ChangeLog.entry Thu Jan 1 01:00:00 1970 +++ automake-patsubst/tests/ChangeLog.entry Wed Oct 25 15:41:59 2000 @@ -0,0 +1,5 @@ +2000-10-25 Alex Hornby <[EMAIL PROTECTED]> + + * patsubst.test: Add test for patsubst variable expansion. + + * Makefile.am: Reference patsubst.test. Only in automake/tests: Makefile diff -r -P -u automake/tests/Makefile.am automake-patsubst/tests/Makefile.am --- automake/tests/Makefile.am Wed Oct 25 11:25:02 2000 +++ automake-patsubst/tests/Makefile.am Wed Oct 25 14:13:05 2000 @@ -181,6 +181,7 @@ output5.test \ package.test \ parse.test \ +patsubst.test \ pluseq.test \ pluseq2.test \ pluseq3.test \ diff -r -P -u automake/tests/patsubst.test automake-patsubst/tests/patsubst.test --- automake/tests/patsubst.testThu Jan 1 01:00:00 1970 +++ automake-patsubst/tests/patsubst.test Wed Oct 25 14:13:01 2000 @@ -0,0 +1,21 @@ +#! /bin/sh + +# Test `patsubst expansion' functionality. + +. $srcdir/defs || exit 1 + +cat > Makefile.am << 'END' +bin_PROGRAMS = zardoz +BASENAMES = zar doz +zardoz_SOURCES = ${BASENAMES:%=%.c} +END + +cat >> configure.in << 'END' +AC_PROG_CC +END + +: > zar.c +: > doz.c + +$AU
Re: PATCH: patsubst support
| Akim, Hi Alex, Glad to see your progresses! | Here is a patch to add patsubst support to value_to_list. I've | included a new test case "patsubst.test" as well. That's great news! Thanks a lot! But I'm going to be a pain, especially because I'm not the official maintainer of Automake, and understand it much less than I understand Autoconf, so I will be even more picky than I already am for Autoconf patches (wow! That much!?! :). | diff -r -P -u automake/ChangeLog.entry automake-patsubst/ChangeLog.entry | --- automake/ChangeLog.entry Thu Jan 1 01:00:00 1970 | +++ automake-patsubst/ChangeLog.entry Wed Oct 25 14:16:08 2000 | @@ -0,0 +1,5 @@ | +2000-10-25 Alex Hornby <[EMAIL PROTECTED]> | + | + * automake.in (value_to_list): added support for patsubst | + style variable substitution. Please use the imperative: you dictate the specification to someone who will have to implement them. | + * automake.in (value_to_list): Adde support for patsubst | + style variable substitution. | diff -r -P -u automake/automake.in automake-patsubst/automake.in | --- automake/automake.in Wed Oct 25 11:25:01 2000 | +++ automake-patsubst/automake.in Wed Oct 25 14:20:07 2000 | @@ -5902,16 +5902,34 @@ | { | $varname = $1; | $to = $3; | - ($from = $2) =~ s/(\W)/\\$1/g; | + ($from = $2) =~ s/(\W)/$1/g; | } | | # Find the value. | @temp_list = &variable_value_as_list_worker ($1, $cond, $var); | | # Now rewrite the value if appropriate. | - if ($from) | + if ($from =~ '^([^%]*)%([^%]*)') | { I think some comments here would really help next hackers. Let only to state that here is handled the patsubst stuff. | +2000-10-25 Alex Hornby <[EMAIL PROTECTED]> | + | + * patsubst.test: added test for new patsubst expansion | + | + * Makefile.am: reference patsubst.test Likewise (plus capitalization and final period). There are two important missing things in your patch: an entry for NEWS, and the patch for the documentation. I'm sorry to insist, but yet the Automake documentation is slightly deficient, and hard work is needed on that point. Akim
PATCH: patsubst support
Akim, Here is a patch to add patsubst support to value_to_list. I've included a new test case "patsubst.test" as well. After applying the patch you will need to make patsubst.test executable, as I haven't yet found a way to make diffs include permissions :) Cheers, Alex. diff -r -P -u automake/ChangeLog.entry automake-patsubst/ChangeLog.entry --- automake/ChangeLog.entryThu Jan 1 01:00:00 1970 +++ automake-patsubst/ChangeLog.entry Wed Oct 25 14:16:08 2000 @@ -0,0 +1,5 @@ +2000-10-25 Alex Hornby <[EMAIL PROTECTED]> + + * automake.in (value_to_list): added support for patsubst + style variable substitution. + diff -r -P -u automake/automake.in automake-patsubst/automake.in --- automake/automake.inWed Oct 25 11:25:01 2000 +++ automake-patsubst/automake.in Wed Oct 25 14:20:07 2000 @@ -5902,16 +5902,34 @@ { $varname = $1; $to = $3; - ($from = $2) =~ s/(\W)/\\$1/g; + ($from = $2) =~ s/(\W)/$1/g; } # Find the value. @temp_list = &variable_value_as_list_worker ($1, $cond, $var); # Now rewrite the value if appropriate. - if ($from) + if ($from =~ '^([^%]*)%([^%]*)') { - grep (s/$from$/$to/, @temp_list); + # patsubst style substitution + local ($prefrom, $suffrom, $preto, $sufto); + $prefrom = $1; + $suffrom = $2; + + if ( $to =~ '^([^%]*)%([^%]*)') + { + $preto = $1; + $sufto = $2; + } + grep { + s/^$prefrom/$preto/; + s/$suffrom$/$sufto/; + } @temp_list; + } + elsif ($from) + { + # standard substitution reference style + grep (s/$from$/$to/, @temp_list); } push (@result, @temp_list); diff -r -P -u automake/tests/ChangeLog.entry automake-patsubst/tests/ChangeLog.entry --- automake/tests/ChangeLog.entry Thu Jan 1 01:00:00 1970 +++ automake-patsubst/tests/ChangeLog.entry Wed Oct 25 14:15:54 2000 @@ -0,0 +1,5 @@ +2000-10-25 Alex Hornby <[EMAIL PROTECTED]> + + * patsubst.test: added test for new patsubst expansion + + * Makefile.am: reference patsubst.test diff -r -P -u automake/tests/Makefile.am automake-patsubst/tests/Makefile.am --- automake/tests/Makefile.am Wed Oct 25 11:25:02 2000 +++ automake-patsubst/tests/Makefile.am Wed Oct 25 14:13:05 2000 @@ -181,6 +181,7 @@ output5.test \ package.test \ parse.test \ +patsubst.test \ pluseq.test \ pluseq2.test \ pluseq3.test \ diff -r -P -u automake/tests/patsubst.test automake-patsubst/tests/patsubst.test --- automake/tests/patsubst.testThu Jan 1 01:00:00 1970 +++ automake-patsubst/tests/patsubst.test Wed Oct 25 14:13:01 2000 @@ -0,0 +1,21 @@ +#! /bin/sh + +# Test `patsubst expansion' functionality. + +. $srcdir/defs || exit 1 + +cat > Makefile.am << 'END' +bin_PROGRAMS = zardoz +BASENAMES = zar doz +zardoz_SOURCES = ${BASENAMES:%=%.c} +END + +cat >> configure.in << 'END' +AC_PROG_CC +END + +: > zar.c +: > doz.c + +$AUTOMAKE || exit 1 +fgrep 'zar.o doz.o' Makefile.in