Skip to content

Commit c800de2

Browse files
committed
simplify Makefile
- use git to check for out-of-date generated files, avoids all the temp files and manual diff in the Makefile. - not specify version for protoc-gen-go, it will use the version from go.mod - do not copy over the protoc binary. Call it directly from unzipped path. So it can find the includes automatically, and we don't need the -I or --go_opts=Mxxx flags. - use paths=source_relative to generate go files directly into destination, avoid the copy. - simplify the sed command used to extract "csi.proto" file.
1 parent 0b3725b commit c800de2

File tree

3 files changed

+25
-65
lines changed

3 files changed

+25
-65
lines changed

.github/workflows/build.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,6 @@ jobs:
2222

2323
- name: Build Test
2424
run: |
25+
touch spec.md # Ensure its timestamp is newer than any generated files
2526
make
27+
git diff --exit-code || (echo "Generated files are out of date. Please run 'make' and commit the changes." && exit 1)

Makefile

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,39 +6,20 @@ CSI_PROTO := csi.proto
66
CSI_A := csi.a
77
CSI_PKG := lib/go/csi
88

9-
# This is the target for building the temporary CSI protobuf file.
10-
#
11-
# The temporary file is not versioned, and thus will always be
12-
# built on GitHub Actions.
13-
$(CSI_PROTO).tmp: $(CSI_SPEC) Makefile
14-
echo "// Code generated by make; DO NOT EDIT." > "$@"
15-
cat $< | sed -n -e '/```protobuf$$/,/^```$$/ p' | sed '/^```/d' >> "$@"
16-
179
# This is the target for building the CSI protobuf file.
18-
#
19-
# This target depends on its temp file, which is not versioned.
20-
# Therefore when built on GitHub Actions the temp file will always
21-
# be built and trigger this target. On GitHub Actions the temp file
22-
# is compared with the real file, and if they differ the build
23-
# will fail.
24-
#
25-
# Locally the temp file is simply copied over the real file.
26-
$(CSI_PROTO): $(CSI_PROTO).tmp
27-
ifeq (true,$(GITHUB_ACTIONS))
28-
diff "$@" "$?"
29-
else
30-
diff "$@" "$?" > /dev/null 2>&1 || cp -f "$?" "$@"
31-
endif
10+
$(CSI_PROTO): $(CSI_SPEC) Makefile
11+
echo "// Code generated by make; DO NOT EDIT." > "$@"
12+
sed -n -e '/```protobuf$$/,/^```$$/ {//!p;}' $< >> "$@"
3213

3314
build: check build_cpp build_go
3415

3516
build_cpp:
3617
$(MAKE) -C lib/cxx
3718

38-
csi_go:
19+
lib/go/csi/%.pb.go: $(CSI_PROTO)
3920
$(MAKE) -C lib/go
4021

