my-ship-it commented on code in PR #1802:
URL: https://github.com/apache/cloudberry/pull/1802#discussion_r3362389346
##########
gpcontrib/Makefile:
##########
@@ -85,12 +85,16 @@ install:
@if [ "$(enable_orafce)" = "yes" ]; then \
$(MAKE) -C orafce NO_PGXS=true install; \
fi
+ @if [ "$(with_zstd)" = "yes" ]; then \
Review Comment:
Hi @reshke thanks for the patch!
▎ How does it even work without this?
It works through the recurse mechanism that's already in place. A few
lines above in the same Makefile:
ifeq "$(with_zstd)" "yes"
recurse_targets += zstd
endif
...
$(call recurse,all install clean distclean, $(recurse_targets))
The recurse function (defined in src/Makefile.global.in) uses $(eval) to
generate, for each target/subdir pair:
install: install-zstd-recurse
install-zstd-recurse: submake-generated-headers
$(MAKE) -C zstd install
Since GNU make allows adding prerequisites to a target whose recipe is
defined elsewhere, the explicit install: recipe below picks up
install-zstd-recurse as a prerequisite — so make install already descends
into zstd/ before running the gpcloud/mapreduce/pxf/orafce steps. Same for
all, clean, and distclean.
This is easy to verify with a dry run:
$ make -n -C gpcontrib install | grep "zstd install"
make -C zstd install
With this patch applied, make -C zstd install (and clean) would run twice
— harmless, but redundant.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
