Moved ndk_api parsing earlier in program initialisation #1472
Conversation
f139507 to
a6f2c83
Compare
|
Testing with this, it seems the API level does not pull through. It picks up API 27 earlier, but then it's 0 when looking in the ndk... https://gist.github.com/Zen-CODE/6d3f1e7daa1d7c305ed3380fbfd18e16 |
a6f2c83 to
869943f
Compare
|
Thanks @Zen-CODE , I've pushed a correction for that. |
pythonforandroid/toolchain.py
Outdated
| @@ -528,7 +537,9 @@ def add_parser(subparsers, *args, **kwargs): | |||
| self.ndk_dir = args.ndk_dir | |||
| self.android_api = args.android_api | |||
| self.ndk_version = args.ndk_version | |||
| self.ndk_api = args.ndk_api | |||
| args.ndk_api = select_ndk_api(args.ndk_api, args.android_api) | |||
| self.ndk_api = args.ndk_Api | |||
There was a problem hiding this comment.
Travis didn't like that either 😄
AttributeError: 'Namespace' object has no attribute 'ndk_Api'
There was a problem hiding this comment.
I have also tried, but got this error:
args.ndk_api = select_ndk_api(args.ndk_api, args.android_api)
NameError: name 'select_ndk_api' is not defined
please any help!
|
Pulling these changes,I still get the same problem. It happens with API 21 and 27.... https://gist.github.com/Zen-CODE/9c1b2ea9022086565f5a7900bc7ea272 ps. This try was with the typo correction mentioned above |
869943f to
e030dbe
Compare
|
Still get the same error... |
e030dbe to
75c29be
Compare
|
@Zen-CODE @AndreMiras Is it working now? Can this be merged? |
|
Hm, does this pull request remove the The thing is that I moved this out into the docker environment image which sets all of this in the env, such that the actual building project doesn't need to worry and can only specify the minimal |
| ndk_api, self.android_api, DEFAULT_NDK_API)) | ||
| ndk_api = int(ndk_api) | ||
| self.ndk_api = ndk_api | ||
| self.ndk_api = user_ndk_api |
There was a problem hiding this comment.
Can we possibly keep NDKAPI env var support in? I think this is really important for all options that are more dependent on the installed build env (NDK/SDK/... downloaded) rather than the project itself - because otherwise, these can't be preconfigured to proper defaults in e.g. a docker build env
|
From my side, I still get as posted earlier.... |
|
Closing as obseleted by more recent changes. |
This is actually quite important because we need to know a reasonable number for the NDK API even in places where the build environment has not been prepared.
I've removed the ability to set this parameter via environment variable because, on reflection, I don't think it's actually really important or useful. The environment variables we do accept are there partly for legacy reasons, and are mainly useful for the ndk/sdk locations rather than the api number to target.
The control flow around argument parsing could do with revisiting, but I think this is the neatest solution for now.