Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
    Jan Eglinger
    @imagejan
    yes, you accounted for that in the ij1-builds right? So ij1-patcher isn’t yet aware of that?
    Tobias Pietzsch
    @tpietzsch
    to which repository should I add the test?
    Jan Eglinger
    @imagejan
    @tpietzsch the melting pot script is in scijava-scripts, but that’s not a maven project. If not a new component, maybe fiji/fiji is suitable?
    Or directly in ij1-patcher?
    Tobias Pietzsch
    @tpietzsch
    ok
    or maybe scijava-common?
    because that is where Context lives?
    Jan Eglinger
    @imagejan

    @tpietzsch wrote:

    I get the exception if I run that CreateContext class

    Another problem might be that this exception is only thrown when run on Mac, right? So the usual CI build won’t catch it…

    scijava-common doesn’t know anything about imagej-legacy or ij1-patcher…
    and it contains a context creation test already, but only with the limited context of SciJava.
    so we need a “high-level” context creation test as well
    Mark Hiner
    @hinerm
    sorry, between meetings
    the problem is in ij1-patcher I believe
    in the handleMacAdapter
    I fixed ij1-builds but not ij1-patcher for the package change
    that should be all
    Mark Hiner
    @hinerm

    Another problem might be that this exception is only thrown when run on Mac, right? So the usual CI build won’t catch it…

    This is correct

    so we need a “high-level” context creation test as well

    This should implicitly happen in the melting pot. But the problem we ran into here isn't specifically about context creation. Any test in imagej-legacy or downstream I would expect to hit this exception.. but as you noted, nothing is testing on mac

    to which repository should I add the test?

    @tpietzsch I'm sorry, I just wanted you to add the test to the bdv-fiji branch to complete the MCVE. But since we identified the problem and have now fixed it (imagej/ij1-patcher@0f3c057) I don't think we need to add a test anywhere

    Mark Hiner
    @hinerm
    @tpietzsch would you mind checking out the latest https://github.com/imagej/ij1-patcher and running mvn clean test to see if it works on mac?
    and maybe also repeating for imagej/ij1-patcher@d45f936 ? which I assume will fail because it lacks the mac adapter
    Tobias Pietzsch
    @tpietzsch
    yes to both
    master goes through
    imagej/ij1-patcher@d45f936 fails
    Mark Hiner
    @hinerm
    :+1: thank you!
    Tobias Pietzsch
    @tpietzsch
    The weird thing is that Java 8 complains about the MacAdapter thing
    Java 11 does not
    https://gist.github.com/tpietzsch/dc728e693f24a5a8d1453e39ce341e3b
    has the output from both java versions (on imagej/ij1-patcher@d45f936)
    For Java 11, some tests go through, e.g. 
    [INFO] Running net.imagej.patcher.LegacyHooksTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.713 s - in net.imagej.patcher.LegacyHooksTest
    For Java 8, the same test fails 
    [INFO] Running net.imagej.patcher.LegacyHooksTest
java.lang.RuntimeException: Found incompatible ij.IJ class
    at net.imagej.patcher.LegacyEnvironment.initialize(LegacyEnvironment.java:112)
    at net.imagej.patcher.LegacyEnvironment.applyPatches(LegacyEnvironment.java:494)
...
Caused by: java.lang.IllegalArgumentException: No such class: MacAdapter
    at net.imagej.patcher.CodeHacker.getClass(CodeHacker.java:878)
    at net.imagej.patcher.CodeHacker.getMethod(CodeHacker.java:904)
    at net.imagej.patcher.CodeHacker.getBehavior(CodeHacker.java:894)
    at net.imagej.patcher.CodeHacker.insertAtTopOfMethod(CodeHacker.java:157)
    ... 35 more
Caused by: javassist.NotFoundException: MacAdapter
    at javassist.ClassPool.get(ClassPool.java:430)
    at net.imagej.patcher.CodeHacker.getClass(CodeHacker.java:873)
    ... 38 more
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.287 s <<< FAILURE! - in net.imagej.patcher.LegacyHooksTest
    Super weird
    Mark Hiner
    @hinerm
    @tpietzsch something about MacAdapter is java-8 specific IIRC
    but I can't find the code that suggests that
    anyway I'm releasing a new ij1-patcher and this will all be happier in the next pom-scijava
    currently waiting on trakem2/TrakEM2#28 and PreibischLab/multiview-reconstruction#32
    Jan Eglinger
    @imagejan
    :+1: thanks for all of this, @hinerm !
    Mark Hiner
    @hinerm
    hey you fixed it first @imagejan! Sorry I missed your PR
    @tpietzsch I can bump BDV-core to 10.2.0.. are there any other BDV versions you want updated?
    Jan Eglinger
    @imagejan

    no need to apologise! If I remember correctly, Curtis tried to keep ij1-patcher compatible to as many versions of ij.jar as possible, i.e. also for older versions. I think that’s not really tenable with the latest package changes of MacAdapter and other changes in IJ1. But I think the current ij1-patcher at least doesn’t throw errors when used in non-headless mode with older versions of ij.jar.

    Also, we should try to catch up with the newer versions up to 1.53g and soon h :-/

    Mark Hiner
    @hinerm

    Also, we should try to catch up with the newer versions up to 1.53g and soon h :-/

    :skull: :skull: :skull: :ghost:

    Jan Eglinger
    @imagejan
    :laughing:
    Tobias Pietzsch
    @tpietzsch
    @hinerm yes bigdataviewer-core-10.2.0
    bigdataviewer-vistools-1.0.0-beta-27
    Mark Hiner
    @hinerm
    @tpietzsch :+1: I'll run a melting pot with those right now
    Tobias Pietzsch
    @tpietzsch
    and I'm releasing bigdataviewer_fiji-6.2.1 now
    that would be also good, if you can still get it in
    (but this is not that important, nothing should depend on bigdataviewer_fiji)