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

Add android build on travis #1981

Merged
merged 5 commits into from
Feb 27, 2018
Merged

Conversation

Bjoe
Copy link
Contributor

@Bjoe Bjoe commented Nov 6, 2017

Add android build on travis.

Copy link
Member

@obiltschnig obiltschnig left a comment

Choose a reason for hiding this comment

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

It seems all other builds are commented out?

@Bjoe
Copy link
Contributor Author

Bjoe commented Nov 6, 2017

@obiltschnig This is only to get faster an result. After android build is working I will comment all in and squash all commits.

@obiltschnig
Copy link
Member

Wouldn't it be faster to set up Travis for your fork?

@obiltschnig
Copy link
Member

I guess we don't need to merge this for now, as Travis builds your PR anyway ;-)

@Bjoe
Copy link
Contributor Author

Bjoe commented Nov 6, 2017

Wouldn't it be faster to set up Travis for your fork?

Would be better, but travis is not building on my fork... but maybe I should create a pull request in my own fork?

@Bjoe
Copy link
Contributor Author

Bjoe commented Nov 6, 2017

... but maybe I should create a pull request in my own fork?

No sorry. Travis is not building in my fork. Only when I create a pull request to poco, travis builds my branch.

@zosrothko
Copy link
Member

@Bjoe "Travis is not building in my fork.".. Yes Travis is building your fork, but you have before to add your github repository to the travis workflow. Go on Travis and open you account. Then add your poco's fork

@Bjoe Bjoe force-pushed the travis-android-build branch 2 times, most recently from 82014ef to d5c3ddc Compare November 8, 2017 00:56
@Bjoe
Copy link
Contributor Author

Bjoe commented Nov 8, 2017

@zosrothko Ok thank you! Now it works.

@obiltschnig Ok I cleanup/enable all builds in travis.yml and squashed my commits.
Unfortunately the android emulator is not yet running :-( I will figured out.
As test name I add here "android API level 19", but this should be changed or?

At the moment the android build will fail because we have two issues in android. These are fixed in #1983 and #1975

@Bjoe Bjoe force-pushed the travis-android-build branch 6 times, most recently from 11f084e to 3566e83 Compare November 13, 2017 23:10
@Bjoe Bjoe force-pushed the travis-android-build branch 3 times, most recently from febffd6 to 80c76b9 Compare November 21, 2017 13:39
@Bjoe Bjoe changed the base branch from poco-1.8.0 to poco-1.8.1 November 21, 2017 13:42
@Bjoe Bjoe force-pushed the travis-android-build branch 4 times, most recently from 6a68f20 to ad03198 Compare December 4, 2017 21:58
@Bjoe
Copy link
Contributor Author

Bjoe commented Dec 4, 2017

Ok, done. Now it is ready to be merged, after review is done :-)

I added two android builds. I think POCO should support as minimum API level 19 and as newest 24. These are the most distributed android platform versions. See Platform Versions

There are still some open issues:

Fatal signal 11 (SIGSEGV) at 0x00000000 (code=1), thread 1690 (Foundation-test)
backtrace:
#00 pc 00000000
#1 pc 00049d63 /data/local/tmp/lib/libCppUnitd.so (std::__ndk1::__stdinbuf::imbue(std::__ndk1::locale const&)+26)
#2 pc 00049a71 /data/local/tmp/lib/libCppUnitd.so (std::__ndk1::__stdinbuf::__stdinbuf(__sFILE*, mbstate_t*)+72)

I think the problem has something to do with the standard C++ library. I will figure out what is the problem. For the first time, I decided to use the android API level 24 at runtime.

  • Unfortunately the android NDK didn't provide any OpenSSL version, so all crypto tests are not running. But I think POCO should have this.

For the develop branch, should I create also a pull request?

@Bjoe Bjoe force-pushed the travis-android-build branch 2 times, most recently from 8ed6ee7 to 959197d Compare December 21, 2017 11:08
@zosrothko
Copy link
Member

@aleks-f From my point of view, all change requests are ok. BTW, it is nice to have Poco on Android. Thanks to @Bjoe for this contribution.

@aleks-f
Copy link
Member

aleks-f commented Feb 21, 2018

@Bjoe this pull should go to poco-1.9.0

@Bjoe
Copy link
Contributor Author

Bjoe commented Feb 21, 2018

@aleks-f Yep, I know. But I will fix first the review findings... then test should be back to green and then I will merge or better squash and rebase to poco-1.9.0. Too many changes at once could be difficult to find the problem if the build fails. So when I'm ready I will inform you. Btw. how can I requested a review?

@aleks-f
Copy link
Member

aleks-f commented Feb 21, 2018

how can I requested a review?

image

@Bjoe
Copy link
Contributor Author

Bjoe commented Feb 21, 2018

@aleks-f I didn't have this "button" on the right site of "Reviewers".

@aleks-f
Copy link
Member

aleks-f commented Feb 21, 2018

I'm not sure why, you are a member of developers team and I think you should be able to:

https://help.github.com/articles/requesting-a-pull-request-review/

@zosrothko can you request reviews on poco pulls with your permissions?

@Bjoe
Copy link
Contributor Author

Bjoe commented Feb 21, 2018

@aleks-f I have read this article about "Requesting a pull request review" before and I didn't have this buttons or can search for a person. I think this is the problem:

Note: Pull request authors can't request reviews unless they are either a repository owner or collaborator with write access to the repository.

@Bjoe Bjoe force-pushed the travis-android-build branch 2 times, most recently from f040de9 to a0caa09 Compare February 23, 2018 07:25
@Bjoe Bjoe changed the base branch from poco-1.8.1 to poco-1.9.0 February 23, 2018 07:30
@Bjoe
Copy link
Contributor Author

Bjoe commented Feb 26, 2018

@zosrothko I used first POCO_ANDROID instead of POCO_OS_ANDROID, because it is not only to add POCO_OS_ANDROID in Poco/Platfrom.h... see my pull request #2186

@Bjoe Bjoe added this to the Release 1.9.0 milestone Feb 26, 2018
@aleks-f aleks-f merged commit 9feabc7 into pocoproject:poco-1.9.0 Feb 27, 2018
Bjoe added a commit to Bjoe/poco that referenced this pull request Mar 4, 2018
* Add build for android in travis CI.

* Fix review findings. Change from __ANDORID__ to POCO_ANDROID

* Add android test

* Fix compile issue after rebase

* Ignore test big ping when its failing
Bjoe added a commit to Bjoe/poco that referenced this pull request Mar 4, 2018
* Add build for android in travis CI.

* Fix review findings. Change from __ANDORID__ to POCO_ANDROID

* Add android test

* Fix compile issue after rebase

* Ignore test big ping when its failing
Bjoe added a commit that referenced this pull request Mar 5, 2018
* Add build for android in travis CI.

* Fix review findings. Change from __ANDORID__ to POCO_ANDROID

* Add android test

* Fix compile issue after rebase

* Ignore test big ping when its failing
@obiltschnig
Copy link
Member

Will temporarily remove/disable in 1.9.0 due to failing tests.

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

Successfully merging this pull request may close these issues.

4 participants