Remove short-named equinox.launcher.cocoa.macosx fragment

[HannesWell]

The full-named org.eclipse.equinox.launcher.cocoa.macosx.x86_64 fragment exists for a long time already and users of the old short-named variant should have had enough time to migrate.

This would also allow to remove the sym-link in the binaries repo: https://github.com/eclipse-equinox/equinox.binaries/blob/af72d120b237ca99ff37c345e9d0ec20ed49f48a/org.eclipse.equinox.launcher.cocoa.macosx#L1 I wonder why it is pointing to the aarch64 fragment/binaries, altough arch64 support was add about only three years ago, while the short-named fragment exists for over 13 years.

Additionally remove consideration of old development locations of the launcher fragments.

[HannesWell]

~Thinking about it further, the current state even means that if somebody launches a secondary eclipse (IIRC the removed code-path is not called if the executable is used, which sets the --launcher.library argument to the latest so-file) then a user on mac-x86_64 will eventually use the aarch64 library. If that's correct, it looks like the launcher libraries for mac can be used on both archs?~ Edit: That assumption was wrong, see https://github.com/eclipse-equinox/equinox/pull/622#issuecomment-2108418621.

[github-actions[bot]]

Test Results

  660 files  ±0    660 suites  ±0   1h 18m 43s :stopwatch: + 5m 53s 2 195 tests ±0  2 148 :white_check_mark: ±0   47 :zzz: ±0  0 :x: ±0  6 729 runs  ±0  6 586 :white_check_mark: ±0  143 :zzz: ±0  0 :x: ±0 

Results for commit 2d4ae1ec. ± Comparison against base commit 77ad2e48.

:recycle: This comment has been updated with latest results.

[HannesWell]

@tjwatson any objections about this? I didn't find any other reference to org.eclipse.equinox.launcher.cocoa.macosx than the one in Main, which is replaced with this PR too.

[HannesWell]

This would also allow to remove the sym-link in the binaries repo: https://github.com/eclipse-equinox/equinox.binaries/blob/af72d120b237ca99ff37c345e9d0ec20ed49f48a/org.eclipse.equinox.launcher.cocoa.macosx#L1

To do that, I prepared https://github.com/eclipse-equinox/equinox.binaries/pull/4

[Phillipus]

If that's correct, it looks like the launcher libraries for mac can be used on both archs?

Do you mean the so files? Each architecture requires its own so compiled for that architecture.

[Phillipus]

I didn't find any other reference to org.eclipse.equinox.launcher.cocoa.macosx than the one in Main, which is replaced with this PR too.

As mentioned in #625 it is used when launching a child Eclipse instance on Mac aarch64. Is that perhaps coded somewhere in PDE?

[HannesWell]

I didn't find any other reference to org.eclipse.equinox.launcher.cocoa.macosx than the one in Main, which is replaced with this PR too.

As mentioned in #625 it is used when launching a child Eclipse instance on Mac aarch64. Is that perhaps coded somewhere in PDE?

As far as I understand it, in a primary Eclipse the launcher library fragment is computed already in the executable. But if you launch a secondary Eclipse the exe is not involved but the native fragment is determined/searched in the Main.setupJNI(). This can be verified if one places a break point in that method. In a secondary Eclipse this is trivial and in a primary eclipse this can be done by adding the corresponding VM args in the ini or cli Argument (after -vmargs): https://www.baeldung.com/java-application-remote-debugging#from-java9

