New build System by mgautierfr · Pull Request #22 · kiwix/java-libkiwix

mgautierfr · 2022-12-20T13:52:38Z

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.

This mainly do: - move `src/wrapper/java` into `lib/src` (and remove `src/wrapper`) - create `lib/src/cpp` and move all `*.(cpp|h)` in it - move all gradle stuff in `android-kiwix-lib-publisher` into `.` - remove `android-kiwix-lib-publisher`

This is mainly a update of gradle itself and new build files. Build files are not working as the current structure is not the same as `Issue#19`.

@MohitMaliFtechiz

zim files has been changed but I don't know why. Change came from commits 3cd8ed9 and 8bff08c but it is not explain what are the changes. @MohitMaliFtechiz ?

Let's be a bit more closer than `android-libkiwixbuild/app/src/main`

MohitMaliFtechiz

Project structure looks clean some of the changes required are requesting.

MohitMaliFtechiz · 2022-12-21T13:02:52Z

+          java-version: 11
+
+      - name: Build Project
+        run: bash contrib/build.sh


it should be bash install_deps.sh

MohitMaliFtechiz · 2022-12-21T13:02:55Z

+          java-version: 11
+
+      - name: Build Project
+        run: bash contrib/build.sh


it should be bash install_deps.sh

MohitMaliFtechiz · 2022-12-21T13:02:59Z

+        run: bash contrib/build.sh
+
+      - name: Compile Project
+        run: |


For new folder structure we don't need to go into android-libkiwixbuild we should remove this. now we can directly use ./gradlew androidSourcesJar build

MohitMaliFtechiz · 2022-12-21T13:03:01Z

+          ./gradlew androidSourcesJar build
+
+      - name: create unit coverage
+        run: |


MohitMaliFtechiz · 2022-12-21T13:03:05Z

+          java-version: 11
+
+      - name: Building Project
+        run: bash contrib/build.sh


Here we should use bash install_deps.sh

MohitMaliFtechiz · 2022-12-21T13:03:07Z

+        run: bash contrib/build.sh
+
+      - name: Release build
+        run: |


For new folder structure we don't need to go into android-libkiwixbuild we should remove this. Now we can directly use ./gradlew androidSourcesJar build

MohitMaliFtechiz · 2022-12-21T13:03:10Z

+          ./gradlew assemble
+
+      - name: Genrate Source jar
+        run: |


MohitMaliFtechiz · 2022-12-21T13:05:57Z

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.

- Build everything in buildDir instead of copy/generate things in src.

mgautierfr · 2022-12-21T15:04:00Z

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 · 2022-12-22T12:54:09Z

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 · 2022-12-22T14:27:30Z

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 · 2023-01-11T15:21:07Z

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

MohitMaliFtechiz · 2023-01-11T16:06:01Z

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.

mgautierfr added 7 commits December 20, 2022 11:46

Reorganize structure.

4cafa3d

This mainly do: - move `src/wrapper/java` into `lib/src` (and remove `src/wrapper`) - create `lib/src/cpp` and move all `*.(cpp|h)` in it - move all gradle stuff in `android-kiwix-lib-publisher` into `.` - remove `android-kiwix-lib-publisher`

Copy gradle things from branch Issue#19

42eb772

This is mainly a update of gradle itself and new build files. Build files are not working as the current structure is not the same as `Issue#19`.

Update settings.gradle.

d5c9e46

Move lib/src/main/java/org/kiwix/testing to lib/src/test

a0d95cb

Copy test from branch Issue#19

3e02f17

zim files has been changed but I don't know why. Change came from commits 3cd8ed9 and 8bff08c but it is not explain what are the changes. @MohitMaliFtechiz ?

Move things in src.

a3a7fa5

Let's be a bit more closer than `android-libkiwixbuild/app/src/main`

Get build scripts from Issue#19

3b4ca8a

mgautierfr requested a review from MohitMaliFtechiz December 20, 2022 13:52

mgautierfr mentioned this pull request Dec 20, 2022

Adapt wrapper to new API of libzim/libkiwix #23

Merged

MohitMaliFtechiz requested changes Dec 21, 2022

View reviewed changes

mgautierfr added 4 commits December 21, 2022 16:01

Adapt build system

f7f6d56

- Build everything in buildDir instead of copy/generate things in src.

Add CI (github and codecov) from branch Issue#19

ba990be

Adapt README.md and change build.sh into install_deps.sh

7de7510

Adapt CI to new structure.

c4b582c

mgautierfr force-pushed the adapt_buildsystem branch from 3a2fa2d to c4b582c Compare December 21, 2022 15:01

kelson42 merged commit 98388aa into main Jan 11, 2023

kelson42 deleted the adapt_buildsystem branch January 11, 2023 16:35

Uh oh!

Conversation

mgautierfr commented Dec 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MohitMaliFtechiz left a comment

Choose a reason for hiding this comment

Uh oh!

MohitMaliFtechiz Dec 21, 2022

Choose a reason for hiding this comment

Uh oh!

MohitMaliFtechiz Dec 21, 2022

Choose a reason for hiding this comment

Uh oh!

MohitMaliFtechiz Dec 21, 2022

Choose a reason for hiding this comment

Uh oh!

MohitMaliFtechiz Dec 21, 2022

Choose a reason for hiding this comment

Uh oh!

MohitMaliFtechiz Dec 21, 2022

Choose a reason for hiding this comment

Uh oh!

MohitMaliFtechiz Dec 21, 2022

Choose a reason for hiding this comment

Uh oh!

MohitMaliFtechiz Dec 21, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MohitMaliFtechiz commented Dec 21, 2022

Uh oh!

mgautierfr commented Dec 21, 2022

Uh oh!

MohitMaliFtechiz commented Dec 22, 2022

Uh oh!

mgautierfr commented Dec 22, 2022

Uh oh!

mgautierfr commented Jan 11, 2023

Uh oh!

MohitMaliFtechiz commented Jan 11, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mgautierfr commented Dec 20, 2022 •

edited

Loading