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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

MacOSX support #1419

Merged
merged 3 commits into from Jul 22, 2017
Merged

MacOSX support #1419

merged 3 commits into from Jul 22, 2017

Conversation

lamont-granquist
Copy link
Contributor

WIP WIP WIP WIP WIP

馃毀 DO NOT MERGE 馃毀

some of the patches to the Makefile need to be made conditional or they'll most likely break Linux builds as well.

@pleroy
Copy link
Member

pleroy commented May 31, 2017

Can one of the admins verify this patch?

@@ -10,7 +11,7 @@ BASE_FLAGS="-fPIC -O3 -g"

PLATFORM=$(uname -s)
if [ "$PLATFORM" == "Darwin" ]; then
C_FLAGS="$BASE_FLAGS -mmacosx-version-min=10.7 -arch i386"
C_FLAGS="$BASE_FLAGS -mmacosx-version-min=10.7 -arch x86_64"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the only line in this file that is strictly necessary to fix for MacOSX -- all the rest of the diff in install_deps.sh here i just found more useful, but it could all be dropped.

Copy link
Member

Choose a reason for hiding this comment

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

The other changes look good to me too.

PLATFORM=$(uname -s)
if [ "$PLATFORM" == "Darwin" ]; then
echo -e "$VERSION_TEMPLATE" |
sed "s/%%DATE%%/`date -j -u -f \"%Y-%m-%d %H:%M:%S %z\" \"$(git log -1 --format=%cd --date=iso)\"`/" |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

mac date doesn't have nice iso format flags, so it has to get done manually here

Copy link
Member

Choose a reason for hiding this comment

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

The format seems inconsistent with the the one we are using, which is really ISO 8601 extended format, that is %Y-%m-%dT%H:%M:%SZ; note the T, and note the timezone written simply as Z (with no space) since we give UTC.
Example in our Catalan build:

char const BuildDate[] = "2017-05-26T12:44:02Z";

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So that string is what parses the git log output and its right because --date=iso gives that. The problem is that it needs the proper format string for output, which I appear to have dropped

@eggrobin
Copy link
Member

eggrobin commented Jun 2, 2017

Looks good, as you said the Makefile changes need to be made conditional.

@lamont-granquist
Copy link
Contributor Author

char const BuildDate[] = "2017-05-28T16:27:13Z";

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>
accidentally dropped the output formatting

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>
soothes dry damaged and brittle makefile code.

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>
@eggrobin
Copy link
Member

Tests pass on linux when merging this, so LGTM.

@eggrobin eggrobin added the LGTM label Jul 22, 2017
@pleroy pleroy merged commit 1c6cffc into mockingbirdnest:master Jul 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants