search

robmaz / distmap

Sequence alignment on Hadoop
0 stars 1 forks source link

Move bundled jars into shared/java #41

Closed robmaz closed 6 years ago

robmaz commented 6 years ago

While I agree in principle with the sentiment that a jvm is a type of machine and therefore libexec/java should be the proper folder for jars, I cannot find such a folder on either MacOS or Linux, where jars always seem to go into share/java.

robmaz commented 6 years ago

Well, we'll certainly find out where distmap uses all this stuff after renaming. I'll fix the execarch stuff or whatever else breaks then. Appending some string for old versions is necessary because it will otherwise lead to installation conflicts, e.g., homebrew puts ReadTools.jar and picard.jar also in share/java. We should proceed under the assumption that those will always be reasonably up-to-date, possibly externally maintained jars, (and should probably check their version before using them.)

I'll send another commit where I rebase and add the proper version number to the old picard.jar.

R.

2017-12-04 14:13 GMT+01:00 Daniel Gómez-Sánchez notifications@github.com:

@magicDGS requested changes on this pull request.

Let's move on with the folder structure and maintain the share/java/ folder as you suggest. Two points for this PR and it will be ready to go:

  1. You should rebase into the latest master to remove completely the MarkDuplicates.jar and ViewSam.jar (or remove them in this PR). After

    37 https://github.com/robmaz/distmap/pull/37, they should not

    longer appear in the repo because they aren't used.

  2. Why the re-name to -old for the picard tools? In addition, I guess that distmap in its current state defaults to the name without appended version (such as ReadTools.jar for trimming in the cluster).

My suggestion for solve 2 is to add a README.md to the share/java folder, with a list with the versions of the tools. I can add it if you want to be formatted.

By the way, picard.jarversion is 2.6.0-SNAPSHOT, and can be retrieved by running any tool with the --version option. For the others, better if we accept as soon as possible #39 https://github.com/robmaz/distmap/pull/39 for removal of things that cannot be track to a version. But after the changes here.

An a final comment: I think that DistMap in the current state archives everything in an execarch zip to distribute in Hadoop, which contains either the provided copies by the command line or the stored files in executables. I think that this change might break that behaviour, but I am not sure where it is doing that. We should be also careful while moving the rest of the executables to libexec

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/robmaz/distmap/pull/41#pullrequestreview-80834795, or mute the thread https://github.com/notifications/unsubscribe-auth/Ad_FfJyYn27seYdrBr5ukawsiuoXnlDQks5s8-_rgaJpZM4Q0bO8 .

robmaz commented 6 years ago

The problem I see is that we cannot (and should not) rely on people reading docs, and the -distmap-BRANDNEW names will look like the jars are new and well integrated with the distmap distribution, so the naming would suggest the opposite of what it should suggest. If they have to type 0.1 version numbers or suffixes like "old" (or mabe even better, "outdated") on the command line, they are maybe more likely to think twice about using them.

What we could do, however, is to make a share/java/distmap subfolder and put them in there.

magicDGS commented 6 years ago

Like that looks perfect to avoid misunderstandings - the distmap folder indicates that it is for distmap usage and the DONTUSE tag for aware everyone that they shouldn't use them (if they do after reading the name, it's their own fault).

Merging now 👍