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

Move Java files into separate repo #4

Closed gouri-panda closed 2 years ago

gouri-panda commented 3 years ago

Moved Android Specific Java native files and CPP files from kiwix-lib

Imported folders: Android-kiwix-lib-publisher src/wrapper/java

kelson42 commented 3 years ago

@gouri-panda Can you please add a PR description?

gouri-panda commented 3 years ago

@kelson42 Is there any file should I add except the README.md file and the .github folder?

kelson42 commented 3 years ago

Depends what you want to achieve... but at least these one yes. Copy the .github from libkiwix and remove the workflows.

gouri-panda commented 3 years ago

Depends what you want to achieve...

A successful build artifact that we can upload to maven central.

Copy the .github from libkiwix and remove the workflows.

👍 although I don't know what's happening in workflows if we remove them how can we automate build it?

kelson42 commented 3 years ago

@gouri-panda How do you plan to build the jni package because you don't have the libkiwix in the repo. Anymore?

gouri-panda commented 3 years ago

@kelson42 I'm not familiar with how libkiwix builds the library for android. Can we do this here also?

kelson42 commented 3 years ago

@gouri-panda I don't think this is a good idea to do this in this PR. But we need to know how the java binding will be compiled. I'm not an expert, but either we put openzim/libkiwix as a git submodule and we compile in a similar manner like done in kiwix/kiwix-build or like for our other bindings (python-libzim, node-libzim), we publish binary version of libkiwix for Android and they are downloaded here by the build process.

mgautierfr commented 3 years ago

@gouri-panda How did you create those commits ? I suppose with some kind of git filter-branch. It would be nice to have the command you've used. It is somehow useless to review the commits themselves. (I'm not even sure we need the history here, for example, a lot of commit are useless (new version xxxx))

I agree with @kelson42 : it is not this PR to add the build system, but it would be nice (here or somewhere else) to know what is the plan. Either, this repository also build libkiwix (with all its dependencies, which could be difficult) or we make kiwix-build publish build libraries for android arch and this repository "just" download, build the wrapper part and package everything.

gouri-panda commented 3 years ago

@kelson42 Sorry for the late reply! I was caught doing in UI of kiwix-android.

But we need to know how the java binding will be compiled.

In build-in android JNI it automatically builds .h file. If not it doesn't compile.

In outside Android JNI I do with this

javac -h . <File Name>

I suppose with some kind of git filter-branch.

@mgautierfr yes!

(I'm not even sure we need the history here, for example, a lot of commit are useless (new version xxxx))

Do you want to delete it?

Either, this repository also build libkiwix (with all its dependencies, which could be difficult) or we make kiwix-build publish build libraries for android arch and this repository "just" download, build the wrapper part and package everything.

@mgautierfr Options 2 sounds good! although I'm not familiar with kiwix-build how to do with this Edit :- @mgautierfr How much time it will take to complete?

mgautierfr commented 3 years ago

Do you want to delete it?

Maybe not delete all commits. But curating a bit by hand the history to just have the commit that "really" change something on the wrapper side. Can you add the "git filter-branch" command you've used in the PR description ?

@mgautierfr Options 2 sounds good! although I'm not familiar with kiwix-build how to do with this Edit :- @mgautierfr How much time it will take to complete?

As explain in this comment https://github.com/kiwix/kiwix-tools/issues/464#issuecomment-862420721 publish .so can be problematic. And as we will drop the the libzim wrapper in libkiwix (https://github.com/kiwix/libkiwix/issues/430), the java wrapper will have at a moment to wrap libkiwix and libzim. We will have to publish two different .so and use it in java-libkiwix (or we create a java-libzim ?) We may need to use the first option instead.


Anyway, for this PR, it would be nice to have a bit cleanup of the git history but there is nothing to say on the code. It already have been review elsewhere and we know that this first PR will not be functional and we need a base to work on.

Please, cleanup the history, document it in the PR description and we will merge.

gouri-panda commented 3 years ago

@mgautierfr I'm supposed to clean that in git folder. Thanks I'll do that.

mgautierfr commented 3 years ago

It would be better to rewrite the git history of the PR to not add the .cxx files at all instead of adding them in a commit and remove them the commit after.

mgautierfr commented 3 years ago

@gouri-panda, do you still work on this ?

gouri-panda commented 3 years ago

@mgautierfr I would happily resume this work. But I need your help to do this :)

mgautierfr commented 3 years ago

For what exactly :

If you are on the kiwix's slack, do not hesitate to ask me for help there.