Re: [Automake-NG] [PATCH 3/3] [ng] memoize: correctly work around a bug of GNU make < 3.83

[Stefano Lattarini]

Hi Akim.

On 05/15/2012 01:44 PM, Akim Demaille wrote:
> Hi Stefano!
> 
> Le 15 mai 2012 à 12:44, Stefano Lattarini a écrit :
> 
>> +am__memoize = \
>> +  $(if $(am__memoized_value/$1),$(am__memoized_value/$1),$(strip \
>> +    $(eval am__memoized_value/$1 := $$2))$(am__memoized_value/$1))
> 
> If the result is large, we might be evaluating twice
> am__memoized_value/$1 for no good reason.  Can't $(or)
> be used instead?
> 
> Or maybe uses $(origin) to implement ifndef.
> 
> But maybe my model of Make is wrong.  I'm thinking like if
> it were m4 here, in which case avoiding duplicate expansion
> would be a Good Thing.  If $(if) is basically O(1) for
> :=-variables, there is no worry.
>
I trust GNU make to be smart enough to handle an $(if ...) like the
one I've written in O(1).  Still, the use of $(or ...) is cleaner
and clearer, an portable back to GNU make 3.80 (our minimal required
version anyway), so I've switched to it.  At this point, I've also
made you the main author of the patch.

Below is what Ill push shortly.

Thanks,
  Stefano

-*-*-*-

>From 78a3c3018a692d18c4137577edc2058bd4b072fa Mon Sep 17 00:00:00 2001
Message-Id: <address@hidden>
From: Akim Demaille <address@hidden>
Date: Tue, 15 May 2012 12:36:37 +0200
Subject: [PATCH] [ng] memoize: correctly work around a bug of GNU make < 3.83

This is a follow-up to commit v1.12-218-g34a7d42 of 2012-05-15,
"[ng] memoize: expose a serious bug (affecting GNU make < 3.83)".

We implement a new workaround aimed at avoiding the subtle GNU make
bug affecting variable memoization:

  <http://lists.gnu.org/archive/html/bug-make/2012-05/msg00013.html>
  <https://savannah.gnu.org/patch/?7534>

Since that bug basically prevents dynamically reassigning to a variable
inside the expansion of the variable itself, we reimplement memoization
by having the memoized variable expanding to an $(if) branching either
to evaluate-and-store in *another* memo variable, else to the value of
that memo variable.

The new workaround works also in the situation that was causing our
previous workaround (as implemented in commit v1.12-217-gd55204f of
2012-05-12, "[ng] vars: implement memoization of make variables (new
'am__memoize' func)") to fail.

Approach suggested by Akim Demaille:
<http://lists.gnu.org/archive/html/automake-ng/2012-05/msg00074.html>
and coded by Stefano Lattarini.

* lib/am/header-vars (am__memoize): Change API and implementation.
(am__memoize_0): Delete, it's not needed anymore.
* t/memoize.tap: Update to new API, also removing checks that makes no
more sense with it.
* t/spy-override.sh: Delete, it's not needed anymore.

Co-authored-by: Stefano Lattarini <address@hidden>
Signed-off-by: Stefano Lattarini <address@hidden>
---
 lib/am/header-vars.am |   13 ++++----
 t/memoize.tap         |   81 +++++++++++++++++--------------------------------
 t/spy-override.sh     |   51 -------------------------------
 3 files changed, 35 insertions(+), 110 deletions(-)
 delete mode 100755 t/spy-override.sh

