[PATCH 2/2] Makefile: clarify definition of TEST_OBJS
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Feb 28, 2010, 09:11 UTC
- Message-ID
- <20100228091155.GB30143@progeny.tock>
- In-Reply-To
- <63cde7731002271503oac53237ubed6d318b46042e9@mail.gmail.com>
The definition of TEST_OBJS in commit daa99a91 (Makefile: make sure test helpers are rebuilt when headers change, 2010-01-26) moved a use of $X to before the platform-specific section where it gets defined. There are at least two ways to fix that:
- Change the definition of TEST_OBJS to use the = delayed evaluation operator. This way, one need not worry about $(X) needing to be defined before TEST_OBJS is set.
- Move the definition of TEST_OBJS to below the definition of $X.
Carry out the second. The later site of definition makes the code more readable, since now a reader only has to look down one line to see what TEST_OBJS is meant to be used for.
Oddly enough, with or without this change the behavior of the Makefile is the same. Since TEST_PROGRAMS is defined with delayed evaluation, the value of
TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS))
is independent of the value of $X when it is evaluated: the $X in the pattern and the $X in $(TEST_PROGRAMS) will simply always cancel out. Make sure $X has the expected expansion anyway to make the code and the reader’s sanity more robust in the face of future changes.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- Michael Lukashov wrote:
Show 9 quoted lines
> It seems there's no difference between > > TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS)) > > and > > TEST_OBJS = $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS)) > > Both variants seem to work under mingw.
Yep. I think the unexpected value of $X is worth fixing regardless just to keep people from going insane.
Thanks, both.
Makefile | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile index b64eec1..e95c128 100644 --- a/Makefile +++ b/Makefile @@ -738,8 +738,6 @@ BUILTIN_OBJS += builtin-verify-pack.o BUILTIN_OBJS += builtin-verify-tag.o BUILTIN_OBJS += builtin-write-tree.o -TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS)) - GITLIBS = $(LIB_FILE) $(XDIFF_LIB) EXTLIBS = @@ -1686,6 +1684,7 @@ git.o git.spec \ $(patsubst %.perl,%,$(SCRIPT_PERL)) \ : GIT-VERSION-FILE +TEST_OBJS := $(patsubst test-%$X,test-%.o,$(TEST_PROGRAMS)) GIT_OBJS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \ git.o http.o http-walker.o remote-curl.o XDIFF_OBJS = xdiff/xdiffi.o xdiff/xprepare.o xdiff/xutils.o xdiff/xemit.o \
-- 1.7.0