clojure-android / lein-droid

A Leiningen plugin for building Clojure/Android projects
Eclipse Public License 1.0
645 stars 56 forks source link

Add extract-native-libraries-from-jars? option #82

Closed AdamClements closed 10 years ago

AdamClements commented 10 years ago

This searches uberjars on the classpath for /native/linux//.so native android libraries and includes those libraries in the build, according to the file layout at https://github.com/swannodette/native-deps.

Should not affect existing native-libs handling at all but offers an alternative to package native binaries with jars for android.

Current issues:

If you want to suggest/review any changes I'd be happy to tweak a little more.

alexander-yakushev commented 10 years ago

Hello Adam,

I'm all for adding this feature. As for the issues: a) extra 0.5 seconds is OK to sacrifice; b) figuring out a caching mechanism to avoid copies every time is enough trouble to just let it copy; c) for the namespace you are right, it doesn't seem to fit into sdk completely. Could you separate all utility functions into a new namespace (something like native.clj or anything better you might think of)?

Also can you please trim the code to fit into 80 chars per line? I try to honour this limit.

Finally, it would be a good idea to eventually add the boilerplate to Leiningen itself, if this way of locating libs is standard. I may try to ask on the #leiningen list.

AdamClements commented 10 years ago

Okay, I have rejigged it into its own namespace and added a cache in the target folder so it'll get blown away with a lein clean. The cache looks at full path of the jar to say whether its dealt with it before and if it has, skips the copy step as well as the jar scanning step, this drops the whole cost to 0.5seconds on the first run and negligible thereafter.

The only edge case I can think of is that you might need to clean between snapshots if your snapshot has updated... don't know what it chooses for snapshot paths, if there's a unique part of the file name then that's not even a problem.

Now it has caching, I wonder if this should be on by default and do away with the option? Your call.

alexander-yakushev commented 10 years ago

All the rest seems OK as long as it works.

One more thing. After you make the final changes, could you please squash all suggested commits into one? To avoid creating abother pull request you can do git rewrite -i on your branch, squash the commits and then forcefully push with git push -f.

alexander-yakushev commented 10 years ago

Hey, Phil just told me there is leiningen.core.classpath/extract-native-deps function that should do the same job. Can we leverage that instead?

AdamClements commented 10 years ago

Oh. Yeah, that'd be much easier, all we actually need to do is add target/native/linux to the native-dependencies vector if it exists... Sort of annoying but ah well! I might just delete this pull request, start again.