Skip to content

Conversation

@oyarzun
Copy link
Contributor

@oyarzun oyarzun commented Mar 11, 2020

This pull request fixes the NetBean RCP installer on macOS with Java 11 since com.apple.eawt classes were removed with JEP 272

This implementation is modeled after the applemenu implementation and addresses @matthiasblaesing comment #1929 (comment) that it needs to work with both Java 8 & 11 and could be accomplished using reflection.

@ebarboni ebarboni requested a review from arusinha May 4, 2020 08:33
public class InitializeMacJDK8 {
public static void initialize(SwingFrameContainer frameContainer)
{
final Application application = Application.getApplication();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this going to throw java.lang.NoClassDefFoundError in jdk9+ runtime?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is the same pattern used in the applemenu module.

NbApplicationAdapterJDK8.java
NbApplicationAdapterJDK9.java

            if (!installAdapter("org.netbeans.modules.applemenu.NbApplicationAdapterJDK8")) {   // NOI18N
                // JDK 8 failed, try JDK 9
                installAdapter("org.netbeans.modules.applemenu.NbApplicationAdapterJDK9");      // NOI18N
            }

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you enclose the code snippet with try-catch as it will throw the error on jdk9+ runtime

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The try-catch is in the initializeMacJDK method

if(application == null) {
// e.g. running OpenJDK port via X11 on Mac OS X
return;
if (!initializeMacJDK("org.netbeans.installer.wizard.containers.initializeMacJDK8")) { // NOI18N
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is not much significant difference in look & feel UI , we can then use Desktop class in JDk8 also instead of Application class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JDK8 Desktop class does not support a QuitHandler.

-->

<project name="NBI_Engine" default="default" basedir=".">
<project name="nbi/engine" default="default" basedir=".">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't change the project name. We tried building NB installer for 12.0 after merging your change set, It failed with below error
BUILD FAILED
netbeans/nbbuild/installer/infra/build/build.xml:133: The following error occurred while executing this line:
nbbuild/installer/infra/build/nbi_all/nbi/engine/build.xml:47: Cannot find /Users/sarillamohanarao/Mohan/Code_Repository/Github_Repository/Installer_Maven_Issue/netbeans/nbbuild/installer/infra/build/nbi_all/nbbuild/templates/projectized.xml imported from /Users/sarillamohanarao/Mohan/Code_Repository/Github_Repository/Installer_Maven_Issue/netbeans/nbbuild/installer/infra/build/nbi_all/nbi/engine/build.xml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to change the project name to get projectized.xml to work. This was needed to download the stubs to compile InitializeMacJDK9 with JDK8.

The build error above is not due to changing the project name, instead it is due to the fact that the relative path does not work when building the NB installer.

<import file="../../nbbuild/templates/projectized.xml"/>

Do you know how the project can download the stubs applemenu-external-desktop-classes-8.2.zip without using projectized.xml?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oyarzun , indeed import of projectized.xml is causing the issue. I currently don't have solution for this issue. Is PR is urgent or we can wait for 12.1, as changing something in NB installer script is risky at this moment as we are near to NB 12.0 release.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arusinha & @ebarboni It can wait until 12.1 since this has been broken since version 10. Plus there are ways around it, create one installer for Java 8 and another for Java 11+.

I do not use this feature and only started looking at it since we switched our RCPs builds to use maven. In maven there is no goal to build a macOS application like there is with ant.

https://issues.apache.org/jira/browse/NETBEANSINFRA-37

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arusinha

I've found a work around that allows the NB installer to be built. Since it is not used for macOS we don't need the mac specific classes that are loaded by reflection. Create a copy of the original nbi/engine build.xml in nbbuild/installer/infra/build/build-nbi-engine.xml then patch it's build.xml as follows.

diff --git a/nbbuild/installer/infra/build/build.xml b/nbbuild/installer/infra/build/build.xml
index 4861fc7092..94134b2634 100644
--- a/nbbuild/installer/infra/build/build.xml
+++ b/nbbuild/installer/infra/build/build.xml
@@ -71,8 +71,13 @@
                 <exclude name="infra/server/**/build/**/*.*"/>
                 <exclude name="infra/server/**/dist/**/*.*"/>
                 <exclude name="infra/server/**/private/**/*.*"/>
+                <!-- macOS uses native installer so ignore initialize macOS classes loaded with reflection -->
+                <exclude name="engine/src/org/netbeans/installer/wizard/containers/InitializeMacJDK8.java"/>
+                <exclude name="engine/src/org/netbeans/installer/wizard/containers/InitializeMacJDK9.java"/>
             </fileset>
         </copy>
+        <!--replace nbi/engine build.xml with one that does not build the initialize macOS classes -->
+        <copy file="build-nbi-engine.xml" tofile="${nbi.core.dir}/engine/build.xml" overwrite="true"/>
         <copy todir="${nbi.netbeans.dir}">
             <fileset dir="${hg.root}/installer">
                 <exclude name="infra/server/**/build/**/*.*"/>

Copy link

@arusinha arusinha May 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oyarzun , separate NB installer is released each for Linux/Mac/Windows.
Conditional import can come handy.
IS_NB_INSTALLER has to be set

Suggested change
<project name="nbi/engine" default="default" basedir=".">
<condition property="relative_path" value=".../../../" else="../../">
<isset property="IS_NB_INSTALLER"/>
</condition>
<import file="${relative_path}/nbbuild/templates/projectized.xml"/>"

IS_NB_INSTALLER property needs to be set in nbbuild/installer/infra/build/build.xml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arusinha I like the idea of a property, but I tried changing the relative path. I ran into the same problem with projectized.xml which made me change the project name. The new path causes this check to fail when building the NB installer.

    <property name="ant.file.1" location="${ant.file}"/>
    <property name="ant.file.2" location="${nb_all}/${ant.project.name}/build.xml"/>
    <fail message="Please keep the Ant project name the same as the project's directory name: ${ant.file.1} vs. ${ant.file.2}">
        <condition>
            <not>
                <equals arg1="${ant.file.1}" arg2="${ant.file.2}"/>
            </not>
        </condition>
    </fail>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oyarzun I am working on some other things, will revisit this issue once those are done

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arusinha when you have time to circle back to this, I have a new branch that has this fix working without using projectized.xml.

oyarzun@cba705b

Let me know if you want me to submit a new PR.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

John is now managing mac NB Installer. @mcdonnell-john request you to review the changes.

public class InitializeMacJDK8 {
public static void initialize(SwingFrameContainer frameContainer)
{
final Application application = Application.getApplication();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you enclose the code snippet with try-catch as it will throw the error on jdk9+ runtime

if(application == null) {
// e.g. running OpenJDK port via X11 on Mac OS X
return;
if (!initializeMacJDK("org.netbeans.installer.wizard.containers.initializeMacJDK8")) { // NOI18N
Copy link

@arusinha arusinha May 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really needs reflection to call the classes?
you can change InitializeMacJDK8.initialize() to return true/false

if(!InitializeMacJDK8.initialize())
InitializeMacJDK9.initialize()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried changing the code to not use reflection and wrapping the initialize in a try catch.

public class InitializeMacJDK8 {
    public static boolean initialize(SwingFrameContainer frameContainer)
    {
        try {
            final Application application = Application.getApplication();
            if(application == null) {
                // e.g. running OpenJDK port via X11 on Mac OS X
                return true;
            }
            application.removeAboutMenuItem();
            application.removePreferencesMenuItem();
            application.addApplicationListener(new ApplicationAdapter() {

                @Override
                public void handleQuit(ApplicationEvent event) {
                    frameContainer.cancelContainer();
                }
            });

            return true;
        }
        catch (NoClassDefFoundError e) {
            // we are running Java 9+
        }
        return false;
    }
}

This did not work when running the installer on Java 11

2020-05-23 15:46:45.507]:     An unexpected exception happened in thread main
[2020-05-23 15:46:45.507]:     java.lang.NoClassDefFoundError: com/apple/eawt/ApplicationListener
[2020-05-23 15:46:45.508]:     	at org.netbeans.installer.wizard.containers.SwingFrameContainer.initializeMacOS(SwingFrameContainer.java:422)
[2020-05-23 15:46:45.508]:     	at org.netbeans.installer.wizard.containers.SwingFrameContainer.initComponents(SwingFrameContainer.java:355)
[2020-05-23 15:46:45.508]:     	at org.netbeans.installer.wizard.containers.SwingFrameContainer.<init>(SwingFrameContainer.java:165)
[2020-05-23 15:46:45.508]:     	at org.netbeans.installer.wizard.Wizard.newWizardContainer(Wizard.java:479)
[2020-05-23 15:46:45.508]:     	at org.netbeans.installer.wizard.Wizard.open(Wizard.java:509)
[2020-05-23 15:46:45.508]:     	at org.netbeans.installer.Installer.start(Installer.java:124)
[2020-05-23 15:46:45.508]:     	at org.netbeans.installer.Installer.main(Installer.java:61)
[2020-05-23 15:46:45.508]:     Caused by: java.lang.ClassNotFoundException: com.apple.eawt.ApplicationListener
[2020-05-23 15:46:45.508]:     	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
[2020-05-23 15:46:45.508]:     	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
[2020-05-23 15:46:45.508]:     	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
[2020-05-23 15:46:45.508]:     	... 7 more
[2020-05-23 15:46:45.508]:     ... show message dialog
[2020-05-23 15:46:45.508]:         title: Critical Error
[2020-05-23 15:46:45.508]:         message: An unexpected exception happened in thread main
[2020-05-23 15:46:45.508]:
[2020-05-23 15:46:45.509]:         Exception:
[2020-05-23 15:46:45.509]:           java.lang.NoClassDefFoundError:
[2020-05-23 15:46:45.509]:           com/apple/eawt/ApplicationListener

@ebarboni
Copy link
Contributor

@arusinha @oyarzun can we merge this ? I cannot check that (no macos)

@arusinha
Copy link

@ebarboni , there are some comments which needs to be resolved, PR is yet not approved

@ebarboni
Copy link
Contributor

ok we can redirect to 12.1 or is this important for 12.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants