-
Notifications
You must be signed in to change notification settings - Fork 99
Adding build flags to inject the version and commit #173
Changes from 3 commits
dbdc55a
b477ac2
d68bac8
7d656bf
bb92a49
882ad02
a88f1fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,14 @@ RC_MUTABLE_IMAGE_NAME = $(DOCKER_REPO)$(BASE_IMAGE_NAME):canary | |
REL_IMAGE_NAME = $(DOCKER_REPO)$(BASE_IMAGE_NAME):$(REL_VERSION) | ||
REL_MUTABLE_IMAGE_NAME = $(DOCKER_REPO)$(BASE_IMAGE_NAME):latest | ||
|
||
ifeq ($(REL_VERSION),) | ||
BROKER_VERSION="devel" | ||
else | ||
BROKER_VERSION=$(REL_VERSION) | ||
endif | ||
|
||
LDFLAGS = -w -X main.commit=$(COMMIT) -X main.commit=$(GIT_VERSION) -X main.version=$(BROKER_VERSION) | ||
|
||
# Checks for the existence of a docker client and prints a nice error message | ||
# if it isn't present | ||
.PHONY: check-docker | ||
|
@@ -167,7 +175,7 @@ stop-test-redis: check-docker-compose | |
.PHONY: build | ||
build: check-docker-compose | ||
docker-compose run --rm dev \ | ||
go build -o ${BINARY_DIR}/${BINARY_NAME} ./cmd/broker | ||
go build -o ${BINARY_DIR}/${BINARY_NAME} -ldflags '$(LDFLAGS)' ./cmd/broker | ||
|
||
# (Re)Build the Docker image for the osba and run it | ||
.PHONY: run | ||
|
@@ -229,16 +237,19 @@ CLI_BINARY_NAME := broker-cli | |
build-mac-broker-cli: check-docker-compose | ||
docker-compose run --rm -e GOOS=darwin -e GOARCH=amd64 dev \ | ||
go build -o ${CONTRIB_BINARY_DIR}/${CLI_BINARY_NAME} \ | ||
-ldflags '$(LDFLAGS)' \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure we need to set this on the CLI builds. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still think we don't need these flags here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even though the CLI is not supposed to be a first-class citizen, it couldn't hurt, right? I could add a version command to the CLI also, if you're ok with keeping the version in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with it. Should this PR include the necessary modification to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Repeating what @krancour and I talked about offline: I am going to remove this and submit a low-priority issue to:
|
||
./contrib/cmd/cli | ||
|
||
.PHONY: build-linux-broker-cli | ||
build-linux-broker-cli: check-docker-compose | ||
docker-compose run --rm -e GOOS=linux -e GOARCH=amd64 dev \ | ||
go build -o ${CONTRIB_BINARY_DIR}/${CLI_BINARY_NAME} \ | ||
-ldflags '$(LDFLAGS)' \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't need these flags here. |
||
./contrib/cmd/cli | ||
|
||
.PHONY: build-win-broker-cli | ||
build-win-broker-cli: check-docker-compose | ||
docker-compose run --rm -e GOOS=windows -e GOARCH=amd64 dev \ | ||
go build -o ${CONTRIB_BINARY_DIR}/${CLI_BINARY_NAME} \ | ||
-ldflags '$(LDFLAGS)' \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't need these flags here. |
||
./contrib/cmd/cli |
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.
-X main.commit=$(COMMIT)
doesn't look right... it seems to write the same property as-X main.commit=$(GIT_VERSION)
, and appears to use an undefined value. Guessing this was a copy/paste error.