-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Add dockerfile for ppc64le and related changes #638
Conversation
Makefile
Outdated
@@ -77,8 +78,14 @@ tarball: $(PROMU) | |||
@$(PROMU) tarball --prefix $(PREFIX) $(BIN_DIR) | |||
|
|||
docker: | |||
ifeq ($(MACH), ppc64le) | |||
@echo ">> building docker image for ppc64le" | |||
$(eval FILE_SUFFIX=.ppc64le) |
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'd call it DOCKER_FILE
and pass the full file. Could also happen setting as variable at the top.
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.
Agreed, pushing a new commit.
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.
Personally I'd keep all variable definitions at the top of the file and avoid eval
. I guess it should be possible to set up to do the following:
- if DOCKERFILE is set already, don't do anything
- if DOCKERFILE isn't set use Dockerfile
- if DOCKERFILE isn't set and arch is ppc64le, use Dockerfile.ppc64le.
@matthiasr surely knows what I want ;-) It's not important though, we can change that later.
Makefile
Outdated
@@ -77,8 +79,11 @@ tarball: $(PROMU) | |||
@$(PROMU) tarball --prefix $(PREFIX) $(BIN_DIR) | |||
|
|||
docker: | |||
ifeq ($(MACH), ppc64le) | |||
$(eval DOCKERFILE=Dockerfile.ppc64le) | |||
endif | |||
@echo ">> building docker image" |
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.
Let's include the dockerfile we're using in this message. Something like >> building docker image from $(DOCKERFILE)
.
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.
Included the dockerfile in the build message.
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.
Thanks!
* Add dockerfile for ppc64le and related changes * Pass the fill file as DOCKEFILE * Add the dockerfile name to build msg
A small change in the makefile determining the right architecture and a new Dockerfile.ppc64le where the base image is different (ppc64le/busybox:glibc).