Skip to content

Commit 0561e48

Browse files
committed
clean up makefile, fmt, add SUBMODULE_PATHS helper
1 parent 63fbded commit 0561e48

File tree

16 files changed

+57
-327
lines changed

16 files changed

+57
-327
lines changed

Makefile

Lines changed: 30 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,6 @@ $(BUILD)/proto-lint:
6262
$(BUILD)/gomod-lint:
6363
$(BUILD)/goversion-lint:
6464
$(BUILD)/fmt: $(BUILD)/codegen # formatting must occur only after all other go-file-modifications are done
65-
# $(BUILD)/copyright
66-
# $(BUILD)/copyright: $(BUILD)/codegen # must add copyright to generated code, sometimes needs re-formatting
6765
$(BUILD)/codegen: $(BUILD)/thrift $(BUILD)/protoc
6866
$(BUILD)/thrift: $(BUILD)/go_mod_check
6967
$(BUILD)/protoc: $(BUILD)/go_mod_check
@@ -79,6 +77,10 @@ $(BUILD)/go_mod_check:
7977
null :=
8078
SPACE := $(null) #
8179
COMMA := ,
80+
define NEWLINE
81+
82+
83+
endef
8284

8385
# set a V=1 env var for verbose output. V=0 (or unset) disables.
8486
# this is used to make two verbose flags:
@@ -112,6 +114,18 @@ endif
112114

113115
PROJECT_ROOT = github.com/uber/cadence
114116

117+
# go submodules only, the top-level go.mod is easy enough to type by hand
118+
SUBMODULE_PATHS = $(shell \
119+
find . \
120+
-type f \
121+
-name "go.mod" \
122+
-not -path "./go.mod" \
123+
-not -path "./idls/*" \
124+
-exec dirname {} \; \
125+
| sed 's|^\./||' \
126+
)
127+
SUBMODULE_FILES = $(foreach MOD,$(SUBMODULE_PATHS),$(MOD)/go.mod)
128+
115129
# helper for executing bins that need other bins, just `$(BIN_PATH) the_command ...`
116130
# I'd recommend not exporting this in general, to reduce the chance of accidentally using non-versioned tools.
117131
BIN_PATH := PATH="$(abspath $(BIN)):$(abspath $(STABLE_BIN)):$$PATH"
@@ -216,10 +230,6 @@ $(BUILD)/go_mod_check: go.mod internal/tools/go.mod go.work
216230
$Q ./scripts/check-gomod-version.sh github.com/golang/mock/gomock $(if $(verbose),-v)
217231
$Q touch $@
218232

219-
# copyright header checker/writer. only requires stdlib, so no other dependencies are needed.
220-
# $(BIN)/copyright: cmd/tools/copyright/licensegen.go
221-
# $Q go build -o $@ ./cmd/tools/copyright/licensegen.go
222-
223233
# https://docs.buf.build/
224234
# changing BUF_VERSION will automatically download and use the specified version.
225235
BUF_VERSION = 0.36.0
@@ -372,12 +382,11 @@ $(BUILD)/proto-lint: $(PROTO_FILES) $(STABLE_BIN)/$(BUF_VERSION_BIN) | $(BUILD)
372382

