On 01/11/11 21:23, Chris Frey wrote:
Can you explain to me the technical reasons for the time.h rename?
I kinda have that in there on purpose, to expose brokenness... and it
looks like Android is broken, if this is a problem. I'm therefore
pre-disposed to not like that patch, unless it comes with a reeeeally,
reeeeally good reason. :-)
Yes. The problem comes that the #include <time.h> on Android includes
itself, which causes compilation to fail as the 'time' function isn't
specified. The reason for this happening in Android and not on desktop
Linux appears to be that the Android build system specifies the Android
system includes using -I to GCC.
The reason for this is, I suspect, due to the Android NDK tools being
able to target several different Android versions. Each version has
different functions present and so the usr/include for that particular
version is included. So you end up with the following search path (with
uninteresting includes removed):
-I/home/tg/src/barry-trees/tobygray/barry/android/jni/barry
-I/home/tg/Android/NDK/platforms/android-3/arch-arm/usr/include
It appears that using the -isystem option to GCC won't help as
directories in -isystem are always searched after -I and -isystem is
really just controlling if warnings are displayed (see
http://gcc.gnu.org/onlinedocs/cpp/System-Headers.html), and my attempts
at modifying the Android toolchain to do this confirm this.
I think there are a few options for working around this:
1) Rename time.h to barrytime.h as in the original patches, but like you
say this is a bit nasty.
2) Use #include_next <time.h> inside barry/src/time.h to make sure the
time.h comes from a different search path. This would need to have
suitable #ifdef guards and configure support for detecting if the cpp
supports include_next.
3) Use #include <../include/time.h> inside barry/src/time.h and ignore
the fact that it's a hideous hack! It also only works if the system
headers are in a directory named include.
4) Use -iquote instead of -I for the barry headers when building barry
for Android.
I went for 4 in the end as it only requires changes to the Android build
system (see 5f7ec2deb5ef91e0069c286dd58eede286bc378b).
Also, instead of the #ifdefs in various non-compatible places in the
source code, I'd rather see a platform specific module, maybe android.cc,
which gets added optionally during ./configure, where things like
stub versions of pthread_cancel() are stored. Also, an iconvandroid.cc,
similar to iconvwin.cc, instead of cluttering the main iconv.cc
This way, we get to rely on common APIs in the rest of the code, and
the platform specific stuff can get as hairy as it needs to in its own
file, and each file is straightforward, without having to run a mental
preprocessor while coding.
I seem you've removed the iconv.cc changes in a later patch, so this
is just commentary now. :-)
I was still doing some #ifdefs in configfile.cc for getpwuid*() calls.
I've done as you suggested in my github branch.
I've also done a tidy on my github branch to merge the changes which I
then undid (such as the iconv.cc one and the build script as a shell
script instead of a Makefile). I'm very impressed by how easy this was
to do with "git rebase --interactive d7816d80".
I've added some documentation on building and using Barry for Android,
which I realised was missing before. Also I've added the Android build
files to the list of extra distribution files so that they get included
in the release tarball.
Finally I apologise for the whitespace changes you've had to make, I
thought I'd managed to set up my emacs to use the correct indentation
levels and characters for the Barry coding style, but I evidently didn't
set it up correctly for all file types.
What do you think of the changes in the master branch of my github
repository now?
Regards,
Toby
------------------------------------------------------------------------------
RSA® Conference 2012
Save $700 by Nov 18
Register now!
http://p.sf.net/sfu/rsa-sfdev2dev1
_______________________________________________
Barry-devel mailing list
Barry-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/barry-devel
