Maven caching

[mahesh-hegde]

This introduces some basic caching of maven dependency download tasks. While maven local caches the dependencies, invoking maven itself can slow down jnigen significantly.

This PR implements a simple caching mechanism based on file last modified timestamps. We write a JSON file containing maven_downloads spec. If any file in target dir has changed, or the spec itself has changed, we reinvoke maven. Otherwise maven command is not invoked.

The cache record can be force-invalidated by passing --invalidate-caches on command line.

Also, when we are at it: should we change default maven dependency download folder to somewhere in .dart_tool?

closes: dart-lang/native#742

[coveralls]

Pull Request Test Coverage Report for Build 4538727886

• 62 of 91 (68.13%) changed or added relevant lines in 4 files are covered.
• 36 unchanged lines in 2 files lost coverage.
• Overall coverage decreased (-2.09%) to 84.817%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
jnigen/lib/src/config/yaml_reader.dart	12	13	92.31%
jnigen/lib/src/generate_bindings.dart	1	4	25.0%
jnigen/lib/src/tools/maven_tools.dart	48	73	65.75%

Files with Coverage Reduction	New Missed Lines	%
jnigen/lib/src/config/yaml_reader.dart	1	81.82%
jnigen/lib/src/tools/maven_tools.dart	35	46.96%

Totals
Change from base Build 4505284851:	-2.09%
Covered Lines:	1972
Relevant Lines:	2325

---

💛 - Coveralls

[HosseinYousefi]

LGTM 🚀 Thanks :)

[HosseinYousefi]

Also, when we are at it: should we change default maven dependency download folder to somewhere in .dart_tool?

I think it's not necessary to jam everything in there.

[mahesh-hegde]

Higher level design question: should the list of files be written to the record, just in case we merge a change which changes layout of these directories?

Edit: I think it's better to add a cacheVersion private field, so that when we change these things, we can ensure invalidation happens.

[HosseinYousefi]

Edit: I think it's better to add a cacheVersion private field, so that when we change these things, we can ensure invalidation happens.

Good idea.

Related: Something like a dart run jnigen:clean would be nice which cleans all the cache/generated files.

[mahesh-hegde]

Related: Something like a dart run jnigen:clean would be nice which cleans all the cache/generated files.

Should this take a config or clean all folders with a cache record file?

[HosseinYousefi]

I think in this specific case getting a config is not necessary.

[mahesh-hegde]

I think in this specific case getting a config is not necessary.

User can change the destination to where downloaded files are stored. (We actually use this functionality in integration test with jackson-core library.)

[HosseinYousefi]

In what case would you not want to remove everything with jnigen:clean and instead remove only the files specified in your config?

[mahesh-hegde]

In what case would you not want to remove everything with jnigen:clean and instead remove only the files specified in your config?

Right. Can this be named jnigen:clean-caches?

[HosseinYousefi]

Right. Can this be named jnigen:clean-caches?

Sure, it's a bit more verbose but a bit more clear. Why not go with jnigen:clean, because it might imply that it also removes the generated bindings or built files after jnigen:setup?

[HosseinYousefi]

That's another issue though, I think we can create another issue and discuss it further there. dart-lang/native#659

[mahesh-hegde]

collect coverage step is hung infinitely

Attempt:3849 waiting for isolate main to check in
Attempt:3850 waiting for isolate main to check in
Attempt:3851 waiting for isolate main to check in
Attempt:3852 waiting for isolate main to check in
Attempt:3853 waiting for isolate main to check in
Attempt:3854 waiting for isolate main to check in
Attempt:3855 waiting for isolate main to check in
Attempt:3856 waiting for isolate main to check in
Attempt:3857 waiting for isolate main to check in
Attempt:3858 waiting for isolate main to check in
Attempt:3859 waiting for isolate main to check in
Attempt:3860 waiting for isolate main to check in
Attempt:3861 waiting for isolate main to check in
Attempt:3862 waiting for isolate main to check in
Attempt:3863 waiting for isolate main to check in
Attempt:3864 waiting for isolate main to check in
Attempt:3865 waiting for isolate main to check in
Attempt:3866 waiting for isolate main to check in
Attempt:3867 waiting for isolate main to check in
Attempt:3868 waiting for isolate main to check in
Attempt:3869 waiting for isolate main to check in
Attempt:3870 waiting for isolate main to check in
Attempt:3871 waiting for isolate main to check in
Attempt:3872 waiting for isolate main to check in
Attempt:3873 waiting for isolate main to check in
Attempt:3874 waiting for isolate main to check in
Attempt:3875 waiting for isolate main to check in
Attempt:3876 waiting for isolate main to check in
Attempt:3877 waiting for isolate main to check in
Attempt:3878 waiting for isolate main to check in
Attempt:3879 waiting for isolate main to check in
Attempt:3880 waiting for isolate main to check in
Attempt:3881 waiting for isolate main to check in
Attempt:3882 waiting for isolate main to check in
Attempt:3883 waiting for isolate main to check in
Attempt:3884 waiting for isolate main to check in
Attempt:3885 waiting for isolate main to check in
Attempt:3886 waiting for isolate main to check in
Attempt:3887 waiting for isolate main to check in
Attempt:3888 waiting for isolate main to check in
Attempt:3889 waiting for isolate main to check in

Interesting

[HosseinYousefi]

One of the tests randomly gets timed out. You can increase it's time limit to 1.5 or 2x for it not to fail again.

[mahesh-hegde]

This PR seems to be causing some problems. Let's hold this for a while.

Edit: I might have mistakenedly committed a bad build.gradle file. I will take a look this evening.

Holding it for now

[HosseinYousefi]

Let's hold this for a while.

Is this ready now?

[mahesh-hegde]

The CI error was a stub build.gradle accidentally being committed.

But I also suspect this can be less stable than expected, because cache invalidation is a classic hard topic. Merging it or not is your choice I guess.

[mahesh-hegde]

Need to solve some merge conflicts and implement clean_caches command. Marking it as draft.

[HosseinYousefi]

Closing this for now.