search

imagej / imagej-maven-plugin

[OBSOLETE] All goals migrated to scijava/scijava-maven-plugin
Other
4 stars 6 forks source link

Adds possibility to define an imageJSubDirectoryProperty #17

Closed lacan closed 7 years ago

lacan commented 7 years ago

This would enable userrs to have their plugins copied to a subfolder of imageJ's jars or plugins.

lacan commented 7 years ago

the error message is not too informative, so I'll wait for some feedback :)

stelfrich commented 7 years ago

Thanks for going ahead with this @lacan! :+1:

I got hooked on the Maven magic and extended your commit on another branch (copy-jar-subdirectory) to work properly. It would be awesome if you could test the version from the branch (mind that I have renamed the property to imagej.app.subdirectory).

Once #19 is merged I can rebase that branch onto master and file a new PR (if that's ok with you?).

lacan commented 7 years ago

Awesome. Quick question while it's fresh in my mind.

Does the artifact finder for the dependencies work as well when a dependency is already installed but in a custom subfolder? I was getting stuck in this one for a bit...

stelfrich commented 7 years ago

Does the artifact finder for the dependencies work as well when a dependency is already installed but in a custom subfolder?

That's currently not supported. As far as I can tell, there are two lookups:

1) does a file with the same name at the location you are trying to install to already exist 2) If delete.other.versions is true, check in the folder you are trying to install to if there is a file that has the same name as the artifact but a different version number (that's not GAV-based but works with regular expressions of filenames)

We could extend lookup 2 to support the case you are describing by looking for other versions in the imagej.app.directory and subfolders instead of just the folder you are copying to. That, however, would make installing a lot slower. So I am not sure if it is worth the hassle. Why don't you open an issue for that one so we can keep track of it and get everyone's opinion?

lacan commented 7 years ago

We could extend lookup 2 to support the case you are describing by looking for other versions in the imagej.app.directory and subfolders instead of just the folder you are copying to.

Exactly what I thought, however do you really think it will be a lot slower? I can imagine that for big projects that rely on a hundred dependencies. But for a simple plugin where Maven Build takes 3-5 seconds, checking the (rather small) folder hierarchy of the jars and plugins folders should be alright?

I'll open a new issue with that one. I'm guessing on this repo, or do you mean on the forum?

stelfrich commented 7 years ago

But for a simple plugin where Maven Build takes 3-5 seconds

The runtime doesn't depend on the project itself but the size of the ImageJ/Fiji instance you are installing to (since that's the folder that has to be iterated to find "other versions").

I'll open a new issue with that one. I'm guessing on this repo, or do you mean on the forum?

Here in the repo...

ctrueden commented 7 years ago

FWIW, my preference would be to always keep all libraries in the base folder, instead of these subfolders. It is a lot of work to support subfolders, and it mainly just makes things more fragile as it becomes easier to have multiple versions of libraries in different subfolders. Whereas if everything goes into the same jars folder, then it is more obvious when two versions of something are installed simultaneously. I regret putting the bio-formats stuff into its own subfolder, even though it made things easier in certain ways, because it causes issues and greater complexity in others.

lacan commented 7 years ago

@ctrueden I fully understand your point. However, I would like to make you regret putting bio-formats into subfolders a bit less, if at all possible. Often, because new bio-formats versions break older functionality on release, we need to rollback the libraries. For one machine it is fine to find the SNAPSHOTS, or whatever version is there, remove it and donwnload every package at the right version again. But reverting bioformats on 10 machines can be quickly tedious.

Having these in separate folders is extremely useful to us, much in the same way that having some form of logic to the plugins / jars folder helps us make sense of what's in there in a more human way.

ctrueden commented 7 years ago

The merge of #21 wrecked this patch, so I redid it as #22. It didn't look like the changes to InstallArtifactMojo really did anything, so I didn't move those over. But I may be wrong about that; I don't really understand this code very well. Anyway, I hope #22 serves as a starting point for you guys to finish this feature.