diff --git a/lib/am/header-vars.am b/lib/am/header-vars.am
index 95345e4..d620c89 100644
--- a/lib/am/header-vars.am
+++ b/lib/am/header-vars.am
@@ -83,19 +83,20 @@ am__uniq = $(strip \
 ## we can't afford to re-calculate it over and over every time the
 ## variable gets expanded.  Example of usage:
 ##
-##   memo/var1 = $(shell EXPENSIVE-COMMAND-LINE)
-##   memo/var2 = $(sort VERY-LONG-LIST)
-##   $(call am__memoize, var1 var2)
+##   var1 = $(am__memoize,var1,$(shell EXPENSIVE-COMMAND-LINE))
+##   var2 = $(am__memoize,var2,$(sort VERY-LONG-LIST))
 ##
 ## This API and implementation seems to work around a bug in GNU make
 ## (present up to and including version 3.82) which caused our first
 ## implementation attempts to fail:
+##
 ##   <http://lists.gnu.org/archive/html/bug-make/2012-05/msg00013.html>
 ##   <https://savannah.gnu.org/patch/?7534>
+##
 ## So please don't change this without a very good reason.
-am__memoize_0 = $(eval $1 = $$(eval override $1 := $$$$($2))$$($1))
-am__memoize = $(foreach am__memo_var, $1, \
-                $(call $(0)_0,$(am__memo_var),memo/$(am__memo_var)))
+##
+am__memoize = $(or $(am__memoized_value/$1),$(strip \
+  $(eval am__memoized_value/$1 := $$2))$(am__memoized_value/$1))

 ## Some derived variables that have been found to be useful.
 pkgdatadir = $(datadir)/@PACKAGE@
diff --git a/t/memoize.tap b/t/memoize.tap
index f379e34..ca7fede 100755
--- a/t/memoize.tap
+++ b/t/memoize.tap
@@ -19,7 +19,7 @@
 am_create_testdir=empty
 . ./defs || Exit 1

-plan_ 14
+plan_ 13

 ocwd=`pwd` || fatal_ "couldn't get current working directory"

@@ -52,8 +52,7 @@ tcount=0

 T "basic usage" <<'END'

-memo/foo = ok
-$(call am__memoize,foo)
+foo = $(call am__memoize,foo,ok)

 test:
        test '$(foo)' = ok
@@ -62,28 +61,25 @@ END

 #---------------------------------------------------------------------------

-T "call with extra spaces" <<'END'
+T "variables expanding to blanks only (1)" <<'END'

-memo/foo = ok
-## There are extra spaces and tabs here; do not normalize them!
-$(call am__memoize,    foo             )
+foo = $(call am__memoize,foo,$(am__empty) )

 test:
-       test '$(foo)' = ok
-       test '$(foo)' = ok
+       test '$(foo)' = ' '
+       test '$(foo)' = ' '
 END

 #---------------------------------------------------------------------------

-T "memoize several variables at once" <<'END'
+T "variables expanding to blanks only (2)" <<END

-memo/foo1 = bar
-memo/foo2 = baz
-$(call am__memoize, foo1 foo2)
+blank = \$(am__empty) $tab$tab   \$(am__empty)
+foo = \$(call am__memoize,foo,\$(blank))

 test:
-       test '$(foo1)' = bar
-       test '$(foo2)' = baz
+       test '\$(foo)' = ' $tab$tab   '
+       test '\$(foo)' = ' $tab$tab   '
 END

 #---------------------------------------------------------------------------
@@ -96,8 +92,7 @@ done

 T "very long variable name" <<END

-memo/$var = foo
-\$(call am__memoize,$var)
+$var = \$(call am__memoize,$var,foo)

 test:
        test '\$($var)' = foo
@@ -116,8 +111,7 @@ done

 T "very long variable name with long content" <<END

-memo/$var = $val
-\$(call am__memoize,$var)
+$var = \$(call am__memoize,$var,$val)

 test:
        test '\$($var)' = '$val'
@@ -128,8 +122,7 @@ END

 T "on indirect recursive variable expansion" <<'END'

-memo/foo = $(indir)
-$(call am__memoize,foo)
+foo = $(call am__memoize,foo,$(indir))

 ## This is delibrately placed after the memoize call.
 indir = zardoz
@@ -143,8 +136,7 @@ END

 T "on indirect immediate variable expansion" <<'END'

-memo/foo = $(indir)
-$(call am__memoize,foo)
+foo = $(call am__memoize,foo,$(indir))

 ## This is delibrately placed after the memoize call.
 indir := blob
@@ -159,9 +151,7 @@ END
 T "on function call" <<'END'

 my_func = $(firstword $(sort $(1)))
-
-memo/foo = $(call my_func, 6 3 1 7)
-$(call am__memoize,foo)
+foo = $(call am__memoize,foo,$(call my_func, 6 3 1 7))

 test:
        test '$(foo)' = 1
@@ -170,22 +160,10 @@ END

 #---------------------------------------------------------------------------

-T "out-of-order memoization should work" <<'END'
-
-$(call am__memoize,foo)
-test:
-       test '$(foo)' = ok
-       test '$(foo)' = ok
-memo/foo = ok
-END
-
-#---------------------------------------------------------------------------
-
 T "memoization actually takes place (1)" <<'END'

 indir := ok
-memo/foo = $(indir)
-$(call am__memoize,foo)
+foo = $(call am__memoize,foo,$(indir))
 expand-it := $(foo)
 override indir := ko

@@ -198,10 +176,8 @@ END

 T "memoization actually takes place (2)" <<'END'

-memo/foo1 = $(shell test -f x && echo "+")
-memo/foo2 = $(shell test -f y || echo "-")
-$(call am__memoize,foo1)
-$(call am__memoize,foo2)
+foo1 = $(call am__memoize,foo1,$(shell test -f x && echo "+"))
+foo2 = $(call am__memoize,foo2,$(shell test -f y || echo "-"))

 test: one two
 two: one
@@ -229,12 +205,11 @@ END

 T "definition on multiple lines" <<'END'

-memo/foo = a \
-b \
-c \
-\
-d
-$(call am__memoize,foo)
+foo = $(call am__memoize,foo,a \
+  b \
+  c \
+  \
+  d)

 test:
        test '$(foo)' = 'a b c d'
@@ -253,8 +228,7 @@ T "known GNU make issue 
(https://savannah.gnu.org/patch/?7534)" <<'END'
 setup := $(shell mkdir -p t/pm && : > t/pm/Cond2.pl)

 TESTS = $(wildcard t/pm/Cond2.pl)
-memo/am__test_bases = $(TESTS)
-$(call am__memoize,am__test_bases)
+am__test_bases = $(call am__memoize,am__test_bases,$(TESTS))

 test:
        test '$(am__test_bases)' = 't/pm/Cond2.pl'
@@ -276,10 +250,11 @@ for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 
20; do
   mv -f t1 t
 done

-(echo 'memo/list = \' && cat t && echo "  $line") > big.mk
+echo "list = \$(call am__memoize,list,$line \\" >> big.mk
+cat t >> big.mk
+echo "  $line)" >> big.mk

 cat >> big.mk << 'END'
-$(call am__memoize,list)
 test:
        test x'$(word  1, $(list))' = x'dummy'
        test x'$(word  3, $(list))' = x'='
diff --git a/t/spy-override.sh b/t/spy-override.sh
deleted file mode 100755
index de552e4..0000000
--- a/t/spy-override.sh
+++ /dev/null
@@ -1,51 +0,0 @@
-#! /bin/sh
-# Copyright (C) 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 <http://www.gnu.org/licenses/>.
-
-# Verify that use of 'override' directive to re-set a variable does
-# not cause any warning or extra output.
-
-am_create_testdir=empty
-. ./defs || Exit 1
-
-cat > Makefile <<'END'
-foo = 1
-override foo = 2
-
-bar = 3
-override bar := 4
-
-override baz = 6
-override zap := 8
-
-override zardoz += doz
-
-nihil:
-       @:
-sanity-check:
-       test '$(foo)' = 2
-       test '$(bar)' = 4
-       test '$(baz)' = 6
-       test '$(zap)' = 8
-       test '$(zardoz)' = 'zar doz'
-.PHONY: nihil sanity-check
-END
-
-$MAKE sanity-check baz=5 zap=7 zardoz=zar
-$MAKE --no-print-directory nihil >output 2>&1 \
-  && test ! -s output \
-  || { cat output; Exit 1; }
-
-:
-- 
1.7.9.5