-
Notifications
You must be signed in to change notification settings - Fork 823
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
Support builds on macOS #90
Conversation
build/Makefile
Outdated
@@ -55,7 +55,13 @@ common_mounts = -v $(build_path)/.config/gcloud:/root/.config/gcloud \ | |||
|
|||
# Use a hash of the Dockerfile for the tag, so when the Dockerfile changes, | |||
# it automatically rebuilds | |||
build_version := $(shell sha256sum $(build_path)/build-image/Dockerfile | head -c 10) | |||
build_version := $(shell \ |
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'm wondering if we should just move this section below the os-include section - and then can just specify the os specific build_version
in each of the includes, rather than writing complicated shell logic?
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.
That's a nice idea. Since build_version
will be used only for development, I suppose it doesn't hurt to define it per os. Does windows have any trouble with sha256sum
?
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.
On windows we push people towards WSL, so we have a nicer cross-platform dev/build cycle - and since it's backed by Ubuntu, sha256sum
is already there! 😄
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.
Perfect. I'll update the PR shortly.
The BSD version of sed does not support multiple `-e` flags and `sha256sum` is not installed by default. This commit makes minor changes to ensure a macOS user may build agones without any additional effort.
489e61b
to
d62fba6
Compare
@markmandel This should be good now that it includes the LF line ending work. |
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.
LGTM
In addition to renaming occurrences of OS X to macOS, this commit makes some minor changes to the Makefile to ensure macOS users can build Agones.
Fixes #46.