Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove duplicate dist directive #2887

Merged
merged 1 commit into from
Sep 5, 2022
Merged

Remove duplicate dist directive #2887

merged 1 commit into from
Sep 5, 2022

Conversation

treid314
Copy link
Contributor

@treid314 treid314 commented Sep 2, 2022

What this PR does

Fixes an issue where we had duplicate dist directives in our makefile which caused an issue while building release packages. This cleans up the hacked makefile I was running earlier and fixes the issue going forward.

Comment on lines -598 to -600
if [ "$$os" = "darwin" ] && [ "$$arch" = "386" ]; then \
continue; \
fi; \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excluding darwin/386 is not in the other one

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Full diff:

***************
*** 2,14 ****
  	rm -fr ./dist
  	mkdir -p ./dist
  	# Build binaries for various architectures and operating systems. Only
! 	# mimirtool supports Windows for now.
  	for os in linux darwin windows; do \
! 		for arch in amd64 arm64; do \
  			suffix="" ; \
  			if [ "$$os" = "windows" ]; then \
  				suffix=".exe" ; \
  			fi; \
  			echo "Building mimirtool for $$os/$$arch"; \
  			GOOS=$$os GOARCH=$$arch CGO_ENABLED=0 go build $(GO_FLAGS) -o ./dist/mimirtool-$$os-$$arch$$suffix ./cmd/mimirtool; \
  			sha256sum ./dist/mimirtool-$$os-$$arch$$suffix | cut -d ' ' -f 1 > ./dist/mimirtool-$$os-$$arch$$suffix-sha-256; \
--- 2,18 ----
  	rm -fr ./dist
  	mkdir -p ./dist
  	# Build binaries for various architectures and operating systems. Only
! 	# mimirtool supports Windows for now. Also darwin/386 is not a valid
! 	# architecture.
  	for os in linux darwin windows; do \
! 		for arch in 386 amd64 arm64; do \
  			suffix="" ; \
  			if [ "$$os" = "windows" ]; then \
  				suffix=".exe" ; \
  			fi; \
+ 			if [ "$$os" = "darwin" ] && [ "$$arch" = "386" ]; then \
+ 				continue; \
+ 			fi; \
  			echo "Building mimirtool for $$os/$$arch"; \
  			GOOS=$$os GOARCH=$$arch CGO_ENABLED=0 go build $(GO_FLAGS) -o ./dist/mimirtool-$$os-$$arch$$suffix ./cmd/mimirtool; \
  			sha256sum ./dist/mimirtool-$$os-$$arch$$suffix | cut -d ' ' -f 1 > ./dist/mimirtool-$$os-$$arch$$suffix-sha-256; \
***************
*** 21,32 ****
  			echo "Building query-tee for $$os/$$arch"; \
  			GOOS=$$os GOARCH=$$arch CGO_ENABLED=0 go build $(GO_FLAGS) -o ./dist/query-tee-$$os-$$arch$$suffix ./cmd/query-tee; \
  			sha256sum ./dist/query-tee-$$os-$$arch$$suffix | cut -d ' ' -f 1 > ./dist/query-tee-$$os-$$arch$$suffix-sha-256; \
- 			echo "Building metaconvert for $$os/$$arch"; \
- 			GOOS=$$os GOARCH=$$arch CGO_ENABLED=0 go build $(GO_FLAGS) -o ./dist/metaconvert-$$os-$$arch$$suffix ./cmd/metaconvert; \
- 			sha256sum ./dist/metaconvert-$$os-$$arch$$suffix | cut -d ' ' -f 1 > ./dist/metaconvert-$$os-$$arch$$suffix-sha-256; \
- 			echo "Building mimir-continuous-test for $$os/$$arch"; \
- 			GOOS=$$os GOARCH=$$arch CGO_ENABLED=0 go build $(GO_FLAGS) -o ./dist/mimir-continuous-test-$$os-$$arch$$suffix ./cmd/mimir-continuous-test; \
- 			sha256sum ./dist/mimir-continuous-test-$$os-$$arch$$suffix | cut -d ' ' -f 1 > ./dist/mimir-continuous-test-$$os-$$arch$$suffix-sha-256; \
  			done; \
  		done; \
  		touch $@
--- 25,30 ----

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't exclude it because we don't do a 386 build there wasn't a 386 build last release as well.

@treid314 treid314 merged this pull request into release-2.3 Sep 5, 2022
@treid314 treid314 deleted the 2.3-makefile-fix branch September 5, 2022 14:52
krajorama pushed a commit that referenced this pull request Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants