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

Adjust Equinox launcher detection for mason-registry packaging. #3379

Merged
merged 1 commit into from
Feb 15, 2025

Conversation

Matthew-Burkard
Copy link
Contributor

Was setting up jdtls on my machine (running Fedora 41) and it was failing to find the equinox launcher jar.
I looked in the plugins directory that the script was searching and the jar was named org.eclipse.equinox.launcher.jar.

I updated this script to use that jar and it started working for me.

@eclipse-ls-bot
Copy link
Contributor

Can one of the admins verify this patch?

@rgrunber
Copy link
Contributor

Is this org.eclipse.equinox.launcher.jar a symbolic link somewhere on the system ? Was this a version of JDT-LS you downloaded yourself ? Is it from the mason-registry ? If not where from ?

As far as I know we only ship bundles that wouldn't contain at least a version number, and in many cases a qualifier. I think I just need to understand what the new structure of the installation is so I can understand why it's failing.

@Matthew-Burkard
Copy link
Contributor Author

org.eclipse.equinox.launcher.jar was not a symbolic link, I installed it using mason through the nvim-lspconfig plugin in nvim.

First thing I tired was updating the glob to launcher* instead of launcher_* but it sill found the wrong jar, think the first match was for macos.

Just confirmed from uninstall and re-install from Mason through nvim-lspconfig that the org.eclipse.equinox.launcher.jar is made under plugins.

@rgrunber rgrunber modified the milestone: End February 2025 Feb 11, 2025
@rgrunber
Copy link
Contributor

I think adding an extra check to work with the versionless equinox launcher is easy enough but just some concerns :

According to https://github.com/mason-org/mason-registry/blob/main/CONTRIBUTING.md#share & https://github.com/mason-org/mason-registry/blob/main/packages/jdtls/package.yaml#L38-L42 , org.eclipse.equinox.launcher.jar should be a symlink.

My main issue is that renaming the launcher is not enough because we have files like config.ini that reference the versioned jars. So all the post processing mason-registry is doing to rename those jars makes me really surprised any of this is working.

@rgrunber rgrunber force-pushed the master branch 2 times, most recently from 84e7684 to f78c184 Compare February 14, 2025 17:21
@rgrunber rgrunber changed the title fix: equinox launcher jar search Adjust Equinox launcher detection for mason-registry packaging. Feb 14, 2025
@rgrunber rgrunber added this to the End February 2025 milestone Feb 14, 2025
Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

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

@Matthew-Burkard , would you be able to sign the ECA, https://www.eclipse.org/legal/eca/ ? I think it's the only remaining requirement.

Signed-off-by: Matthew Burkard <matthewjburkard@gmail.com>
@Matthew-Burkard
Copy link
Contributor Author

@Matthew-Burkard , would you be able to sign the ECA, https://www.eclipse.org/legal/eca/ ? I think it's the only remaining requirement.

I updated the author email of the commit to the one I used when signing the ECA.

Copy link
Contributor

@rgrunber rgrunber left a comment

Choose a reason for hiding this comment

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

I've manually verified that you have a signed ECA on file with the provided author email. The problem is the committer email is still the previous one :

Author:     Matthew Burkard <matthewjburkard@gmail.com>
AuthorDate: Fri Feb 7 20:33:18 2025 -0500
Commit:     Matthew Burkard <matthew@burkard.cloud>
CommitDate: Fri Feb 14 17:45:18 2025 -0500

I suspect that's what the validation doesn't like, but I think this is fine.

@rgrunber rgrunber merged commit a4ba0c1 into eclipse-jdtls:master Feb 15, 2025
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants