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

BZ-4216 Android build fails when not cross-compiled on Linux #279

Merged
merged 2 commits into from
Sep 12, 2016

Conversation

dlifshitz-maca
Copy link
Contributor

[Changed]
platform_android.GNU

  • replace the platform_linux_common.GNU include with its contents
    except the erroneous call to getconf

[Tests]
Test 1:

  1. Compile ACE for Android using OSX or Windows
  2. Verify build is successful

[Changed]
platform_android.GNU
- replace the platform_linux_common.GNU include with its contents
  except the erroneous call to getconf

[Tests]
Test 1:
1) Compile ACE for Android using OSX or Windows
2) Verify build is successful
@@ -9,7 +9,123 @@ no_hidden_visibility ?= 1
# as of NDK r6 inlining is required
inline ?= 1

include $(ACE_ROOT)/include/makeinclude/platform_linux_common.GNU
# start of: include most of platform_linux_common.GNU
Copy link
Member

Choose a reason for hiding this comment

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

No need for this comment to my idea, we have git history to know where changes are coming from

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. I added this for my project but wasn't sure if it was needed in the master repo. I'll remove it in my next update when this round of the review is complete.

@jwillemsen
Copy link
Member

Probably more contents taken from the original file doesn't apply to android, but can imagine it is hard to sort that out

@dlifshitz-maca
Copy link
Contributor Author

Agreed that we most likely don't need a lot of the content. I decided to leave it so I don't accidentally break something. I'm not an expert on compiler flags. Please let me know when you have finished the review and I will upload my latest changes.

@jwillemsen
Copy link
Member

I will not be able to review further at this moment, please upload your latest changes, maybe someone else can review further

@jwillemsen
Copy link
Member

jwillemsen commented Sep 9, 2016

@dlifshitz-maca can you commit and push your improvements?

@dlifshitz-maca
Copy link
Contributor Author

@jwillemsen Yes, I will put it on my list for next week. Thanks for the reminder.

[Changed]
platform_android.GNU
- removed extra comment and include
@jwillemsen jwillemsen merged commit a49ca87 into DOCGroup:master Sep 12, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants