Skip to content
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

DO-NOT-MERGE: debug android CI #1225

Closed
wants to merge 8 commits into from

Conversation

neolit123
Copy link
Member

@neolit123 neolit123 commented Apr 28, 2018

Describe the pull request:

  • Bug fix
  • Functional change
  • New feature
  • Code cleanup
  • Build system change
  • Documentation change
  • Language translation

Pull request long description:

In android-build-wrapper.sh:

  • Always create the folder 'licenses'
  • Export 'SDKMANAGER_OPTS'

Fixes a recent failure of the Android CI build
where 'licenses/android-sdk-license' was no found
after the 'yes | ...' call.

i don't really know what's going on here. found a solution online and tried the CI multiple times until it worked.

related thread:
https://stackoverflow.com/a/47150411

Changes made:

Related issues:

Additional information:

Release note:

Documentation change:

Mentions:

@glance- @dirkhh

@neolit123 neolit123 force-pushed the debug-ci-android branch 11 times, most recently from 73e56c5 to 88600f6 Compare April 28, 2018 22:00
In android-build-wrapper.sh:
- Always create the folder 'licenses'
- Export 'SDKMANAGER_OPTS'

Fixes a recent failure of the Android CI build
where 'licenses/android-sdk-license' was no found
after the 'yes | ...' call.

Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
@neolit123 neolit123 changed the title [do-not-merge]: DEBUG: android-build-wrapper.sh android: fix sdkmanager license issues Apr 28, 2018
@@ -77,6 +77,9 @@ if [ ! -d $ANDROID_SDK ] ; then
mkdir $ANDROID_SDK
pushd $ANDROID_SDK
unzip -q ../$SDK_TOOLS
mkdir -p licenses
# use this fix from https://stackoverflow.com/a/47150411 or sdkmanager throws Java exceptions
export SDKMANAGER_OPTS='-XX:+IgnoreUnrecognizedVMOptions --add-modules java.se.ee'
yes | tools/bin/sdkmanager --licenses > /dev/null 2>&1 || echo "d56f5187479451eabf01fb78af6dfcb131a6481e" > licenses/android-sdk-license
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the weird thing is that this last line after the '||' already tries to manually force a license. I assume that key is no longer valid or something?
If your solution works more reliably, even better.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think the export made the call before || work.

@dirkhh
Copy link
Collaborator

dirkhh commented Apr 28, 2018

Build failed on Android - but with weird errors:

/workspace/libgit2-0.26.0/src/diff_print.c:739: error: undefined reference to 'stdout'
/workspace/libgit2-0.26.0/src/unix/map.c:52: error: undefined reference to 'mmap64'
collect2: error: ld returned 1 exit status
make[2]: *** [libsubsurface-mobile.so] Error 1
make[1]: *** [CMakeFiles/subsurface-mobile.dir/all] Error 2

Did we change SDKs or something while I wasn't paying attention?

@neolit123
Copy link
Member Author

neolit123 commented Apr 28, 2018

but then it fails with:

[ 81%] Linking CXX shared library libsubsurface-mobile.so
/workspace/libgit2-0.26.0/src/diff_print.c:739: error: undefined reference to 'stdout'
/workspace/libgit2-0.26.0/src/unix/map.c:52: error: undefined reference to 'mmap64'

sorry, didn't see the above comment earlier:

Did we change SDKs or something while I wasn't paying attention?

that's a good question. i don't think we did and the breakage started happening yesterday,.

@neolit123
Copy link
Member Author

possibly relevant comment:
android/ndk#543 (comment)

so if you're getting undefined references to stdin/stdout/stderr, you're not building for < 23 even if you think you are.

@neolit123 neolit123 force-pushed the debug-ci-android branch 2 times, most recently from 0e4b9ab to 6dad4d4 Compare April 29, 2018 00:58
Attempt to fix "undefined reference to 'stdout'" by
adding a __ANDROID_API__ definition equal to the Android
API version for the toolchain.

ref:
android/ndk#272 (comment)

Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
@neolit123
Copy link
Member Author

so. i've tried to fix the stdout issue but then this happens:

