-
Notifications
You must be signed in to change notification settings - Fork 269
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
NDK should have a #define for the NDK version #407
Comments
There is a file source.properties in NDK_HOME which contains NDK version. |
In some older versions, the file was called |
the problem i have is that there's no purely numeric representation of the NDK version. something like "r15beta2" doesn't play nice with i could use the NDK build number, but i feel bad inflicting basically random (albeit monotonically increasing) numbers on the world. |
Do we want to cram all the version info into one define, or have a handful? #define __NDK_MAJOR__ 15 // r15.
#define __NDK_MINOR__ 1 // 0 is no hotfix, 1 is r15b, etc.
#define __NDK_BETA__ 0 // Not a beta.
#define __NDK_BUILD__ FULL_BUILD_NUMBER
#define __NDK_CANARY__ 0 // 1 if this is a canary build. Or #define __NDK_VERSION__ 150100 // Leading digits are major, 2 digits for minor, 2 digits for beta.
#define __NDK_BUILD__ FULL_BUILD_NUMBER
#define __NDK_CANARY__ 0 |
i don't have a strong opinion, but i think my experience has been that what people do with the individual numbers is multiply them up into a single number for convenience. with this aiui you're suggesting: 150001: r15beta1 which is fine by me: as long as it works with >= that lgtm. |
Trivial enough for us to provide both, I suppose. My thinking is that the only thing that should matter is the major version. People should only be on the betas until the release goes stable, and should always take the hotfix releases. Projects shouldn't be keeping around compatibility for beta or unpatched versions IMO. |
Why invent something new? You have -DANDROID. What could go wrong with -DANDROID=15? |
That would cause confusion between ANDROID platform API version and the NDK headers/compiler toolchain version. |
wait a sec. Compilers set their version #defines separately |
"am i building with the NDK?" - what will be the answer for Android Studio native cmake? For standalone toolchain? |
Decoder ring for the many Android related defines:
|
Actually, I am trying to come up with a legitimate use case for compile-time check |
Some third-party projects want to support a huge range of NDKs. Not all of our header fixes are backwards compatible, if you want to support both the latest thing and r9 for whatever reason, you may need different code for each. Knowing that you are building with the NDK ( |
Concrete example - breakpad (the crash reporting library from Google) redefines missing structs for some kernel headers and those structs have been added piece by piece to NDK 13 and 14. Having a version define would help there, since there's now just simply no good way of conditionally defining those missing structs and supporting compilation on several NDK versions. |
@izacus, I am afraid breakpad is not a good example. First of all, this falls in the category of solutions that require time machine. There is no way such #define can help to sort out the headers between NDK r13 and r14. Second, the breakpad library provides android/google_breakpad/Android.mk which can easily read and analyze the existing $(NDK_ROOT)/source.properties file and tune the headers and defines as necessary. Third, for crash reporting library like breakpad, it is legitimate to require an up-to-date NDK release, and drop support for legacy NDK. Today, the master requires NDK release r11c or higher. This README.ANDROID has not been updated for a year, while the library itself is changing weekly. Are all changes actually tested with r11c? I doubt it. |
You're now bikeshedding on an example which was used to demonstrate a wider point of issues we're facing when not being able to check which version of NDK the code is currently compiled with. |
i don't know... his example mostly convinced me. i'm not sure it's in anyone's interests for us to make it easier for projects to support old versions of the NDK in addition to the current one. what i'd originally been thinking about was actually the opposite though: making it easier for projects to support the next/current NDK release before all their users have upgraded. (and so they can fit such changes into their own release schedule better. we feel pain because the NDK has to release somewhat in sync with the platform and Studio and so on... i don't want every project that builds with the NDK to end up with a similar dependency.) i did also originally intend this to help with the platform/app distinction danalbert mentions, but i'm also not sure that's a great idea. maybe folks should be asking what they really mean: "do i have boringssl available?", say, which is strictly orthogonal to platform/app, even though it's trivially true for platform and a PITA for apps to make true. |
Re: releasing in sync with Studio, see https://issuetracker.google.com/issues/62529380. Or maybe you think it's more appropriate to be opened as an issue for NDK. |
As for future versions of NDK, you mean tuning a library for the next while it is still in beta. IMO, and in my experience, keeping support for more than one may easily get very hard. An alternative is to open a separate branch that is tuned for the beta toolchain, and merge it into master when NDK matures. |
Yeah, I think the latter is what we'd be doing. If we force people to pick only one, network effect is going to force people to stick on an old release, which increases the network effect, and so on.
This should be getting easier over time (unified headers were almost certainly the opposite of this, but with that out of the way it should be getting easier now). The define isn't hard for us to provide, so if this helps anyone (I suspect this is more likely useful to middleware vendors than actual apps) I think it's worthwhile. |
That's definitely true, but the question is whether you will have it buried in some very central header, or in one of the core scripts. Is there any sense in keeping this define for a standalone toolchain, or not. Any choice is better than none, but the decision will have long-term implications.
I did not mean the 'accidental' support, e.g. scripts tuned for NDK r.5 were most of the time OK for r.6 because the changes were minimal and very compatible. I was speaking about the deliberate effort to 'support NDK r11b and higher', as stated for breakpad. |
random example from toybox: when building against the platform, they want #include <cutils/sched_policy.h>, but obviously that's not an option for folks building with the NDK. a __NDK__ #define would let them say #ifdef __NDK__ rather than the current not-quite-correct #ifdef __ANDROID__. (this actually makes me think we should probably call this __ANDROID_NDK__ instead. roku has an NDK, for example.) |
I wanted to add a patch to CMake that would detect the version of NDK. This requires supporting all versions of NDK, ideally. This isn't a compile-time check, but something similar to At the moment, I don't think this is possible pre-r11 NDK. |
A real example of where this is needed now is #272. Right now one needs hacks to work around that issue, but you'll only want to keep those hacks around while r15 and earlier are still in circulation. |
I've added some things like __ANDROID_MAJOR__ to an ndk-version.h, but that is only in the NDK itself and so doesn't help the platform. Add __ANDROID_NDK__ to identify that you're building for the NDK and not the platform. Test: make checkbuild Bug: android/ndk#407 Change-Id: I2d1f1c28e3764e4e658cf675b290b7a17253ee33
Another concrete example: with the newly released ndk |
Test: ./checkbuild.py, examine output Bug: android/ndk#407 Change-Id: I4c0c58b0807f6da565f4c680fec574a3ceb58fde
"__ANDROID__ is defined by the compiler. It is defined for all Android targets." "ANDROID is set by some build systems, and not in any reliable manner (in AOSP it actually just means that you're in AOSP; it's set even when you're building host code for Windows, so it doesn't mean much of anything)." android/ndk#407
there are places where it would be helpful to know "am i building with the NDK?" but even more so, places where you want to know which version of the NDK. if we want to get support for the next/beta release into a project but they (reasonably enough) want to keep working with the current stable release, for example, it would help if they could say something like
#if (__NDK__+0)>=15
.The text was updated successfully, but these errors were encountered: