Cache external binaries downloaded from maven in NetBeans' cache.

[jlahoda]

Currently for maven external dependencies, the build is downloading them from the internet each time it is missing in the checkout. The proposal here is to cache them in the $HOME/.hgexternalbinaries, as other binaries are cached.

(Jarda proposed caching them directly in .m2, but it is not clear what structures Maven expects there, so seems safer to cache just in .hgexternalbinaries.)

[CMoH]

Maven's "local repository" (the stuff in ~/.m2/repository) is a cache containing both remote artifacts and locally built artifacts. I am unsure what "maven external dependencies" means in this context, but usually maven caches external dependencies already.

[jlahoda]

NetBeans is not using Maven to build, and is downloading its external dependencies using an ant task. Originally, this task was only downloading from a (NetBeans) binary server (and caching the downloaded stuff in $HOME/.hgexternalcache), but recently, this was enhanced, so that it can download from Maven central as well (or use the .m2 cache). But, the current support, if it downloads from central, it won't cache the data anywhere, so after a clean, it will download the binary again.

So, basically, the proposal here is to always cache external binaries needed by NetBeans in $HOME/.hgexternalcache. This may mean some of the binaries will be in both .m2 and .hgexternalcache, but that does not seem too bad (compared to always downloading some them from the remote).

This is of course not the only possible solution, but I think it is one that can be reliable and not require too much maintenance.

[matthiasblaesing]

Looks good. The cache is a good idea - having one cache keeps the code simple and does not risk corrupting a maven repository, so I would agree with that.

If you are interested, I fixed the unittests for the module (one was introduced when the extended maven coordinate syntax was added and the others are artifacts from license header changes and missing files):

https://github.com/matthiasblaesing/incubator-netbeans/commits/unittests_nbbuild

Feel free to pull the commits into this branch, else I'll rebase the changes and will propose these in a separate PR.

[emilianbold]

The proposal here is to cache them in the $HOME/.hgexternalbinaries, as other binaries are cached.

+1

I actually assumed the existing code did that.

[CMoH]

Thank you for taking the time to explain the situation.

[geertjanw]

All questions on this have been answered -- should this be merged or should matthiasblaesing comment about re unit tests be handled first.

[jlahoda]

Matthias, do you want to first push the tests fix? No problem for me to rebase, check and merge after that. Thanks.

[javydreamercsw]

I guess I would ask what's the reason for the .hgexternalbinaries name? It's not straightforward at all. It looks like it is related to Mercurial support from the name when it looks like it isn't.

[matthiasblaesing]

@jlahoda I created a PR with the fixes #170
@javydreamercsw this might answer you question: http://wiki.netbeans.org/ExternalBinaries
The answer is basicly: it has historic reasons and my personal opinion: I would not start bikeshedding right now.

[jlahoda]

@javydreamercsw, as @matthiasblaesing says, the name is for historical reasons (NetBeans for using Mercurial for a decade, and this was a local cache used by the build during that time). I think that we will change this in some way eventually (maybe we can avoid using a private cache at all; use Ivy; rename the directory to something else, etc.), but I concur with Matthias that I'd start discussing it right now.

Edit: I'd not start discussing it right now.

[javydreamercsw]

It just sounded like a new thing. If it's not I agree there's no need to change it.

[jlahoda]

I've rebased the patch on the current master. If there are no objects, I'd like to merge to master tonight or tomorrow.

[matthiasblaesing]

@jlahoda I see nothing that would prevent merging. Some things might be worth looking into later:

• use try-with-resource (in many cases this leads to better readability)
• eliminate firstProblem from doDownload

[geertjanw]

@jlahoda, looks good to me.

[geertjanw]

@jlahoda, will you merge this, would do it myself, but you said above you'd do it and since this is yours and you may have other ideas here, would rather leave it to you to merge.

[javydreamercsw]

@geertjanw I want to raise a concern. My SW quality engineer senses went crazy when I saw this merged with no check passed.

This is just a bad practice and can be configured to be prevented at all.