Makefile refactor #119
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| /bin | ||
| /doc/repl/glj.wasm | ||
| /report.html | ||
| .direnv | ||
|
|
||
| # useful to symlink in for context | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,29 @@ | ||
| # Usage: | ||
| # make clean all test GO-VERSION=1.25.1 | ||
|
|
||
| SHELL := bash | ||
|
|
||
| GO-VERSION ?= 1.19.3 | ||
| CLOJURE-VERSION ?= 1.12.1 | ||
|
|
||
| CLOJURE-STDLIB-VERSION := clojure-$(CLOJURE-VERSION) | ||
| STDLIB-ORIGINALS-DIR := scripts/rewrite-core/originals | ||
| STDLIB-ORIGINALS := $(wildcard $(STDLIB-ORIGINALS-DIR)/*.clj) | ||
| STDLIB-NAMES := $(STDLIB-ORIGINALS:scripts/rewrite-core/originals/%=%) | ||
| STDLIB-ORIGINALS := $(STDLIB-NAMES:%=scripts/rewrite-core/originals/,%) | ||
|
Contributor
Author
There was a problem hiding this comment. Yes. It's this line in |
||
| STDLIB-TARGETS := $(addprefix pkg/stdlib/clojure/,$(STDLIB-NAMES:.clj=.glj)) | ||
|
|
||
| AOT-NAMESPACES := \ | ||
| clojure.core \ | ||
| clojure.core.async \ | ||
| clojure.string \ | ||
| clojure.template \ | ||
| clojure.test \ | ||
| clojure.uuid \ | ||
| clojure.walk \ | ||
| glojure.go.io \ | ||
| glojure.go.types \ | ||
| $(EXTRA-AOT-NAMESPACES) | ||
|
|
||
| OS-TYPE := $(shell bash -c 'echo $$OSTYPE') | ||
| OS-NAME := \ | ||
|
|
@@ -23,88 +42,142 @@ OA-linux-arm64 := linux_arm64 | |
| OA-linux-int64 := linux_amd64 | ||
| OA-macos-arm64 := darwin_arm64 | ||
| OA-macos-int64 := darwin_amd64 | ||
| GLJ-CMD := bin/$(OA-$(OS-ARCH))/glj | ||
| endif | ||
| endif | ||
|
|
||
| TEST-GLJ-DIR := test/glojure | ||
| TEST-GLJ-FILES := $(shell find $(TEST-GLJ-DIR) -name '*.glj' | sort) | ||
| TEST-GLJ-TARGETS := $(addsuffix .test,$(TEST-GLJ-FILES)) | ||
| TEST-SUITE-DIR := test/clojure-test-suite | ||
| TEST-SUITE-FILE := test-glojure.glj | ||
|
|
||
| GO-PLATFORMS := \ | ||
| darwin_arm64 \ | ||
| darwin_amd64 \ | ||
| linux_arm64 \ | ||
| linux_amd64 \ | ||
| windows_arm \ | ||
| windows_amd64 \ | ||
| js_wasm \ | ||
| $(EXTRA-GO-PLATFORMS) | ||
|
|
||
| GLJ-IMPORTS=$(foreach platform,$(GO-PLATFORMS) \ | ||
| ,pkg/gen/gljimports/gljimports_$(platform).go) | ||
|
|
||
| # wasm should have .wasm suffix; others should not | ||
| GLJ-BINS=$(foreach platform,$(GO-PLATFORMS) \ | ||
| ,bin/$(platform)/glj$(if $(findstring wasm,$(platform)),.wasm,)) | ||
|
|
||
| GO-CMD := go$(GO-VERSION) | ||
|
|
||
| ALL-TARGETS := \ | ||
| $(if $(force),update-clojure-sources) \ | ||
| stdlib-targets \ | ||
| generate \ | ||
| aot \ | ||
| glj-imports \ | ||
| glj-bins \ | ||
|
|
||
| #------------------------------------------------------------------------------- | ||
| default: all | ||
|
|
||
|
Contributor
There was a problem hiding this comment. should we keep the
Contributor
Author
There was a problem hiding this comment. No. Here https://github.com/glojurelang/glojure/blob/main/Makefile#L44
Contributor
Author
Contributor
There was a problem hiding this comment. fwiw i've been bitten by missing .PHONY before, but i don't feel strongly. happy to run with this
Contributor
Author
There was a problem hiding this comment. Me too, but those bites are rare and almost instantly recognized.
Contributor
Author
There was a problem hiding this comment. Here's an interesting fact about PHONY you might now know... You can put all your PHONY targets on one line at the end of the file if you want:
Contributor
There was a problem hiding this comment.
ah, i'm familiar with that, but i prefer to keep them adjacent to their targets so it's harder for them to get out of sync — which i managed to do with one of the targets anyway :) |
||
| # Dummy target for commands like: | ||
| # make all force=1 | ||
| # make stdlib-targets force=1 | ||
| force: | ||
|
|
||
| all: $(ALL-TARGETS) | ||
|
|
||
| gocmd: | ||
| @$(GO-CMD) version &> /dev/null || { \ | ||
| (go install "golang.org/dl/$(GO-CMD)@latest" && \ | ||
| $(GO-CMD) download > /dev/null && \ | ||
| $(GO-CMD) version > /dev/null); } | ||
|
|
||
| stdlib-targets: $(STDLIB-TARGETS) | ||
|
|
||
| generate: | ||
| go generate ./... | ||
|
|
||
| aot: gocmd $(STDLIB-TARGETS) | ||
| GLOJURE_USE_AOT=false \ | ||
| GLOJURE_STDLIB_PATH=./pkg/stdlib \ | ||
| $(GO-CMD) run -tags glj_no_aot_stdlib ./cmd/glj \ | ||
| <<<"(map compile '[$(AOT-NAMESPACES)])" | ||
|
|
||
| glj-imports: $(GLJ-IMPORTS) | ||
|
|
||
| glj-bins: $(GLJ-BINS) | ||
|
|
||
| build: $(GLJ-CMD) | ||
|
|
||
| clean: | ||
| $(RM) report.html | ||
| $(RM) -r bin/ scripts/rewrite-core/.cpcache/ | ||
|
|
||
| pkg/gen/gljimports/gljimports_%.go: \ | ||
| ./scripts/gen-gljimports.sh \ | ||
| ./cmd/gen-import-interop/main.go \ | ||
| ./internal/genpkg/genpkg.go \ | ||
| $(wildcard ./pkg/lang/*.go) \ | ||
| $(wildcard ./pkg/runtime/*.go) \ | ||
| $(if $(force),force) | ||
| @echo "Generating $@" | ||
| ./scripts/gen-gljimports.sh $@ $* $(GO-CMD) | ||
|
|
||
| pkg/stdlib/clojure/%.glj: \ | ||
| scripts/rewrite-core/originals/%.clj \ | ||
| scripts/rewrite-core/run.sh \ | ||
| scripts/rewrite-core/rewrite.clj \ | ||
| $(if $(force),force) | ||
| @echo "Rewriting $< to $@" | ||
| @mkdir -p $(dir $@) | ||
| scripts/rewrite-core/run.sh $< > $@ | ||
|
|
||
| bin/%/glj: generate \ | ||
| $(wildcard ./cmd/glj/*.go) \ | ||
| $(wildcard ./pkg/**/*.go) \ | ||
| $(wildcard ./internal/**/*.go) \ | ||
| $(if $(force),force) | ||
| @echo "Building $@" | ||
| @mkdir -p $(dir $@) | ||
| scripts/build-glj.sh $@ $* | ||
|
|
||
| bin/%/glj.wasm: \ | ||
| $(wildcard ./cmd/glj/*.go) \ | ||
| $(wildcard ./pkg/**/*.go) \ | ||
| $(wildcard ./internal/**/*.go) \ | ||
| $(if $(force),force) | ||
| @echo "Building $@" | ||
| @mkdir -p $(dir $@) | ||
| scripts/build-glj.sh $@ $* | ||
|
|
||
| vet: | ||
| go vet ./... | ||
|
|
||
| .PHONY: test | ||
| # vet is disabled until we fix errors in generated code | ||
| test: test-glj test-suite # vet | ||
|
|
||
| test-glj: $(TEST-GLJ-TARGETS) | ||
|
|
||
| test-suite: $(GLJ-CMD) | ||
| cd $(TEST-SUITE-DIR) && \ | ||
| $(abspath $<) $(TEST-SUITE-FILE) \ | ||
| --expect-failures 38 \ | ||
| --expect-errors 151 \ | ||
| 2>/dev/null | ||
|
|
||
| $(TEST-GLJ-TARGETS): $(GLJ-CMD) | ||
| $< $(basename $@) | ||
|
|
||
| format: | ||
| @if go fmt ./... | grep -q ''; then \ | ||
| echo "Files were formatted. Please commit the changes."; \ | ||
| exit 1; \ | ||
| fi | ||
|
|
||
| update-clojure-sources: | ||
| scripts/rewrite-core/update-clojure-sources.sh \ | ||
| $(CLOJURE-STDLIB-VERSION) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the switch from SNAKE_CASE to KEBAB-CASE? i've seen kebab for targets but snake seems like the norm for variables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if you read through my explanations in the commit messages, but 7f2d72d
Makefiles inherit from the env which only allows SNAKE_CASE. KEBAB-BASE is only allowed in Makefile vars, not env ones, so it makes the vars ownership very clear.
You can override vars from the command line like this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also in commit 1b0f4cd I made almost all of the SINGLEWORD Makefile vars into MULTI-WORD vars, to drive home that they don't ever come from (can't come from) the environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair point! don't feel strongly, but i wanted to understand the motivation and i failed to read the (useful!) commit messages :)