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

Remove unused equinox.executable.feature resources #592

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HannesWell
Copy link
Member

The build.xml is currently shipped with the o.e.equinox.executable.feature which is odd and it at least seems not to be used for the build of Equinox.
I wonder if this is used by Tycho or probably more likely by the PDE Build Ant targets?
@iloveeclipse can you tell that with certainty? I have found some hits in PDE that indicate that, but I'm not sure and as far as I understand that content I'm in doubt it is useful for anything?

More or less the same question I want to raise for to the o.e.equinox.executable.feature/resources/build.properties. It is also shipped with the feature but is content is partly outdated (linux.aarch64 is missing) and its content is inconsistent. Sometimes it uses launcher as name of the executable and sometimes ${launcherName} (which I assume is supposed to be interpolated?

####Even though marked as custom, the content of the build.properties needs to be updated to ensure correctness of RCP app exported
root.permissions.755=${launcherName}
root.win32.win32.x86_64=file:bin/win32/win32/x86_64/launcher.exe
root.win32.win32.x86_64.permissions.755=launcher.exe
root.linux.gtk.ppc64le=bin/gtk/linux/ppc64le,gtk_root
root.linux.gtk.ppc64le.permissions.755=launcher
root.macosx.cocoa.x86_64=bin/cocoa/macosx/x86_64
root.macosx.cocoa.x86_64.permissions.755=Contents/MacOS/${launcherName}
root.macosx.cocoa.aarch64=bin/cocoa/macosx/aarch64
root.macosx.cocoa.aarch64.permissions.755=Contents/MacOS/${launcherName}
root.linux.gtk.x86_64=bin/gtk/linux/x86_64,gtk_root
root.linux.gtk.x86_64.permissions.755=libcairo-swt.so
root.linux.gtk.loongarch64=bin/gtk/linux/loongarch64,gtk_root
root.linux.gtk.loongarch64.permissions.755=launcher

@akurtakov or @laeubi maybe you know something about this?

Copy link

github-actions bot commented Apr 10, 2024

Test Results

  660 files  ±0    660 suites  ±0   1h 11m 51s ⏱️ +40s
2 195 tests ±0  2 148 ✅ ±0   47 💤 ±0  0 ❌ ±0 
6 729 runs  ±0  6 586 ✅ ±0  143 💤 ±0  0 ❌ ±0 

Results for commit e94162d. ± Comparison against base commit 99a8578.

♻️ This comment has been updated with latest results.

@iloveeclipse
Copy link
Member

I don't have any knowledge in this area. May be @sravanlakkimsetti knows more?

@merks
Copy link
Contributor

merks commented Apr 11, 2024

This is needed to build products, at least with Tycho. It, e.g., https://download.eclipse.org/eclipse/updates/4.32-I-builds/I20240410-1800/features/org.eclipse.equinox.executable_3.8.2500.v20240410-1706.jar, contains the launchers:

image

For Oomph we need to take special steps to ensure it's specified the target platform so that we can build the installer product:

image

When a product is built, you can see it in the target folder:

image

When I remove it from the target platform, the build fails like this:

[INFO] -------------------------[ eclipse-repository ]-------------------------
[INFO] Resolving dependencies of MavenProject: org.eclipse.oomph:org.eclipse.oomph.setup.installer.product:1.33.0-SNAPSHOT @ D:\Users\merks\oomph-1.33\git\oomph\products\org.eclipse.oomph.setup.installer.product\pom.xml
[INFO] {osgi.os=win32, osgi.ws=win32, org.eclipse.update.install.features=true, osgi.arch=x86_64, org.eclipse.update.install.sources=true}
[ERROR] Cannot resolve project dependencies:
[ERROR]   Software being installed: org.eclipse.oomph.setup.installer.product.with-jre 1.33.0.qualifier
[ERROR]   Missing requirement: org.eclipse.oomph.setup.installer.product.with-jre 1.33.0.qualifier requires 'org.eclipse.equinox.p2.iu; org.eclipse.equinox.executable.feature.group 0.0.0' but it could not be found
[ERROR] 
[ERROR] See https://wiki.eclipse.org/Tycho/Dependency_Resolution_Troubleshooting for help.

So products appears to have an implicit requirement on this feature.

@HannesWell HannesWell force-pushed the remove-equinox-executable-resources branch from 09f6bdc to ea4dd45 Compare April 11, 2024 18:39
@HannesWell HannesWell marked this pull request as draft April 11, 2024 18:42
@HannesWell
Copy link
Member Author

HannesWell commented Apr 11, 2024

So products appears to have an implicit requirement on this feature.

Yes, but this PR is not about removing the feature, it is only about to removing the build.xml (and build.properties) embedded in the built feature jar:

grafik

I have updated this PR to also remove the build.properties to show its 'full' state, given both are not required.
I would be surprised if Tycho relies on an ANT script delivered with this feature, but I'm not sure if the build.properties delivered with it is used somehow.
At the same time I would not be surprised if the PDE build ANT task rely on the shipped build.xml or build.properties in some way. For ordinary bundles and features the build.properties are not included in the built jar. But as Ed already elaborated, the equinox.executable feature is special so I will investigate Tycho and PDE build in detail unless somebody else can say with confident that any of those files is useful or not.

@sravanlakkimsetti
Copy link
Member

I remember build.xml being used to package native launchers for equinox download page. See https://download.eclipse.org/equinox/drops/I20240411-1800/index.php(section Native Launchers).

If these zip files are created without build.xml we can get rid of build.xml.

@akurtakov
Copy link
Member

I remember build.xml being used to package native launchers for equinox download page. See https://download.eclipse.org/equinox/drops/I20240411-1800/index.php(section Native Launchers).

If these zip files are created without build.xml we can get rid of build.xml.

I am surprised by the many N/A on that page
image

This makes me wonder whether anyone is ever looking at this page as we never got any report about this at all.

@laeubi
Copy link
Member

laeubi commented Apr 14, 2024

I remember that the pde-build tests depend on these files, I already wondered about them a while back but gave up on this as keeping them is obviously not an issue but removing them will probably break some unknown consumers.

@HannesWell HannesWell force-pushed the remove-equinox-executable-resources branch from f3f1453 to e94162d Compare May 9, 2024 19:48
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.

6 participants