#1259 Allow a local kiwix-lib aar to be used

[mhutti1]

Work on #1259

[macgills]

I would get rid of the ! and flip the branches

[mhutti1]

Maybe, the top branch will be hit 99% of the time though, so I would personally prefer it to be that way round.

[macgills]

kiwix/kiwix-build#346 (comment) sounds like we are not doing this anymore.

[mhutti1]

What I am using this for is to test kiwix-lib changes directly. The workflow is:

1. symlink libs directory to the output of kiwix-build
2. Build aar using kiwix-build
3. Build and run the app.
4. Repeat steps 2-4 until you fix whatever bug you are trying to fix.

Turnaround time with this is about a minute.

[macgills]

With unit tests turnaround is in seconds if not milliseconds. We should have this discussion here

[mhutti1]

Sure unit tests would help and might reduce the need for this but we don't have them now. This is a small change that can be implemented now and allows other bugs such as #735 to be solved now. Simply open the effected file and debug using a combination of breakpoints and print statements between c++ recompiles.

[macgills]

will this be necessary? I think transitive = true should work

[mhutti1]

I did try getting relinker inside the aar but was struggling. (and we would only want to do this for debug builds). I think this is probably an acceptable solution instead.

[mhutti1]

I am going to merge this into the fix branch and then try get the tests passing on that.

[macgills]

Why when you could just do it locally? Also what tests?

[mhutti1]

Travis is failing showing 0 tests. The tests don't do this for me locally.

[macgills]

We had that conversation on slack, you haven't added the aar to the project in git

[macgills]

if you were to add this functionality not off a failing branch then tests would pass but the branch you based off of is failing and will continue to fail because we need an update from kiwixlib

[mhutti1]

Yes I know, which is why I merged this so that once a new kiwix-lib is released (It has been fixed but not released yet) we can simply use that branch to merge in quickly.

[macgills]

Once it has been released the pre-existing branch would have worked without this update (after incrementing the version number).
I am still not a fan of this functionality and we have made no commitment to do it.

[mhutti1]

So I have been working on getting the stackoverflow ZIM files working the last 2 days and this change has been essential to doing that. We don't have a JNI testing system yet so I don't see any reason why we can't add this to develop until such a system exists.

[macgills]

It is fine to have this change local but I don't see the necessity for the change to happen to dev. Ideally we have a better system in place for the next ticket

If we must see it I'd prefer to see

impl real_lib
// uncomment these lines to use local copy, also comment out real lib TODO Delete this
//impl relink
//impl aar

as it doesn't just integrate it into our project as a normal way of working

[mhutti1]

We have had the current style system for over a year now and it works well. Uncommenting could also work but I really don't see the reason we can't have a check to see if the user has created a file and then use that file to override our import if they have.

It makes debugging simpler and I really can't see a downside.

[macgills]

it is active code in the build system, I have never seen a conditional dependency before so for me it was quite weird to see and interrupted my reading flow (the original kiwixlib one).
I don't necessarily want this to be simple because I want it to stick out as unusual and a reason to remove it and use a proper way.