41-
$(CSI_A): csi_go
22+
$(CSI_A): lib/go/csi/*.go
4223
go mod download
4324
go install ./$(CSI_PKG)
4425
go build -o "$@" ./$(CSI_PKG)
@@ -51,10 +32,10 @@ clean:
5132

5233
clobber: clean
5334
$(MAKE) -C lib/go $@
54-
rm -f $(CSI_PROTO) $(CSI_PROTO).tmp
35+
rm -f $(CSI_PROTO)
5536

5637
# check generated files for violation of standards
5738
check: $(CSI_PROTO)
5839
awk '{ if (length > 72) print NR, $$0 }' $? | diff - /dev/null
5940

60-
.PHONY: clean clobber check csi_go build_go build_cpp
41+
.PHONY: clean clobber check build_go build_cpp

lib/go/Makefile

Lines changed: 16 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -36,22 +36,24 @@ else ifeq (arm64,$(PROTOC_ARCH))
3636
PROTOC_ARCH := aarch_64
3737
endif
3838

39-
PROTOC := ./protoc
4039
PROTOC_ZIP := protoc-$(PROTOC_VER)-$(PROTOC_OS)-$(PROTOC_ARCH).zip
4140
PROTOC_URL := https://github.com/protocolbuffers/protobuf/releases/download/v$(PROTOC_VER)/$(PROTOC_ZIP)
4241
PROTOC_TMP_DIR := .protoc
43-
PROTOC_TMP_BIN := $(PROTOC_TMP_DIR)/bin/protoc
42+
PROTOC := $(PROTOC_TMP_DIR)/bin/protoc
43+
44+
$(GOPATH)/bin/protoc-gen-go: ../../go.mod
45+
go install google.golang.org/protobuf/cmd/protoc-gen-go
46+
$(GOPATH)/bin/protoc-gen-go-grpc:
47+
go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.3.0
4448

4549
$(PROTOC):
46-
-go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.32.0 && \
47-
go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.3.0 && \
48-
mkdir -p "$(PROTOC_TMP_DIR)" && \
50+
-mkdir -p "$(PROTOC_TMP_DIR)" && \
4951
curl -L $(PROTOC_URL) -o "$(PROTOC_TMP_DIR)/$(PROTOC_ZIP)" && \
5052
unzip "$(PROTOC_TMP_DIR)/$(PROTOC_ZIP)" -d "$(PROTOC_TMP_DIR)" && \
51-
chmod 0755 "$(PROTOC_TMP_BIN)" && \
52-
cp -f "$(PROTOC_TMP_BIN)" "$@"
53+
chmod 0755 "$@"
5354
stat "$@" > /dev/null 2>&1
5455

56+
PROTOC_ALL := $(GOPATH)/bin/protoc-gen-go $(GOPATH)/bin/protoc-gen-go-grpc $(PROTOC)
5557

5658
########################################################################
5759
## PATH ##
@@ -66,49 +68,24 @@ export PATH := $(GOPATH)/bin:$(PATH)
6668
## BUILD ##
6769
########################################################################
6870
CSI_PROTO := ../../csi.proto
69-
CSI_PKG_ROOT := github.com/container-storage-interface/spec
70-
CSI_PKG_SUB := $(shell cat $(CSI_PROTO) | sed -nE -e 's/^package.([^;]*).v[0-9]+;$$/\1/p'|tr '.' '/')
71-
CSI_BUILD := $(CSI_PKG_SUB)/.build
71+
CSI_PKG_SUB := csi
7272
CSI_GO := $(CSI_PKG_SUB)/csi.pb.go
7373
CSI_GRPC := $(CSI_PKG_SUB)/csi_grpc.pb.go
74-
CSI_GRPC_TMP := $(CSI_BUILD)/$(CSI_PKG_ROOT)/lib/go/$(CSI_PKG_SUB)/csi_grpc.pb.go
75-
CSI_GO_TMP := $(CSI_BUILD)/$(CSI_PKG_ROOT)/lib/go/$(CSI_PKG_SUB)/csi.pb.go
7674

77-
# This recipe generates the go language bindings to a temp area.
78-
$(CSI_GO_TMP) $(CSI_GRPC_TMP): INCLUDE := -I../.. -I$(PROTOC_TMP_DIR)/include
79-
$(CSI_GO_TMP) $(CSI_GRPC_TMP): $(CSI_PROTO) | $(PROTOC)
75+
# This recipe generates the go language bindings
76+
$(CSI_GO) $(CSI_GRPC): $(CSI_PROTO) $(PROTOC_ALL)
8077
@mkdir -p "$(@D)"
81-
$(PROTOC) $(INCLUDE) --go-grpc_out=$(CSI_BUILD) --go_out=$(CSI_BUILD) \
82-
--go_opt=Mgoogle/protobuf/descriptor.proto=google.golang.org/protobuf/types/descriptorpb \
83-
--go_opt=Mgoogle/protobuf/wrappers.proto=google.golang.org/protobuf/types/known/wrapperspb \
78+
$(PROTOC) -I../.. --go-grpc_out=$(CSI_PKG_SUB) --go_out=$(CSI_PKG_SUB) \
79+
--go_opt=paths=source_relative --go-grpc_opt=paths=source_relative \
8480
"$(<F)"
8581

86-
# The temp language bindings are compared to the ones that are
87-
# versioned. If they are different then it means the language
88-
# bindings were not updated prior to being committed.
89-
$(CSI_GO): $(CSI_GO_TMP)
90-
ifeq (true,$(GITHUB_ACTIONS))
91-
diff "$@" "$?"
92-
else
93-
@mkdir -p "$(@D)"
94-
diff "$@" "$?" > /dev/null 2>&1 || cp -f "$?" "$@"
95-
endif
96-
$(CSI_GRPC): $(CSI_GRPC_TMP)
97-
ifeq (true,$(GITHUB_ACTIONS))
98-
diff "$@" "$?"
99-
else
100-
@mkdir -p "$(@D)"
101-
diff "$@" "$?" > /dev/null 2>&1 || cp -f "$?" "$@"
102-
endif
103-
104-
10582
build: $(CSI_GO) $(CSI_GRPC)
10683

10784
clean:
10885
go clean -i ./...
109-
rm -rf "$(CSI_GO)" "$(CSI_GRPC)" "$(CSI_BUILD)"
86+
rm -rf "$(CSI_PKG_SUB)"
11087

11188
clobber: clean
112-
rm -fr "$(PROTOC)" "$(CSI_PKG_SUB)"
89+
rm -fr "$(PROTOC_TMP_DIR)"
11390

11491
.PHONY: clean clobber

0 commit comments

Comments
 (0)