Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OMERO.insight Java Application Bundler task for Mac OS X #3337

Merged
merged 14 commits into from Jan 28, 2015

Conversation

chris-allan
Copy link
Member

This pull request adds support for the Java Application Bundler (https://java.net/projects/appbundler) and its open source fork (https://bitbucket.org/infinitekind/appbundler). This bundler and corresponding launcher are intended, at least as far as we're concerned, as a transition point for Java 1.7+ on Mac OS X.

The high level zips target can be run to trigger the execution of the Java Application Bundler specific dist-osx-appbundler task as follows:

./build.py -f components/insight/build.xml zips

You will likely find that this is best run after a current build run via ./build.py clean build. The new OMERO.insight.app is then available in the following subdirectory:

components/insight/OUT/dist/appbundler

Due to the fact that execution sets the "current working directory" to the user's home directory initial testing can be performed as follows:

components/insight/OUT/dist/appbundler/OMERO.insight.app/Contents/MacOS/OMERO.insight \
    container.xml `pwd`/components/insight/OUT/app

If you have errors, launcher debugging can be turned on with an environment variable (https://java.net/projects/appbundler/lists/dev/archive/2012-11/message/2):

export JAVA_LAUNCHER_VERBOSE=1

Work will have to be done on the following before this can be merged:

  • Resolving issues with the way that OMERO.insight locates configuration files

/cc @jburel, @rleigh-dundee

@joshmoore
Copy link
Member

@chris-allan : before merging, I'd propose we force push away the jar and either use some existing maven repository or copy it to our "manual" repository.

@joshmoore
Copy link
Member

Having looked at chris-allan@92ebe4c now, perhaps maybe the way to go is to set up a job that automatically builds and pushes to http://artifacts.openmicroscopy.org /cc @sbesson

@sbesson
Copy link
Member

sbesson commented Jan 14, 2015

With regard to packaging and assuming this new app bundler will eventually be the long term solution for the Insight application under OS X, a suggestion would be to:

  • rename the old OMERO.insight e.g. as OMERO.insight_legacy (.app folder, zips etc etc)
  • bundle both legacy and non-legacy apps in the OMERO.clients (and deprecate legacy in a later release)
  • do we need the same strategy for editor and importer?
  • do we backport openGL removal on this line to reduce the build time, artifact list?

@joshmoore
Copy link
Member

Perhaps OMERO.insight_Java6 ?

@ghost
Copy link

ghost commented Jan 14, 2015

Bundling the new and legacy .app bundles is convenient, but these are quite large due to each having a complete copy of all the jars, so having them separate does have its advantages. Especially if this also includes editor and importer. [are these two not deprecated as standalone programs now Insight has both? Maybe consider removal of both if we want the legacy app in the same zip]

@sbesson
Copy link
Member

sbesson commented Jan 14, 2015

Separate OMERO.insight bundles should also be available via the downloads page. The only place where we would have doubled size are the OMERO.clients for mac. As noted above this is a transition as we expect to drop the _Java6 bundle eventually.
Would be good to have @gusferguson's opinion on this, the major questions we need to answer in terms of packaging are:

  • what is the less confusing for our users?
  • how to properly document this (PR against ome-help?)

@gusferguson
Copy link

I would really like to see the Editor and Importer disappear from the bundles - they are very confusing for users.
We do 32 and 64 bit versions for Windows - so I presume 2 versions for Mac will be much the same overhead. As long as it is clear what is what the user probably won't care about size.
On the _legacy or _Java6 issue - I feel the _Java6 is much more intuitive and self-evident than _legacy - so my vote would go there.
Documentation will be fun - just have to see what it looks like and try and work out a way to make it obvious what a particular user needs.

@jburel
Copy link
Member

jburel commented Jan 14, 2015

@gusferguson: editor has been removed for 5.1. we will have to keep it for 5.0.x

@chris-allan
Copy link
Member Author

Two .app's, OMERO.insight_Java6.app and OMERO.insight_Java7+.app, will now be included in the client ZIPs. We still have the issue of configuration file loading but that will have to be taken care of by @jburel.

@jburel
Copy link
Member

jburel commented Jan 15, 2015

great. I will start working on it tomorrow. Final adjustment on imageJ work prior to demo tomorrow.

@joshmoore
Copy link
Member

(travis kicked)

@mtbc
Copy link
Member

mtbc commented Jan 18, 2015

Ready for review again?

@jburel
Copy link
Member

jburel commented Jan 18, 2015

@mtbc: suggestions have been implemented. investigating solutions to get the config files picked up. Probably no need to list tomorrow

@mtbc
Copy link
Member

mtbc commented Jan 18, 2015

Thank you, @jburel. Good luck with it.

@jburel
Copy link
Member

jburel commented Jan 19, 2015

PR opened against this PR.
Maybe for 5.1, we may want to include (e.g. under Contents/Resources) the config directory.
This will require an adjustment to dist.xml

@jburel
Copy link
Member

jburel commented Jan 20, 2015

@mtbc: to be listed tomorrow

@ghost
Copy link

ghost commented Jan 21, 2015

Test platform: MacOSX 10.10 with Java 1.7.0_51 and Java 1.8.0_25. The _Java6 app (untested since I don't have Java6) warns than I need a Java6 VM [tested successfully by @qidane]. The new app works just fine, starts up and lets me log in, do stuff etc. I've tested the build on MacOSX and Linux, and tested the resulting Java7 artifacts, without any problems.

I would suggest removing the _Java7+ suffix (keeping _Java6). It makes the application title ugly in the menubar, and it's also what we should have as the default option. The only people who need the Java6 version are the small minority on MaxOSX 10.6. Maybe also rename the old one to _OldJava6 to highlight that?

Also, the Editor and Importer apps have no Java6/Java7 split. They need a _Java6 suffix to highlight that they won't work on older systems even if we don't provide Java7 equivalents.

There is this annoying UI glitch:

screen shot 2015-01-21 at 09 34 56

which goes away on resize. Maybe Java7 is not firing a resize event so it needs doing manually when the window is mapped?

screen shot 2015-01-21 at 09 35 09

@dominikl
Copy link
Member

Built with Java 8, ran with Java 7, didn't notice any problems.
@rleigh-dundee Looks like to be a similar problem to #3174 (just had to make another call to pack() to fix this). For reproducing the problem in #3174 I needed exactly this setup, compile with Java 8, run with Java 7. Unfortunately this is not sufficient to reproduce the problem with the server dialog, can't see that, looks normal for me.

@jburel
Copy link
Member

jburel commented Jan 21, 2015

The UI issue will be covered in another PR.

@ghost
Copy link

ghost commented Jan 21, 2015

From the build POV, looks good to merge. The cosmetic issues (application suffixes, UI) do need further work.

@joshmoore
Copy link
Member

Waiting on appbundler modifications from @sbesson.

@manics
Copy link
Member

manics commented Jan 21, 2015

$ open components/insight/OUT/dist/OMERO.insight_Java7+.app fails:
screen shot 2015-01-21 at 15 08 40

$ open components/insight/OUT/dist/OMERO.insight_Java7+.app --args container.xml $PWD/components/insight/OUT/app works

$ sw_vers
ProductName:    Mac OS X
ProductVersion: 10.9.5
BuildVersion:   13F34
$ javac -version
javac 1.8.0_31

@sbesson
Copy link
Member

sbesson commented Jan 21, 2015

Jar uploaded to our artifactory and accessible to Ivy via

<dependency org="ome" name="appbundler" rev="1.0.0">
    <artifact name="appbundler" ext="jar"/>
</dependency>

@chris-allan
Copy link
Member Author

That is expected @manics. The configuration files are not copied in to the dist directory so those .app's will not work directly from there.

@jburel
Copy link
Member

jburel commented Jan 21, 2015

The idea (cf. a previous comment) is to move the config folder under Contents/Resources.
This should have been done a while ago.

@jburel
Copy link
Member

jburel commented Jan 22, 2015

for UI issue see #3375
Fixed in develop but not rebased.

@chris-allan
Copy link
Member Author

This is going to require some more reworking. In adding the above three commits I've noticed that OMERO.editor and OMERO.importer .app's have been broken by the commits from @jburel above. This is because each of these is delineated by a command line option to specify the container.xml instance to use.

We'll need some time tomorrow to work through all of this.

@jburel
Copy link
Member

jburel commented Jan 23, 2015

@chris-allan: I will have a look either today or over the weekend

@joshmoore
Copy link
Member

On 10.10, all three Java7+ apps launch for me.

@sbesson
Copy link
Member

sbesson commented Jan 27, 2015

The thress Java7+ apps also work on 10.9.5 for me

@pwalczysko
Copy link
Member

Tested OMERO.clients.zip on ice 3.3, 3.4 and 3.5 on the octopus build.
Mac OS X 10.8
java version "1.7.0_51"
Java(TM) SE Runtime Environment (build 1.7.0_51-b13)
Java HotSpot(TM) 64-Bit Server VM (build 24.51-b03, mixed mode)

All passed fine (6 clients per ice version tested, 2x ed, 2x imp, 2x insight each).

@joshmoore
Copy link
Member

10.6 / Java_6: 🆗 (all three)
10.6 / Java_7+: double click instantly exits; no warning or error.

chris-allan and others added 14 commits January 27, 2015 05:21
This commit adds a new `dist-osx-appbundler` target to OMERO.insight and
adds it as a dependency to the OMERO.insight specific `dist` target.
The `dist` target is used by the top level `zips` target which is used
by overarching OMERO build system.
This commit switches to load the `appbundler` dependency from
Artifactory as configured by Ivy.  As Ivy resolution needs to be
performed, JAR locations have been deferred to a new `appbundler-init`
target which is now responsible for performing the task definition.
@ghost
Copy link

ghost commented Jan 27, 2015

Also tested all three Java 7 applications on 10.10 and it's working fine. If we have the split into 10.6 and 10.7+ zips, that will also remove the need for the special application suffixes.

@chris-allan
Copy link
Member Author

Force pushed this without JARs added to lib/repository and resolution of artifacts thanks to work from @sbesson.

 ______________
< Artifactory! >
 --------------
        \   ^__^
         \  (**)\_______
            (__)\       )\/\
             U  ||----w |
                ||     ||

@sbesson
Copy link
Member

sbesson commented Jan 28, 2015

Thanks guys. Merging

@sbesson
Copy link
Member

sbesson commented Jan 29, 2015

--rebased-to #3417

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants