search

mikofski / JGit4MATLAB

JGit wrapper for MATLAB
https://mikofski.github.io/JGit4MATLAB
BSD 2-Clause "Simplified" License
5 stars 3 forks source link

Display no assertion if JGit path is already in javasclasspath.txt #1

Closed fralik closed 11 years ago

mikofski commented 11 years ago

@fralik can you describe the issue you're having? e.g.: "I expected to see this but instead I observed this." I'm not getting why this pull request is needed.

The assertion message wording is not perfectly suited for every outcome of the various conditions when checking the javaclasspath.txt file but it should be displayed if any writes to javaclasspath.txt occur because they won't take effect unless MATLAB is restarted, which is the key part of the assertion message.

Also MATLAB would have to be restarted if you update (or revert) your jgit JAR, even if you already have it in the javaclasspath.txt because the newly downloaded JAR file won't be loaded by MATLAB unless you restart.

Thanks!

fralik commented 11 years ago

Hi!

My scenario is that I am making a silent installation system for non-tech people. One step in this installation is initialization of JGit, it is done with such command:

matlab -nodesktop -r "cd <JGit4MATLAB_path>; jgit; exit();"

It turned out that even if JGit have been installed, in nodesktop mode it doesn't appear in javaclasspath('-static'). So assertion was presented and the whole installation stopped.

I now agree, that my modification brings little sense. I think that what I should do is call JGit installation like this:

matlab -nodesktop -r "cd <JGit4MATLAB_path>; try, jgit; catch err, end; exit();"
fralik commented 11 years ago

Just to summarize: in nodesktop mode, if JGit have been already installed, there are two things:

  1. No jar file has been downloaded.
  2. No modifications to javaclasspath.txt have been made.

But the assertion is shown, cause valid is false.

mikofski commented 11 years ago

Does adding the try-catch block solve the issue? If so, I think this is the best solution, because your use case is specific. If it works and you're satisfied, then I will close the issue.

If not, then I propose adding a "silent install switch" to handle your case. If the switch is activated then the assertion is always skipped. Since validateJavaClassPath is called during instantiation of JGit I think you might have to add a file to the install folder like .noassertion which validateJavaClassPath can look for and if found, triggers the switch - that's just one idea. I'm also open to other suggestions. So if this is still an issue for you can your refactor your pull request to implement a silent install option? Also if possible please add a unit-test and supplement the documentation.

fralik commented 11 years ago

Yes, try-catch block does the thing. So, I think you can close the issue. Sorry for the disturbance..

mikofski commented 11 years ago

No disturbance at all, so glad that someone actually finds this useful! If you have any other tweaks or contributions don't hesitate to send another pull request.