373383
# lints that go modules are as expected, e.g. parent does not import submodule.
374384
# tool builds that need to be in sync with the parent are partially checked through go_mod_build_tool, but should probably be checked here too
375-
$(BUILD)/gomod-lint: go.mod internal/tools/go.mod common/archiver/gcloud/go.mod | $(BUILD)
385+
$(BUILD)/gomod-lint: go.mod $(SUBMODULE_FILES) | $(BUILD)
376386
$Q echo "checking for direct submodule dependencies in root go.mod..."
377387
$Q ( \
378388
MAIN_MODULE=$$(grep "^module " go.mod | awk '{print $$2}'); \
379-
SUBMODULES=$$(find . -type f -name "go.mod" -not -path "./go.mod" -not -path "./idls/*" -exec dirname {} \; | sed 's|^\./||'); \
380-
for submodule in $$SUBMODULES; do \
389+
for submodule in $(SUBMODULE_PATHS); do \
381390
submodule_path="$$MAIN_MODULE/$$submodule"; \
382391
if grep -q "$$submodule_path" go.mod; then \
383392
echo "ERROR: Root go.mod directly depends on submodule: $$submodule_path" >&2; \
@@ -410,19 +419,6 @@ $(BUILD)/goversion-lint: go.work Dockerfile docker/github_actions/Dockerfile${DO
410419
$Q ./scripts/check-go-toolchain.sh $(GOWORK_TOOLCHAIN)
411420
$Q touch $@
412421

413-
# fmt and copyright are mutually cyclic with their inputs, so if a copyright header is modified:
414-
# - copyright -> makes changes
415-
# - fmt sees changes -> makes changes
416-
# - now copyright thinks it needs to run again (but does nothing)
417-
# - which means fmt needs to run again (but does nothing)
418-
# and now after two passes it's finally stable, because they stopped making changes.
419-
#
420-
# this is not fatal, we can just run 2x.
421-
# to be fancier though, we can detect when *both* are run, and re-touch the book-keeping files to prevent the second run.
422-
# this STRICTLY REQUIRES that `copyright` and `fmt` are mutually stable, and that copyright runs before fmt.
423-
# if either changes, this will need to change.
424-
MAYBE_TOUCH_COPYRIGHT=
425-
426422
# use FRESH_ALL_SRC so it won't miss any generated files produced earlier.
427423
$(BUILD)/fmt: $(ALL_SRC) $(BIN)/goimports $(BIN)/gci | $(BUILD)
428424
$Q echo "removing unused imports..."
@@ -431,12 +427,6 @@ $(BUILD)/fmt: $(ALL_SRC) $(BIN)/goimports $(BIN)/gci | $(BUILD)
431427
$Q echo "grouping imports..."
432428
$Q $(BIN)/gci write --section standard --section 'Prefix(github.com/uber/cadence/)' --section default --section blank $(FRESH_ALL_SRC)
433429
$Q touch $@
434-
# $Q $(MAYBE_TOUCH_COPYRIGHT)
435-
436-
# $(BUILD)/copyright: $(ALL_SRC) $(BIN)/copyright | $(BUILD)
437-
# $(BIN)/copyright --verifyOnly
438-
# $Q $(eval MAYBE_TOUCH_COPYRIGHT=touch $@)
439-
# $Q touch $@
440430

441431
# ====================================
442432
# developer-oriented targets
@@ -453,7 +443,7 @@ $Q rm -f $(addprefix $(BUILD)/,$(1))
453443
$Q +$(MAKE) --no-print-directory $(addprefix $(BUILD)/,$(1))
454444
endef
455445

456-
.PHONY: lint fmt copyright pr
446+
.PHONY: lint fmt pr
457447

458448
# useful to actually re-run to get output again.
459449
# reuse the intermediates for simplicity and consistency.
@@ -463,11 +453,6 @@ lint: ## (Re)run the linter
463453
# intentionally not re-making, it's a bit slow and it's clear when it's unnecessary
464454
fmt: $(BUILD)/fmt ## Run `gofmt` / organize imports / etc
465455

466-
# not identical to the intermediate target, but does provide the same codegen (or more).
467-
# copyright: $(BIN)/copyright | $(BUILD) ## Update copyright headers
468-
# $(BIN)/copyright
469-
# $Q touch $(BUILD)/copyright
470-
471456
define make_quietly
472457
$Q echo "make $1..."
473458
$Q output=$$(mktemp); $(MAKE) $1 > $$output 2>&1 || ( cat $$output; echo -e '\nfailed `make $1`, check output above' >&2; exit 1)
@@ -479,7 +464,6 @@ pr: ## Redo all codegen and basic checks, to ensure your PR will be able to run
479464
$Q $(if $(verbose),$(MAKE) go-generate,$(call make_quietly,go-generate))
480465
$Q $(if $(verbose),$(MAKE) fmt,$(call make_quietly,fmt))
481466
$Q $(if $(verbose),$(MAKE) lint,$(call make_quietly,lint))
482-
# $Q $(if $(verbose),$(MAKE) copyright,$(call make_quietly,copyright))
483467

484468
# ====================================
485469
# binaries to build
@@ -533,7 +517,7 @@ cadence-bench: $(BINS_DEPEND_ON)
533517
$Q ./scripts/build-with-ldflags.sh -o $@ cmd/bench/main.go
534518

535519

536-
BINS += cadence-releaser
520+
BINS += cadence-releaser
537521
cadence-releaser: $(BINS_DEPEND_ON)
538522
$Q echo "compiling cadence-releaser with OS: $(GOOS), ARCH: $(GOARCH)"
539523
$Q ./scripts/build-with-ldflags.sh -o $@ cmd/tools/releaser/releaser.go
@@ -549,8 +533,6 @@ go-generate: $(BIN)/mockgen $(BIN)/enumer $(BIN)/mockery $(BIN)/gowrap ## Run `
549533
$Q # add our bins to PATH so `go generate` can find them
550534
$Q $(BIN_PATH) go generate $(if $(verbose),-v) ./...
551535
$Q $(MAKE) --no-print-directory fmt
552-
# $Q echo "updating copyright headers"
553-
# $Q $(MAKE) --no-print-directory copyright
554536

555537
release: ## Re-generate generated code and run tests
556538
$(MAKE) --no-print-directory go-generate
@@ -569,15 +551,21 @@ build: ## `go build` all packages and tests (a quick compile check only, skips a
569551
$Q cd cmd/server; go test -exec /usr/bin/true ./... >/dev/null
570552

571553
tidy: ## `go mod tidy` all packages
572-
$Q # tidy in dependency order
554+
$Q # tidy needs to run in dependency order:
555+
$Q # everything depends on the main module, server depends on everything else so it should be last.
556+
$Q # but we can just tidy everything 2x, which should be stable as our dependency tree is only 2 deep.
573557
$Q go mod tidy
574-
$Q cd common/archiver/gcloud; go mod tidy || (echo "failed to tidy gcloud plugin, try manually copying go.mod contents into common/archiver/gcloud/go.mod and rerunning" >&2; exit 1)
575-
$Q cd cmd/server; go mod tidy || (echo "failed to tidy main server module, try manually copying go.mod and common/archiver/gcloud/go.mod contents into cmd/server/go.mod and rerunning" >&2; exit 1)
558+
$Q $(foreach sub,$(SUBMODULE_PATHS), \
559+
$Q cd $(sub); go mod tidy || $$(>&2 echo "failed to tidy $(sub) plugin, try manually copying go.mod contents into $(sub)/go.mod and rerunning by hand"; exit 1)$(NEWLINE) \
560+
)
561+
$Q $(foreach sub,$(SUBMODULE_PATHS), \
562+
$Q cd $(sub); go mod tidy || $$(>&2 echo "failed to tidy $(sub) plugin, try manually copying go.mod contents into $(sub)/go.mod and rerunning by hand"; exit 1)$(NEWLINE) \
563+
)
576564

577565
clean: ## Clean build products and SQLite database
578566
rm -f $(BINS)
579567
rm -Rf $(BUILD)
580-
rm *.db
568+
rm -f *.db
581569
$(if \
582570
$(wildcard $(STABLE_BIN)/*), \
583571
$(warning usually-stable build tools still exist, delete the $(STABLE_BIN) folder to rebuild them),)

0 commit comments

Comments
 (0)