Merge cockroachdb#62698

62698: build: fix spurious rebuilds due to geo .dylib r=jordanlewis a=jordanlewis

Previously, the .ALWAYS_REBUILD target for the libgeos .dylib caused
some bad behavior. Since the .dylib would be replaced on every
invocation to `make`, and since the .dylib was also included in the list
of C_LIBS which are dependencies to nearly every Makefile binary target,
it meant that the Makefile would cause spurious rebuilds of binary
targets every other build.

This was particularly obvious when running the `make testlogic`
variants, which usually permit editing the testdata text files without
causing a rebuild to bin/logictest. Before this patch, the bin/logictest
binary would be rebuilt every other invocation to `make`, even if no
source files changed.

The fix is to move the .dylib files to their own dependency list, and
mark them as order-only dependencies in the places that need them. This
is okay because, as dynamic libraries, changes to them don't need to
force changes to the binaries that depend on them.

Release note: None

Co-authored-by: Jordan Lewis <[email protected]>

Makefile

@@ -519,10 +519,11 @@ LIBGEOS     := $(DYN_LIB_DIR)/libgeos.$(DYN_EXT)
 C_LIBS_COMMON = \
 	$(if $(use-stdmalloc),,$(LIBJEMALLOC)) \
 	$(if $(target-is-windows),,$(LIBEDIT)) \
-	$(LIBPROJ) $(LIBGEOS) $(LIBROACH)
+	$(LIBPROJ) $(LIBROACH)
 C_LIBS_SHORT = $(C_LIBS_COMMON)
 C_LIBS_OSS = $(C_LIBS_COMMON) $(LIBPROTOBUF)
 C_LIBS_CCL = $(C_LIBS_COMMON) $(LIBPROTOBUF)
+C_LIBS_DYNAMIC = $(LIBGEOS)
 
 # We only include krb5 on linux, non-musl builds.
 ifeq "$(findstring linux-gnu,$(TARGET_TRIPLE))" "linux-gnu"
@@ -942,7 +943,7 @@ go-targets := $(go-targets-ccl) $(COCKROACHOSS) $(COCKROACHSHORT)
 all: build
 
 .PHONY: c-deps
-c-deps: $(C_LIBS_CCL)
+c-deps: $(C_LIBS_CCL) | $(C_LIBS_DYNAMIC)
 
 build-mode = build -o $@
 
@@ -951,16 +952,16 @@ go-install: build-mode = install
 $(COCKROACH) go-install generate: pkg/ui/distccl/bindata.go
 
 $(COCKROACHOSS): BUILDTARGET = ./pkg/cmd/cockroach-oss
-$(COCKROACHOSS): $(C_LIBS_OSS) pkg/ui/distoss/bindata.go
+$(COCKROACHOSS): $(C_LIBS_OSS) pkg/ui/distoss/bindata.go | $(C_LIBS_DYNAMIC)
 
 $(COCKROACHSHORT): BUILDTARGET = ./pkg/cmd/cockroach-short
 $(COCKROACHSHORT): TAGS += short
-$(COCKROACHSHORT): $(C_LIBS_SHORT)
+$(COCKROACHSHORT): $(C_LIBS_SHORT) | $(C_LIBS_DYNAMIC)
 
 # For test targets, add a tag (used to enable extra assertions).
 $(test-targets): TAGS += crdb_test
 
-$(go-targets-ccl): $(C_LIBS_CCL)
+$(go-targets-ccl): $(C_LIBS_CCL) | $(C_LIBS_DYNAMIC)
 
 BUILDINFO = .buildinfo/tag .buildinfo/rev
 BUILD_TAGGED_RELEASE =
@@ -1768,7 +1769,7 @@ logictest-bins := bin/logictest bin/logictestopt bin/logictestccl
 # TODO(benesch): Derive this automatically. This is getting out of hand.
 bin/workload bin/docgen bin/execgen bin/roachtest $(logictest-bins): $(SQLPARSER_TARGETS) $(LOG_TARGETS) $(PROTOBUF_TARGETS)
 bin/workload bin/docgen bin/roachtest $(logictest-bins): $(LIBPROJ) $(CGO_FLAGS_FILES)
-bin/roachtest $(logictest-bins): $(C_LIBS_CCL) $(CGO_FLAGS_FILES) $(OPTGEN_TARGETS)
+bin/roachtest $(logictest-bins): $(C_LIBS_CCL) $(CGO_FLAGS_FILES) $(OPTGEN_TARGETS) | $(C_LIBS_DYNAMIC)
 
 $(bins): bin/%: bin/%.d | bin/prereqs bin/.submodules-initialized
 	@echo go install -v $*

build/variables.mk

@@ -34,6 +34,7 @@ define VALID_VARS
   C_DEPS_DIR
   C_LIBS_CCL
   C_LIBS_COMMON
+  C_LIBS_DYNAMIC
   C_LIBS_OSS
   C_LIBS_SHORT
   DESTDIR