android compilation and link flags

[xdegaye]

BPO	26851
Nosy	@Yhg1s, @doko42, @vstinner, @xdegaye, @vadmium, @moreati, @yan12125
PRs	[Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
Files	build-flags.patch build-flags_2.patch build-flags_3.patch build-flags_3.patch: uploading twice the same file build-flags_4.patch build-flags_5.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/xdegaye'
closed_at = <Date 2017-01-04.20:58:21.080>
created_at = <Date 2016-04-26.12:20:11.511>
labels = ['type-feature', '3.7', 'build']
title = 'android compilation and link flags'
updated_at = <Date 2017-03-31.16:36:19.595>
user = 'https://github.com/xdegaye'

bugs.python.org fields:

activity = <Date 2017-03-31.16:36:19.595>
actor = 'dstufft'
assignee = 'xdegaye'
closed = True
closed_date = <Date 2017-01-04.20:58:21.080>
closer = 'xdegaye'
components = ['Cross-Build']
creation = <Date 2016-04-26.12:20:11.511>
creator = 'xdegaye'
dependencies = []
files = ['42601', '43868', '43875', '43878', '43893', '45101']
hgrepos = []
issue_num = 26851
keywords = ['patch']
message_count = 19.0
messages = ['264266', '271169', '271193', '271246', '271275', '271282', '271292', '271353', '271518', '271530', '271543', '271552', '278708', '278712', '284667', '284668', '284739', '284745', '284752']
nosy_count = 8.0
nosy_names = ['twouters', 'doko', 'vstinner', 'xdegaye', 'python-dev', 'martin.panter', 'Alex.Willmer', 'yan12125']
pr_nums = ['552']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue26851'
versions = ['Python 3.6', 'Python 3.7']

[xdegaye]

The attached patch:

• Sets the recommended android compilation flags, see: http://developer.android.com/ndk/guides/standalone_toolchain.html#abi. Note that the android toolchains already set the -fpic flag, as shown with:

  arm-linux-androideabi-gcc -v -S main.c 2>&1 | grep main.c

• Sets the Position independent executables (PIE) flag which is mandatory starting at API level 21 and supported starting with API level 16.

[xdegaye]

The previous patch was using awkwardly, a 'host_cpu' configure argument to specify the arm ABI and missed setting LDFLAGS for the armv7 ABI.
This patch instead uses the __ARM_ARCH macro defined by each compiler of the Android toolchains:

    echo | ./clang -target armv7-none-linux-androideabi -dM -E - | grep ARM_ARCH
      #define __ARM_ARCH 7
      #define __ARM_ARCH_7A__ 1
    echo | ./clang -target armv5te-none-linux-androideabi -dM -E - | grep ARM_ARCH
      #define __ARM_ARCH 5
      #define __ARM_ARCH_5TE__ 1
    echo | ./arm-linux-androideabi-gcc -march=armv7-a -dM -E - | grep ARM_ARCH
      #define __ARM_ARCH 7
      #define __ARM_ARCH_7A__ 1
    echo | ./arm-linux-androideabi-gcc -dM -E - | grep ARM_ARCH
      #define __ARM_ARCH_5TE__ 1
      #define __ARM_ARCH 5

Python built with clang for the armv5te target crashes as reported in bpo-27606.

[vadmium]

I left some suggestions and questions on the code review.

[xdegaye]

Thanks for the review and the suggestions Martin :)
New patch.

[xdegaye]

The first 'build-flags_3.patch' file that was uploaded did not get a review button because ssh://hg@hg.python.org/cpython was down at that time.

The second upload has been successfull !

[vstinner]

Note: build-flags_3.patch includes configure.ac and configure. Usually, we suggest to not included generated files (configure) in patches, but push them when the patch is ready.

[xdegaye]

Thanks for the suggestion, I won't include them anymore in patches then.

[xdegaye]

New patch. The sed commands used to evaluate ANDROID_API_LEVEL and _arm_arch do not need to discard comment lines and empty lines in the output of the preprocessor.

[vadmium]

All the bits that I understand look okay now. :)

I am still curious what configures the preprocessor to set __ARM_ARCH to 7 (I guess the clang -target argument?), and why we can’t set LDFLAGS at the same time or place. Is it just more convenient this way?

[xdegaye]

I am still curious what configures the preprocessor to set __ARM_ARCH to 7 (I guess the clang -target argument?)
Yes, the -target clang argument or the -march gcc argument.

and why we can’t set LDFLAGS at the same time or place. Is it just more convenient this way?
I don't understand the question. LDFLAGS is set at the time we know that __ARM_ARCH is 7, just after the preprocessing is done. Would you set it elsewhere ?
LDFLAGS is set in configure.ac with always this same idiom: LDFLAGS="$LDFLAGS new_options..." and this setting is done in what seems to be random places in configure.ac.
CCSHARED is set in one case statement.

[vadmium]

Where is the code that sets the clang -target argument, or gcc -march? Is it hidden away somewhere in the autoconf code? Do you manually set it with ‘./configure CFLAGS="-target . . ." ’?

[xdegaye]

Yes, the packager must use appropriately either CFLAGS CPPFLAGS [1] and LDFLAGS, or CC. I am using:
CC="clang --sysroot=$(SYSROOT) -target $(TARGET) -gcc-toolchain $(GCC_TOOLCHAIN)".

[1] See bpo-27453, for the remaining problem upon using CPPFLAGS.