Skip to content

Commit 4421b74

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 88a5080 commit 4421b74

File tree

3 files changed

+25
-68
lines changed

3 files changed

+25
-68
lines changed

.github/workflows/build.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,4 @@ jobs:
2424
run: |
2525
touch spec.md # Ensure its timestamp is newer than any generated files
2626
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: 8 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,43 +4,22 @@ CSI_SPEC := spec.md
44
CSI_PROTO := csi.proto
55
## Build go language bindings
66
CSI_A := csi.a
7-
CSI_GO := lib/go/csi/csi.pb.go
87
CSI_PKG := lib/go/csi
98

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

3414
build: check build_cpp build_go
3515

3616
build_cpp:
3717
$(MAKE) -C lib/cxx
3818

39-
# The file exists, but could be out-of-date.
40-
$(CSI_GO): $(CSI_PROTO)
41-
$(MAKE) -C lib/go csi/csi.pb.go
19+
$(CSI_PKG)/%.pb.go: $(CSI_PROTO)
20+
$(MAKE) -C lib/go
4221

43-
$(CSI_A): $(CSI_GO)
22+
$(CSI_A): $(CSI_PKG)/*.go
4423
go mod download
4524
go install ./$(CSI_PKG)
4625
go build -o "$@" ./$(CSI_PKG)
@@ -53,10 +32,10 @@ clean:
5332

5433
clobber: clean
5534
$(MAKE) -C lib/go $@
56-
rm -f $(CSI_PROTO) $(CSI_PROTO).tmp
35+
rm -f $(CSI_PROTO)
5736

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

62-
.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
@@ -41,22 +41,24 @@ else ifeq (arm64,$(PROTOC_ARCH))
4141
PROTOC_ARCH := aarch_64
4242
endif
4343

44-
PROTOC := ./protoc
4544
PROTOC_ZIP := protoc-$(PROTOC_VER)-$(PROTOC_OS)-$(PROTOC_ARCH).zip
4645
PROTOC_URL := https://github.com/protocolbuffers/protobuf/releases/download/v$(PROTOC_VER)/$(PROTOC_ZIP)
4746
PROTOC_TMP_DIR := .protoc
48-
PROTOC_TMP_BIN := $(PROTOC_TMP_DIR)/bin/protoc
47+
PROTOC := $(PROTOC_TMP_DIR)/bin/protoc
48+
49+
$(GOBIN)/protoc-gen-go: ../../go.mod
50+
go install google.golang.org/protobuf/cmd/protoc-gen-go
51+
$(GOBIN)/protoc-gen-go-grpc:
52+
go install google.golang.org/grpc/cmd/[email protected]
4953

5054
$(PROTOC):
51-
-go install google.golang.org/protobuf/cmd/[email protected] && \
52-
go install google.golang.org/grpc/cmd/[email protected] && \
53-
mkdir -p "$(PROTOC_TMP_DIR)" && \
55+
-mkdir -p "$(PROTOC_TMP_DIR)" && \
5456
curl -L $(PROTOC_URL) -o "$(PROTOC_TMP_DIR)/$(PROTOC_ZIP)" && \
5557
unzip "$(PROTOC_TMP_DIR)/$(PROTOC_ZIP)" -d "$(PROTOC_TMP_DIR)" && \
56-
chmod 0755 "$(PROTOC_TMP_BIN)" && \
57-
cp -f "$(PROTOC_TMP_BIN)" "$@"
58+
chmod 0755 "$@"
5859
stat "$@" > /dev/null 2>&1
5960

61+
PROTOC_ALL := $(GOBIN)/protoc-gen-go $(GOBIN)/protoc-gen-go-grpc $(PROTOC)
6062

6163
########################################################################
6264
## PATH ##
@@ -71,49 +73,24 @@ export PATH := $(GOBIN):$(PATH)
7173
## BUILD ##
7274
########################################################################
7375
CSI_PROTO := ../../csi.proto
74-
CSI_PKG_ROOT := github.com/container-storage-interface/spec
75-
CSI_PKG_SUB := $(shell cat $(CSI_PROTO) | sed -nE -e 's/^package.([^;]*).v[0-9]+;$$/\1/p'|tr '.' '/')
76-
CSI_BUILD := $(CSI_PKG_SUB)/.build
76+
CSI_PKG_SUB := csi
7777
CSI_GO := $(CSI_PKG_SUB)/csi.pb.go
7878
CSI_GRPC := $(CSI_PKG_SUB)/csi_grpc.pb.go
79-
CSI_GRPC_TMP := $(CSI_BUILD)/$(CSI_PKG_ROOT)/lib/go/$(CSI_PKG_SUB)/csi_grpc.pb.go
80-
CSI_GO_TMP := $(CSI_BUILD)/$(CSI_PKG_ROOT)/lib/go/$(CSI_PKG_SUB)/csi.pb.go
8179

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

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

11289
clean:
11390
go clean -i ./...
114-
rm -rf "$(CSI_GO)" "$(CSI_GRPC)" "$(CSI_BUILD)"
91+
rm -rf "$(CSI_PKG_SUB)"
11592

11693
clobber: clean
117-
rm -fr "$(PROTOC)" "$(CSI_PKG_SUB)"
94+
rm -fr "$(PROTOC_TMP_DIR)"
11895

11996
.PHONY: clean clobber

0 commit comments

Comments
 (0)