/workspace/libgit2-0.26.0/src/hash/hash_openssl.h:13:25: fatal error: openssl/sha.h: No such file or directory

even with:

		-DOPENSSL_INCLUDE_DIR="$PREFIX"/include/openssl \

tried to fix that with explicitly passing -I"$PREFIX"/include/openssl' but it doesn't work.

also, i'm seeing:

warning: implicit declaration of function 'futimens'

which will probably fail at link time for libgit.

no idea what what changed in the CI but it appears to not be on our side and is a PITA to deal with.

@neolit123 neolit123 changed the title android: fix sdkmanager license issues DO-NOT-MERGE: debug android CI Apr 29, 2018
@dirkhh
Copy link
Collaborator

dirkhh commented Apr 29, 2018

Yes, debugging these builds on Travis is indeed a PITA.
If we go back to an older build that passed and restart that, does it still pass?
I'll try that.
Also, @glance- is typically our experts when I run out of ideas, maybe he can take a look?

@dirkhh
Copy link
Collaborator

dirkhh commented Apr 29, 2018

This build passed a couple of days ago, I just restarted it: https://travis-ci.org/Subsurface-divelog/subsurface/jobs/372257556
If that now fails the change wasn't ours...

@dirkhh
Copy link
Collaborator

dirkhh commented Apr 30, 2018

This is massively weird.
makedepend is a host command - so installing xutils-dev should be all you need.
Is the PATH messed up? Can you do an ls -l /usr/bin/makedepend?

@neolit123 neolit123 force-pushed the debug-ci-android branch 3 times, most recently from 80ef2ea to 1d7c19f Compare April 30, 2018 01:38
@neolit123
Copy link
Member Author

i've added this to the yaml for android:

sudo: required
...
      addons:
        apt:
          packages:
            - xutils-dev

Can you do an ls -l /usr/bin/makedepend?

it's not present even after the yaml additions.

then i tried a:

if [ ! -e /usr/bin/makedepend ] ; then
	sudo apt-get install xutils-dev
fi
ls -l /usr/bin/makedepend

and it says:

sudo: command not found

as if sudo: required doesn't work.
ending this for now. will see if i can find some time tomorrow (today).

@glance-
Copy link
Collaborator

glance- commented Apr 30, 2018

      addons:                                                                   
        apt:                                                                    
          packages:                                                             

doesn't work there for the android builds. They are happening inside a container to get a sane build enviorment. You need to add it in scripts/android/before_install.sh instead.

Yes, its sort of odd, but it was the least 4kery to get it to work.

If the whole debacle isn't fixed by someone else I'll take a stab at it tonight.

@neolit123 neolit123 force-pushed the debug-ci-android branch 3 times, most recently from 7f51de7 to fcfaac8 Compare April 30, 2018 12:22
For some reason the Android CI build started failing with:
    makedepend: not found
when building OpenSSL.

Try to 'apt-get install' it in before_install.sh
via xutils-dev.

Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
@neolit123
Copy link
Member Author

got makedepend working.

but had this error:
OpenSSL : unknown argument: '-mandroid'

found this comment online:
OpenSSL doesn't support compilation with clang (on Android) yet. You'll have to use GCC.

well this sucks...but before i switch back to GCC i'm going to try to hack the makefiles to make them work for clang.

@neolit123 neolit123 force-pushed the debug-ci-android branch 5 times, most recently from 9e7a336 to e5b26f3 Compare April 30, 2018 14:14
@dirkhh
Copy link
Collaborator

dirkhh commented Apr 30, 2018

OK, at least I wasn't completely lunatic when I remembered that one of our components didn't compile with clang...

@neolit123
Copy link
Member Author

the latest error is:

arm-linux-androideabi-clang -I.. -I../.. -I../modes -I../asn1 -I../evp -I../../include  -fPIC -DOPENSSL_PIC -DOPENSSL_THREADS -D_REENTRANT -DDSO_DLFCN -DHAVE_DLFCN_H -march=armv7-a  -I/include -B/lib -O3 -fomit-frame-pointer -Wall -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_GF2m -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DAES_ASM -DBSAES_ASM -DGHASH_ASM -c --sysroot=/workspace/ndk-arm/sysroot  -c -o sha256-armv4.o sha256-armv4.S

