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

Support builds on macOS #90

Merged
merged 1 commit into from
Feb 13, 2018
Merged

Support builds on macOS #90

merged 1 commit into from
Feb 13, 2018

Conversation

enocom
Copy link
Contributor

@enocom enocom commented Feb 12, 2018

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.

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 \
Copy link
Member

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?

Copy link
Contributor Author

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?

Copy link
Member

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! 😄

Copy link
Contributor Author

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.
@enocom
Copy link
Contributor Author

enocom commented Feb 12, 2018

@markmandel This should be good now that it includes the LF line ending work.

Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

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

LGTM

@markmandel markmandel merged commit e56472a into master Feb 13, 2018
@markmandel markmandel deleted the macOS-testing branch February 13, 2018 00:25
@markmandel markmandel added this to the 0.1 milestone May 19, 2018
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