I think the problem is that currently in case the os is mac and the arch is x86_64then the fragment without arch suffix is used, which contains (in fact at the moment used to contain see #625) the aarch64 library. This does not look right to me. At @Phillipus could you please debug what actually happens in the current state of Main.setupJNI()? E.g. which arch is reported for x86_64 and aarch64 and when the short named fragment is really used respectively if this PR would fix #625?

[Phillipus]

@HannesWell See https://github.com/eclipse-equinox/equinox/issues/625#issuecomment-2105956028

[Phillipus]

Sorry, I just read this:

if this PR would fix https://github.com/eclipse-equinox/equinox/issues/625?

Will check the PR tomorrow!

[Phillipus]

@HannesWell Actually I just did a quick smoke test:

1. Copied your Main code from your PR to my local copy
2. Deleted the org.eclipse.equinox.launcher.cocoa.macosx folder from my target bundle pool (workspace/.metadata/.plugins/org.eclipse.pde.core/.bundle_pool/plugins/ but left the org.eclipse.equinox.launcher.cocoa.macosx.aarch64 folder there
3. Launched child RCP app - splash screen present and correct!

[HannesWell]

Thank you for the fast check and the reference. I think we should definitely have the code change in the Main class since it fixes #625. Of course we could also fully restore the old content but adjusting the main class in general looks like a step forward.

Regarding the removal of the org.eclipse.equinox.launcher.cocoa.macosx fragment, this was also tried in the past e.g.: https://bugs.eclipse.org/bugs/show_bug.cgi?id=572799#c14 @sravanlakkimsetti can you please explain what exactly failed in adjustmentimg P2?

What also confuses me in general: I assume the launcher Mac fragment without arch initially used to target x86_64? But now it targets aarch64? Isn't that a problem?

[Phillipus]

From my own usage on aarch64 and x86_64 there are two scenarios:

1. Launch primary Eclipse or RCP app
2. Launch secondary Eclipse or RCP app

On x86_64 (1) and (2) use the org.eclipse.equinox.launcher.cocoa.macosx.x86_64 bundle. No problem. On aarch64 (1) uses org.eclipse.equinox.launcher.cocoa.macosx.aarch64 and (2) uses org.eclipse.equinox.launcher.cocoa.macosx

AFAICT your PR should resolve this discrepancy.

[Phillipus]

To be clear - on my Mac x86_64 machine there is no org.eclipse.equinox.launcher.cocoa.macosx bundle in the bundle pool, only the org.eclipse.equinox.launcher.cocoa.macosx.x86_64 one

[HannesWell]

1. Copied your Main code from your PR to my local copy

Could you please also check what the value of the arch variable is? Actually the if condition can only be true if arch has the value x86_64? But then it uses an aarch64 fragment?

[Phillipus]

Could you please also check what the value of the arch variable is?

Here:

    private String getArch() {
        if (arch != null)

arch is aarch64

[Phillipus]

Tested on x86_64 machine launching RCP app from Eclipse and splash screen is showing OK.

[Phillipus]

I assume the launcher Mac fragment without arch initially used to target x86_64? But now it targets aarch64? Isn't that a problem?

It's not a problem on x86_64 because it's not present on x86_64 installations or the target bundle pool. It is only present on aarch64 and is a redundant duplicate, used when launching a child instance.

[Phillipus]

AFAICT this PR should just work. Is it possible to create a build to test it? Or maybe just commit and I can test the I-build on x86_64 and aarch64.

[sravanlakkimsetti]

@HannesWell

Hard link org.eclipse.equinox.launcher.cocoa.macosx was added as a workaround during aarch64 support work. During the full SDK build p2 builder was looking for org.eclipse.equinox.launcher.cocoa.macosx for aarch64 architecture though there was no reference.

When I investigated at that time I found there is a hard coding in p2 builder for mac to look for mac universal binaries/libraries(meaning org.eclipse.equinox.launcher.cocoa.macosx fragment). Only in case of x86_64 we had a code to look for org.eclipse.equinox.launcher.cocoa.macosx.x86_64 fragment.

I could not find the exactly where to change so as a stop gap I added hard link org.eclipse.equinox.launcher.cocoa.macosx pointing to org.eclipse.equinox.launcher.cocoa.macosx.aarch64

Please do check whether you are able to do a full SDK build locally. if the SDK build works we can go ahead with this changes.

[HannesWell]

I assume the launcher Mac fragment without arch initially used to target x86_64? But now it targets aarch64? Isn't that a problem?

It's not a problem on x86_64 because it's not present on x86_64 installations or the target bundle pool. It is only present on aarch64 and is a redundant duplicate, used when launching a child instance.

Yes, my confusion was because I miss-understood the braces in https://github.com/eclipse-equinox/equinox/blob/fadf6d115c8ef7fe7586a64bcfb01f19861cd513/bundles/org.eclipse.equinox.launcher/src/org/eclipse/equinox/launcher/Main.java#L453

With negations pushed down the condition actually is !fragmentOS.equals(Constants.OS_MACOSX) || Constants.ARCH_X86_64.equals(fragmentArch), but I falsely read it as !fragmentOS.equals(Constants.OS_MACOSX) && !Constants.ARCH_X86_64.equals(fragmentArch) and therefore falsely concluded that only for mac and x86_64 arch the short name is used, but actually it used the fragment without arch for mac only if the arch is NOT x86_64. Thinking about it again, even with the falsely read condition my conclusion was logically wrong...

Furthermore it's not the executable that searches for the launcher's native fragment, it is recorded in the ini file (or specified as CLI argument) via the --launcher.library argument. For example in the current I-build Eclipse SDK for Windows x86_64 it is:

--launcher.library
plugins/org.eclipse.equinox.launcher.win32.win32.x86_64_1.2.1000.v20240507-1834

In my Oomph provisioned SDK it is an absolute path to that fragment located in my User bundle-pool and for the Eclipse SDK for mac-aarch64 it is:

--launcher.library
../Eclipse/plugins/org.eclipse.equinox.launcher.cocoa.macosx.aarch64_1.2.1000.v20240507-1834

Please do check whether you are able to do a full SDK build locally. if the SDK build works we can go ahead with this changes.

Thank you Sravan for the elaboration, with that and my confusion from above resolved I now understand what is going on.

And found the following locations that need adjustment:

• https://github.com/eclipse-equinox/p2/blob/0f3af2c47b4a42ba9529763dbe54cb5a825237c9/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/p2/publisher/eclipse/EquinoxExecutableAction.java#L146-L148
• https://github.com/eclipse-tycho/tycho/blob/e82043c3e1d7cda55fed53584ca874579b762561/tycho-core/src/main/java/org/eclipse/tycho/core/osgitools/AbstractArtifactDependencyWalker.java#L134-L142

IIRC also also saw such pattern in PDE-build but I can't find it anymore.

All these locations could be changed to use the full-named launcher fragment in all cases, but I think it would be best if the requirement informations are encoded in the main launcher bundle or its fragments. For example in case we have only one fragment that supports all archs (IIRC this is possible for mac binaries or if the FFM API is used) or if no fragments are nessary at all anymore (looking at https://github.com/eclipse-equinox/equinox/issues/623), then all these places have to be adjusted again and old and new versions have to be supported for some time, making it even more complex. Therefore it would be ideal if there would also be a requirement of the launcher host to the corresponding fragment in the environment, similar it is suggested in https://github.com/eclipse-platform/eclipse.platform.swt/issues/490. The launcher code is executed before the OSGi runtime starts but such metadata would feed P2's requirement and would also allow to determine the requirements in a target-platform.

AFAICT this PR should just work. Is it possible to create a build to test it? Or maybe just commit and I can test the I-build on x86_64 and aarch64.

I have now created https://github.com/eclipse-equinox/equinox/pull/629 to only apply the changes in the Main class to first of all address https://github.com/eclipse-equinox/equinox/issues/625.

Eventually removing the org.eclipse.equinox.launcher.cocoa.macosx will probably take a bit longer and probably won't happen in this release since the P2 changes have to trickle down into Tycho first.

[Phillipus]

Eventually removing the org.eclipse.equinox.launcher.cocoa.macosx.x86_64 will probably take a bit longer

You mean org.eclipse.equinox.launcher.cocoa.macosx? If so, that currently now has no so file in it so it's redundant.

[HannesWell]

Eventually removing the org.eclipse.equinox.launcher.cocoa.macosx.x86_64 will probably take a bit longer

You mean org.eclipse.equinox.launcher.cocoa.macosx? If so, that currently now has no so file in it so it's redundant.

You are right. Sorry, Copy-paste-mistake.