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

Make python flags relative for Android.mk (bootstraps/common...) #1615

Closed
wants to merge 1 commit into from

Conversation

opacam
Copy link
Member

@opacam opacam commented Jan 27, 2019

For bootstraps/common/build/jni/application/src/Android.mk

To fix possible linkage problems in some OS and because it's the recommended way.

See also: https://stackoverflow.com/questions/19720421/android-mk-relative-or-absolute-paths

…jni/application/src

To fix possible linkage problems in some OS and because it's the recommended way.

See also: https://stackoverflow.com/questions/19720421/android-mk-relative-or-absolute-paths
@inclement
Copy link
Member

I think I misunderstood the diff when you mentioned this before - what is this actually fixing? It seems counterintuitive to me that switching to relative paths should be better, because they are difficult to understand by eye and more vulnerable to falling out of date.

I couldn't find where this is recommended in the NDK docs, as claimed in the linked stackoverflow post.

@inclement
Copy link
Member

inclement commented Jan 27, 2019

Ah, found it at https://developer.android.com/ndk/guides/android_mk

We recommend avoiding absolute file paths; relative paths make your Android.mk file more portable.

This isn't important for python-for-android because we are autogenerating everything anyway, so we have different concerns. I think the file paths should remain absolute.

The only portability gain of using the relative paths is really that the whole build folder could be moved elsewhere, but that seems only a very minor benefit considering there's no workflow that requires it.

@opacam
Copy link
Member Author

opacam commented Jan 28, 2019

@inclement, you are right

but I was thinking that we never finished the conversation that we started in the core-update pr and I applied some changes in there that we never finished to discuss and right now those are already in master branch...
so maybe we should revert the changes I made in there which involves those MK_PYTHON... variables..so I just created pr for that in here:

#1619

I close this ;)

@opacam opacam closed this Jan 28, 2019
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