sha256-armv4.S:2638:2: error: invalid instruction

 adrl r3,K256

i will remove some of the DSHA256_ASM to see if it helps.

@neolit123 neolit123 force-pushed the debug-ci-android branch 2 times, most recently from 89b3dfb to 97ba5b5 Compare April 30, 2018 14:48
OpenSSL fails to build after 'make depend'.

1) The -mandroid flag is not supported on Clang.
Remove it from the Makefile.
2) Some .S files throw:
    error: invalid instruction
Remove the *_ASM defines from the Makefile.
3) add no-asm to the 'config' line in 'env'

Signed-off-by: Lubomir I. Ivanov <neolit123@gmail.com>
@neolit123
Copy link
Member Author

neolit123 commented Apr 30, 2018

alright, i'm going to have to put this to rest for today.
can probably go back to it this week. not sure when.

https://api.travis-ci.org/v3/job/373075755/log.txt

the good:
openssl now builds with clang

the bad:
libgit2 still has errors.


/workspace/libgit2-0.26.0/src/unix/posix.h: In function 'p_futimes':

/workspace/libgit2-0.26.0/src/unix/posix.h:82:2: warning: implicit declaration of function 'futimens' [-Wimplicit-function-declaration]

  return futimens(f, s);

the posix.h header needs futimens which seems to be missing for both GCC and Clang. this will surely fail and link time. i don't see means in libgit2 to not use this header or to tell it that the symbol is not available. maybe yet another inline hack is needed here. the NDK toolchains have weird ways to define things like stdout so assuming that they are POSIX compliant is quite bold.

/workspace/libgit2-0.26.0/src/hash/hash_openssl.h: At top level:

/workspace/libgit2-0.26.0/src/hash/hash_openssl.h:13:25: fatal error: openssl/sha.h: No such file or directory

 #include <openssl/sha.h>

this one is weird given the previous action in the .sh is to make install openssl. but it appears that either pkg-config isn't working or something is messed up with the includes.

note: i was seeing these error messages with the old NDK and with GCC so it's not a Clang only issue.

it seems as if every other library has build issues. the build.sh is quite hacky and would be quite impressive if we get the APK to actually run.

@dirkhh
Copy link
Collaborator

dirkhh commented Apr 30, 2018

Given that everything was building fine with the OLD 14b until just a couple of days ago - I'm tempted to just upload that old 14b from my local copy. Not the right long term solution, but this is just a nightmare.

@glance- any insights from you would be highly appreciated.

@dirkhh
Copy link
Collaborator

dirkhh commented May 6, 2018

ok, looks like @glance- doesn't have the time to look into this right now (?)
I think I'll just say "SCREW THIS" and upload an older version of the 14b SDK that still should work - hopefully I'll get to this later today or tomorrow... gotta run right now

@neolit123
Copy link
Member Author

if the old archive works, great. 👍

debugging the CI in the case of such a bad toolchain is very time consuming and i wish there was a way to "pause" a Travis VM and SSH into it or something in those lines.

both the above issues seem solvable with extra defines and includes but i don't know what else will break next in the script. could give it another try next week.

@dirkhh
Copy link
Collaborator

dirkhh commented May 6, 2018

Took a step back and compared again what's different between a working build and a failing build.
And that made the ACTUAL problem obvious. No, Google did NOT change the old 14b repo without telling anyone. That would, frankly, be a pretty insane thing to do for them.
Instead our Travis build used Ubuntu:latest - and two weeks ago that switched to Bionic which breaks the build. Forcing this to be Xenial (16.04) made it all work again.
Closing this (sure, eventually we'll want to figure out how to build on Bionic; maybe).

@dirkhh dirkhh closed this May 6, 2018
@neolit123
Copy link
Member Author

thanks for finding the actual cause!

it's interesting how Bionic broke this that bad...
i will keep this branch if we intend a switch in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants