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
Update Android Instructions for newer NDKs #9732
Conversation
[Pretty sure the failures are unrelated to this change, since I only changed docs, and other PRs are failing, too.] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I'm not sure what you mean with __has_includes. What would be the difference? They're just defines that tell if the header file exits on the platform or not. And yes we have them to make sure curl builds on as many platforms as possible. |
I'm scared of the avalanche of issues and problems coming with that, without people around in the project to care for them. We already ship (too) many different build options and many of them lag behind the autotool build in terms of features and completeness a lot of the time. |
The (new) uppercase check 1275 fails on this:
but I think that's an error in the script. I will work on a fix for that separately. |
Thanks for looking and for your fast approval! Honor getting to interact with you. Re Bazel: Totally get it and sorta assumed as much. I'll release it separately as a wrapper and see if I can get the TensorFlow guys to help support. Re __has_includes: It's a sweet preprocessor operator that accomplishes the same goal, but without having to run a (slowish) check in each build system. Could be used to make headers/code that just does the right thing, regardless of build system! |
Unfortunately, |
#9733 fixed the bogus test fail |
Thanks! |
Hi, @bagder!
First and foremost, thank you so much for such a wonderful tool and library--and for sharing it with the world.
I was building things across platforms, reading your good docs. Along the way, I noticed that some of the Android instructions seemed to have gotten out of date with the switch to LLVM toolchains in newer NDK versions. So, I figured I'd try to leave things even better than I found them, especially since the triggered error messages are different enough from their root causes. Here's my shot at that for your consideration.
Cheers!
Chris
P.S. To explain some of the changes:
P.P.S. What originally brought me to the doc was to create a cross-platforms set of build scripts for libcurl for Bazel, which is an increasingly popular build system from Google. Lots of folks have been spinning up Bazel BUILD files for libcurl online, mostly copying tensorflow's (fairly buggy) ones. Since those all looked problematic, I figured I'd take a shot at writing an improved set. If you'd be open to accepting my good set of Bazel BUILD files, please let me know! The advantage would be stupid easy (incremental) builds across platforms, making libcurl easily usable by Bazel users.
P.P.P.S :) Just to confirm a hypothesis: It's the need to support older compilers that prevents all the HAVE_[whatever]_H macros from being converted to __has_include's, right?