search

imagej / ij1-patcher

Extension points for ImageJ via runtime patching to support (limited) headless operation and ImageJ2's legacy layer
BSD 2-Clause "Simplified" License
5 stars 6 forks source link

Respect IJ_PREFS_DIR environment variable #40

Closed y3nr1ng closed 7 years ago

y3nr1ng commented 7 years ago

Fix both the injection method and the injection location :)

y3nr1ng commented 7 years ago

Uh.. awkward, CI shows build failed, but seems like it's not relevant to what I've modified. Can anyone help me confirmed this?

ctrueden commented 7 years ago

@liuyenting Thank you very much for digging into this code! I am rather shocked, actually, since this code thoroughly scares nearly everyone in the ImageJ community. 😄

Sorry about the failing CI. I very recently switched the master branch builds for all ImageJ/SciJava/etc. components from the ImageJ Jenkins to Travis CI, and this is one of the few components which had problems with the transition. If mvn clean install is building OK on your system, then I would not worry about Travis CI here at this moment.

I was wondering, though, if you would feel comfortable adding a unit test which catches this bug, and starts passing due to your fixes? That would be very helpful for validating that the solution is correct, and preventing future regressions.

y3nr1ng commented 7 years ago

@ctrueden The mvn clean install instruction yields a perfect result on my side ;)

After glimpsing through the available test, I doubt the bug is catchable, so to speak. It's something matters to behavior only, and the code path is completely isolated (the field modification part, no one reused overwriteWriteField). What I can think of right now is setup the IJ_PREFS_DIR and execute macro in headless mode, trigger a preference save and compare the location of the preference file and the environment variable. I can try to work out something like that if you believe it worth a shot. (But I've come across numerous bug I'd like to fix first, working on cluster with Fiji can trigger lots of unknown factors :( )

Glad to be of assistance :) As a side note.. I believe I'm facing issues from this post, can you point me to a possible direction? The issue tracker returns 500, not quite helping D:

ctrueden commented 7 years ago

Let's not bother with the test. We could do it as an integration test using Maven Invoker Plugin (you will sometimes see such integration tests in the src/it directory of projects), but it is more trouble than it's worth here.

As a side note.. I believe I'm facing issues from this post, can you point me to a possible direction? The issue tracker returns 500, not quite helping D:

Argh. Thanks for the heads up about the 500. I am investigating now; however, that is a very old blog post which has since been resolved: AFAIK, you should be able to run as many simultaneous ImageJ2 contexts as you like. Unless you are trying to include imagej-legacy, in which case you can only have one at a time—just call context.dispose() on the old one before making a new one.

Could you file a new issue, maybe in imagej/imagej unless you can pinpoint the problem more specifically, describing what you are trying to do, and what goes wrong?

Thanks again for the patch! :beers:

y3nr1ng commented 7 years ago

Hmm.. Ok, I'll look into it more closely, after isolating the userPrefs through -D and tweaking Lustre file lock paramters, my macro runs fine for now. Thanks for the heads up in imagej/imagej :)