New build System - Githubissues

kiwix / java-libkiwix

Libkiwix binding for Java & Kotlin

https://central.sonatype.com/artifact/org.kiwix/libkiwix

GNU General Public License v3.0

3 stars 4 forks source link

New build System #22

Closed mgautierfr closed 1 year ago

mgautierfr commented 1 year ago

This branch is mainly a cleanup/rebase of #20

With far less commits
Less unnecessary files added and removed
No adaption of wrapper itself (so it doesn't compile with current version)

While it "compile" (with the change in PR #23) it is NOT TESTED. The testing part itself doesn't work (and I'm not sure how it should work, @MohitMaliFtechiz we have to discuss about this).

@MohitMaliFtechiz please review this PR and check that I haven't forget something compared to #20. There is few questions to you in the commits' messages.

There is still few things I disagree with this PR but I suggest we merge it (if there is nothing obviously wrong) as it is and fix the issues later, one by one.

MohitMaliFtechiz commented 1 year ago

hi @mgautierfr ,

Related to testing part , Test cases were written purely in java they are not android specific test cases , so to run test cases written on java required .so files which are compiled on Linux variant not android variant so that why we are compiling its separately.

mgautierfr commented 1 year ago

I've indeed forget to adapt the github workflow to the new file structure. Thanks. It should be ok now.

Related to testing part , Test cases were written purely in java they are not android specific test cases , so to run test cases written on java required .so files which are compiled on Linux variant not android variant so that why we are compiling its separately.

I don't think that testing part should compile the library to test. We should have a target to build the library (and different targets for different archs) and having the test target depending of the target building the library for linux x86_64

MohitMaliFtechiz commented 1 year ago

I've indeed forget to adapt the github workflow to the new file structure. Thanks. It should be ok now.

Related to testing part , Test cases were written purely in java they are not android specific test cases , so to run test cases written on java required .so files which are compiled on Linux variant not android variant so that why we are compiling its separately.

I don't think that testing part should compile the library to test. We should have a target to build the library (and different targets for different archs) and having the test target depending of the target building the library for linux x86_64

We are only building wrapper files to generate .so file while for libzim and libkiwiz we are downloading linux variant .so file from download.kiwix.org

mgautierfr commented 1 year ago

Yes. Here what we should have (big picture):

5 "targets" downloading the libzim.so for the 5 archs (4 android and 1 linux native)
5 "targets" downloading the libkiwix.so for the 5 archs (as libzim)
1 "target" generating the c headers of the wrapper from java classes (maybe 2 targets if we handle wrappers around libzim and libkiwix separately)
5 "targets" creating the wrappers .so for the 5 archs. (Maybe 10 targets if we handle wrappers separately). The targets should depends of the downloading target and header generation target.
1 "target" to build the aar from all .so. This target depends of the 4/8 "creating wrapper" targets (android only)
1 test target which depends of the linux native "creating wraper" and "just" use the .so to test everything is ok

With the chain of dependencies. We would only have to launch ./gradlew build or ./gradlew test to have everything downloaded and compiled.

mgautierfr commented 1 year ago

I propose to merge this PR as it is and fix testing and target dependencies later. @MohitMaliFtechiz, is it ok for you ?

MohitMaliFtechiz commented 1 year ago

I propose to merge this PR as it is and fix testing and target dependencies later. @MohitMaliFtechiz, is it ok for you ?

Yes its fine for now.