This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2016年07月02日 14:52 by xdegaye, last changed 2022年04月11日 14:58 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| android_api.patch | xdegaye, 2016年07月02日 14:57 | review | ||
| android_api_2.patch | xdegaye, 2016年07月04日 20:02 | |||
| android_api_3.patch | xdegaye, 2016年07月06日 10:18 | |||
| issue27442-3.patch | skrah, 2016年07月08日 08:19 | regenerated android_api_3.patch | review | |
| Messages (29) | |||
|---|---|---|---|
| msg269716 - (view) | Author: Xavier de Gaye (xdegaye) * (Python triager) | Date: 2016年07月02日 14:52 | |
Expose the Android API level that python was built against, in sys.implementation as _android_api. Purposes: * Identify the platform as Android. * Allow detecting a mismatch with the runtime sdk version returned by platform.android_ver() (issue 26855), for example when the runtime sdk is lower than the built API level. |
|||
| msg269717 - (view) | Author: Xavier de Gaye (xdegaye) * (Python triager) | Date: 2016年07月02日 14:57 | |
Patch added. |
|||
| msg269719 - (view) | Author: (yan12125) * | Date: 2016年07月02日 15:11 | |
I don't think sys.implementation is a good place to contain the information of the underlying platform. By Doc/sys.rst: An object containing information about the implementation of the currently running Python interpreter. |
|||
| msg269720 - (view) | Author: (yan12125) * | Date: 2016年07月02日 15:12 | |
Sorry, it's Doc/library/sys.rst |
|||
| msg269721 - (view) | Author: Xavier de Gaye (xdegaye) * (Python triager) | Date: 2016年07月02日 15:28 | |
> I don't think sys.implementation is a good place to contain the information of the underlying platform. Quite the opposite, the ndk API level gives an information about the implementation of the currently running Python interpreter saying that this Python has been built against this version of Android libc identified by this API level. |
|||
| msg269724 - (view) | Author: (yan12125) * | Date: 2016年07月02日 17:39 | |
OK I see the rationale. |
|||
| msg269726 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2016年07月02日 18:21 | |
I woupd prefer to have a public attribute. |
|||
| msg269748 - (view) | Author: Ned Deily (ned.deily) * (Python committer) | Date: 2016年07月03日 00:09 | |
Typically, for other sorts of build configuration data, we have relied on extracting that from the ./configure-produced Makefile and making it available via sysconfig.get_config_var(). I think we should be cautious about bloating sys.implementation with platform-specific data unless there is an overriding need for it, for example, if it is needed during interpreter initialization before sysconfig can be initialized. If not, I'd look at adding the needed values as configuration variables in configure.ac. |
|||
| msg269756 - (view) | Author: Xavier de Gaye (xdegaye) * (Python triager) | Date: 2016年07月03日 08:40 | |
@Ned The information that the interpreter is an Android built, is needed in issue 16353 [1][2]. In msg266135, the corresponding patch adds the 'is_android' attribute to the already bloated sys module. I had thought then about using sysconfig.get_config_var() as you are suggesting, but this means that the os module would need now to import sysconfig on all linux platforms and build this big dictionary (about 650 entries on linux). So it seems that the choice is between: (1) adding an attribute (is_android or android_api) to the bloated sys module (2) bloating sys.implementation with platform-specific data I think that (2) would be better since _android_api is an information about the implementation, it tells that in these binaries, such and such features are available or not (issue 26857 for example). The drawback is that it would not be documented except in Misc/NEWS [2]. @Victor Do you mean a public attribute of sys.implementation ? Adding a new sys.implementation required attribute is described in pep 421. In that case the attribute name could be 'libc_version' or 'libc'. [1] msg175006 suggested another approach though, but this seems to be abandoned. [2] For completeness, Stefan has submitted a feature request to google in msg266089. |
|||
| msg269757 - (view) | Author: Xavier de Gaye (xdegaye) * (Python triager) | Date: 2016年07月03日 09:37 | |
Well, issue 16353 has been entered upon attempting to fix issue 16255 (see msg173477). So an alternative exists. Issue 16353 could be closed as 'wont't fix'. The list of locations where '/bin/sh' is hard coded in the standard library in msg266084 shows that only the subprocess module and the test suite need to know the location of the system shell. So the subprocess module and issue 27027 could deduct the location of this shell via sysconfig.get_config_var('android_api'). |
|||
| msg269807 - (view) | Author: Xavier de Gaye (xdegaye) * (Python triager) | Date: 2016年07月04日 20:02 | |
A patch to expose the Android API level in sysconfig.get_config_vars(). |
|||
| msg269828 - (view) | Author: Xavier de Gaye (xdegaye) * (Python triager) | Date: 2016年07月05日 15:34 | |
According to issue 27453, do this minor change in the patch: s/$CC -E/$CPP $CPPFLAGS. |
|||
| msg269829 - (view) | Author: (yan12125) * | Date: 2016年07月05日 15:49 | |
Here's an issue - there's already a macro called ANDROID_API defined in libcutils [1] If someone is going to integrate Python into AOSP, redefining macros may cause a problem. [1] https://android.googlesource.com/platform/system/core/+/master/include/cutils/compiler.h#42 |
|||
| msg269832 - (view) | Author: Xavier de Gaye (xdegaye) * (Python triager) | Date: 2016年07月05日 16:03 | |
Integrating Python into AOSP does not make sense. The patch can be changed with s/ANDROID_API/ANDROID_API_LEVEL. |
|||
| msg269834 - (view) | Author: (yan12125) * | Date: 2016年07月05日 16:11 | |
Yep adding Python to Android's build system is a rare case. Just to mention there's already an macro and avoiding possible redefined macros is always good. |
|||
| msg269878 - (view) | Author: Xavier de Gaye (xdegaye) * (Python triager) | Date: 2016年07月06日 10:18 | |
New patch incorporating the substitutions to '$CPP $CPPFLAGS' and ANDROID_API_LEVEL. |
|||
| msg269902 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2016年07月06日 19:19 | |
+ @unittest.skipIf(sysconfig.get_config_var('ANDROID_API_LEVEL') == 0,
+ "not an android platform")
Hum, sysconfig.get_config_var() returns None for unknown variable. Why checking ==0?
@Xavier: Are you generating the patch using "hg diff"? I don't see the base revision in your patch, and so there is no [Review] link on your patch.
|
|||
| msg269937 - (view) | Author: Xavier de Gaye (xdegaye) * (Python triager) | Date: 2016年07月07日 14:43 | |
In sysconfig.parse_config_h(), the variables in pyconfig.h that have a commented-out '#undef' line are set to 0. Fortunately, there is no Android API level 0. Checking '== 0' ensures that autoreconf has been run to add '#undef ANDROID_API_LEVEL' to pyconfig.h.in. If this autoreconf step were to be missed, the test would (correctly) fail on the buildbots that are not Android as get_config_var() would return None then and the test would not be skipped and fail. Most of the tests in the Python test suite do check 'not sysconfig.get_config_var()' instead, except: Lib/test/test_cmath.py|543 col 22| @unittest.skipIf(sysconfig.get_config_var('TANH_PRESERVES_ZERO_SIGN') == 0, "system tanh() function doesn't copy the sign") Lib/test/test_math.py|978 col 22| @unittest.skipIf(sysconfig.get_config_var('TANH_PRESERVES_ZERO_SIGN') == 0, "system tanh() function doesn't copy the sign") |
|||
| msg269938 - (view) | Author: Xavier de Gaye (xdegaye) * (Python triager) | Date: 2016年07月07日 14:48 | |
> Are you generating the patch using "hg diff"? I don't see the base revision in your patch, and so there is no [Review] link on your patch. I am using 'hg diff' with ~/.hgrc set to 'git = on' and this time, naively removed the '--git' from the output of 'hg diff' instead of commenting out 'git = on' in the config file :( (not finding any reference explaining why this setting is wrong for Rietveld). I will change that to have 'git = on' always commented out. The Python Developer’s Guide in section '16.1. Minimal Configuration' recommends setting 'git = on'. Since we are switching to git shortly, it is probably not very useful to enter a new issue to update the Guide explaining that this setting must not be used when producing patches to be reviewed by Rietveld because the base revision is missing in this case. |
|||
| msg269954 - (view) | Author: Berker Peksag (berker.peksag) * (Python committer) | Date: 2016年07月07日 17:38 | |
> I am using 'hg diff' with ~/.hgrc set to 'git = on' and this time, naively removed the '--git' from the output of 'hg diff' instead of commenting out 'git = on' in the config file :( You don't need to do that. I've been using the same setting [1] for 5 years without having a single problem. If you have a fresh clone of https://hg.python.org/cpython/ and using the 'default' branch, you're good. [1] [diff] git = True showfunc = True unified = 8 |
|||
| msg269958 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2016年07月07日 20:24 | |
+ @unittest.skipIf(sysconfig.get_config_var('ANDROID_API_LEVEL') == 0,
+ "not an android platform")
+ def test_is_android(self):
+ # Use an heuristic, the shell on Android is at /system/bin/sh.
+ proc = subprocess.run(['/system/bin/sh', '-c', 'echo OK'],
+ stdout=subprocess.PIPE)
+ self.assertIn(b'OK', proc.stdout)
I dislike such functional test, it might fail and might need maintaince.
I suggest to push the change without the unit test.
|
|||
| msg269962 - (view) | Author: Xavier de Gaye (xdegaye) * (Python triager) | Date: 2016年07月07日 22:24 | |
@Berker This does not work for me. I found an old discussion [1] on the git format problem with Rietveld. The thread diverges rapidly toward another subject, "submitting a branch instead of a patch to the issue tracker" but comes back to the original subject at the end. It seems that "not including the base changeset id in the --git diff is an oversight" of mercurial that has never been fixed. [1] http://grokbase.com/t/python/python-dev/1135q4xxa8/hg-diff |
|||
| msg269963 - (view) | Author: Xavier de Gaye (xdegaye) * (Python triager) | Date: 2016年07月07日 22:26 | |
> I suggest to push the change without the unit test. Agreed. |
|||
| msg269980 - (view) | Author: Stefan Krah (skrah) * (Python committer) | Date: 2016年07月08日 08:23 | |
I did "hg pull -u" before re-generating the diff. Other than that, sometimes newer mercurial versions behave better (I have 2.8.2). |
|||
| msg269981 - (view) | Author: STINNER Victor (vstinner) * (Python committer) | Date: 2016年07月08日 09:39 | |
issue27442-3.patch: be careful, the new item in Misc/NEWS is not at the top of its section. Moreover, I still consider that the unit test is risky and not really needed (just remove it). Good job, there is now a [review] button :-) |
|||
| msg269982 - (view) | Author: Stefan Krah (skrah) * (Python committer) | Date: 2016年07月08日 10:01 | |
Good point about Misc/NEWS: I just regenerated the patch mindlessly to check the "diff --git" situation :) Actually, Xavier, it's often better to leave out the NEWS section in the bug tracker patches and just add it before pushing. Perhaps Rietveld choked on the NEWS file before because it's constantly changing? |
|||
| msg270008 - (view) | Author: Xavier de Gaye (xdegaye) * (Python triager) | Date: 2016年07月08日 20:00 | |
Thanks Berker and Stefan for your help on 'hg diff --git'. Yes Stefan, maybe your patch has a [review] button because Rietveld found that your patch applied cleanly against the head of the default branch at the time you have submitted it, so it could guess the base revision while mines were submitted late, or rather without a 'hg pull -u' right before the submission ? |
|||
| msg270013 - (view) | Author: Stefan Krah (skrah) * (Python committer) | Date: 2016年07月08日 22:51 | |
I guess so. Our Rietveld setup apparently has some heuristics that work best when you're on the default branch and completely synced with the main repo. |
|||
| msg270031 - (view) | Author: Roundup Robot (python-dev) (Python triager) | Date: 2016年07月09日 09:06 | |
New changeset 46567fda0b29 by Xavier de Gaye in branch 'default': Issue #27442: Expose the Android API level in sysconfig.get_config_vars() https://hg.python.org/cpython/rev/46567fda0b29 |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:58:33 | admin | set | github: 71629 |
| 2016年07月09日 09:09:29 | xdegaye | set | status: open -> closed resolution: fixed stage: commit review -> resolved |
| 2016年07月09日 09:06:42 | python-dev | set | nosy:
+ python-dev messages: + msg270031 |
| 2016年07月08日 22:51:12 | skrah | set | messages: + msg270013 |
| 2016年07月08日 20:00:55 | xdegaye | set | messages: + msg270008 |
| 2016年07月08日 10:01:17 | skrah | set | messages: + msg269982 |
| 2016年07月08日 09:39:49 | vstinner | set | messages: + msg269981 |
| 2016年07月08日 08:23:14 | skrah | set | messages: + msg269980 |
| 2016年07月08日 08:19:25 | skrah | set | files: + issue27442-3.patch |
| 2016年07月07日 22:26:47 | xdegaye | set | messages: + msg269963 |
| 2016年07月07日 22:24:31 | xdegaye | set | messages: + msg269962 |
| 2016年07月07日 20:24:03 | vstinner | set | messages: + msg269958 |
| 2016年07月07日 17:38:11 | berker.peksag | set | nosy:
+ berker.peksag messages: + msg269954 |
| 2016年07月07日 14:48:47 | xdegaye | set | messages: + msg269938 |
| 2016年07月07日 14:43:39 | xdegaye | set | messages: + msg269937 |
| 2016年07月06日 19:19:37 | vstinner | set | messages: + msg269902 |
| 2016年07月06日 10:18:24 | xdegaye | set | files:
+ android_api_3.patch messages: + msg269878 stage: patch review -> commit review |
| 2016年07月05日 16:11:38 | yan12125 | set | messages: + msg269834 |
| 2016年07月05日 16:03:05 | xdegaye | set | messages: + msg269832 |
| 2016年07月05日 15:49:47 | yan12125 | set | messages: + msg269829 |
| 2016年07月05日 15:34:04 | xdegaye | set | messages: + msg269828 |
| 2016年07月04日 20:02:10 | xdegaye | set | files:
+ android_api_2.patch messages: + msg269807 components: + Library (Lib), - Interpreter Core title: expose Android API level in sys.implementation -> expose the Android API level in sysconfig.get_config_vars() |
| 2016年07月03日 09:37:01 | xdegaye | set | messages: + msg269757 |
| 2016年07月03日 08:40:42 | xdegaye | set | nosy:
+ skrah messages: + msg269756 |
| 2016年07月03日 00:09:52 | ned.deily | set | nosy:
+ ned.deily messages: + msg269748 |
| 2016年07月02日 18:21:08 | vstinner | set | messages: + msg269726 |
| 2016年07月02日 17:39:57 | yan12125 | set | messages: + msg269724 |
| 2016年07月02日 15:28:08 | xdegaye | set | messages: + msg269721 |
| 2016年07月02日 15:12:20 | yan12125 | set | messages: + msg269720 |
| 2016年07月02日 15:11:45 | yan12125 | set | nosy:
+ yan12125 messages: + msg269719 |
| 2016年07月02日 15:00:04 | xdegaye | link | issue26865 dependencies |
| 2016年07月02日 14:57:16 | xdegaye | set | files:
+ android_api.patch keywords: + patch messages: + msg269717 stage: patch review |
| 2016年07月02日 14:52:55 | xdegaye | create | |