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.

Sign up for GitHub

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

Jump to bottom

Automatically turn off splash screen if SWT_GTK4=1 #833

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

Automatically turn off splash screen if SWT_GTK4=1 #833

wants to merge 1 commit into from

Conversation

jonahgraham
Copy link
Contributor

When using GTK4 we must not run any GTK3 initialization. Therefore automatically apply -nosplash when SWT_GTK4=1

Fixes #831

@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 2, 2025 •
edited
Loading

Test Results

  669 files  ±0    669 suites  ±0   1h 16m 23s ⏱️ -7s
2 223 tests ±0  2 176 ✅ ±0   47 💤 ±0  0 ❌ ±0 
6 813 runs  ±0  6 670 ✅ ±0  143 💤 ±0  0 ❌ ±0 

Results for commit e7a2e4a. ± Comparison against base commit b9f26bf.

♻️ This comment has been updated with latest results.

@merks
Copy link
Contributor

merks commented Feb 3, 2025

There are errors like this suggesting a version bump is needed:

00:04:58 [ERROR] Failed to execute goal org.eclipse.tycho.extras:tycho-p2-extras-plugin:4.0.11-SNAPSHOT:compare-version-with-baselines (compare-attached-artifacts-with-release) on project org.eclipse.equinox.launcher.win32.win32.aarch64: Only qualifier changed for (org.eclipse.equinox.launcher.win32.win32.aarch64/1.2.1100.v20250202-2240). Expected to have bigger x.y.z than what is available in baseline (1.2.1100.v20240722-2106) -> [Help 1]

@jonahgraham
Copy link
Contributor Author

The failing test (testCoordinatedConfigurationOnBeforeRegisteredManagedService (org.osgi.test.cases.cm.junit.CMCoordinationTestCase) failed) seems to be #190 so I think that failure can be ignored and I don't think anything in this PR could affect this test anyway.

@laeubi
Copy link
Member

laeubi commented Feb 3, 2025

Yes the TCK test failure is expected see:

@jonahgraham @HannesWell
Automatically turn off splash screen if SWT_GTK4=1
e7a2e4a 
When using GTK4 we must not run any GTK3 initialization. Therefore
automatically apply -nosplash when SWT_GTK4=1

Fixes eclipse-equinox#831
@HannesWell HannesWell force-pushed the disable-splash-if-gtk4 branch from 42fec5a to e7a2e4a Compare February 5, 2025 22:08
HannesWell
HannesWell reviewed Feb 5, 2025
View reviewed changes
features/org.eclipse.equinox.executable.feature/library/eclipse.c
// it must be started without a splash screen
// https://github.com/eclipse-equinox/equinox/issues/831
// If the user didn't already specify -nosplash print a warning and turn the splashscreen off
_ftprintf(stderr, "WARNING: SWT_GTK4 does not support splash screen yet. Therefore it has been disabled. To suppress this message launch with -nosplash\n");
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to support splash-screens with GTK4 or is this too complicated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will be - this is a temporary state to resolve #831. To do splash screens on GTK4 we need to use GTK4 to start the splash on the native side. That means the launcher will need GTK4 soon.

What I think we will need to do is similar to what we do in SWT. I created #836 for adding the real version of the splash screen for GTK4.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PS I was looking for a simple patch to write for eclipse launcher to make sure I understood build flow. If this change seems unnecessary, then we can also just close it and work on the correct GTK4 splash screens at a suitable time in the future.

laeubi
laeubi approved these changes Feb 6, 2025
View reviewed changes
Copy link
Member

@laeubi laeubi left a comment

Choose a reason for hiding this comment

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

It is better to have no splash screen until we have GTK4 support instead of failing completely.

@laeubi
Copy link
Member

laeubi commented Feb 6, 2025

@jonahgraham thanks for working on this, I think we have even one more case here:

If the application crash (e.g. exit with an exception in the main, JDK can not be loaded and so on) the the launcher shows a popup (see option --launcher.suppressErrors) I suspect this is using GTK too and would need a similar guard.

@jonahgraham jonahgraham mentioned this pull request Feb 6, 2025
Open
@jonahgraham
Copy link
Contributor Author

@jonahgraham thanks for working on this, I think we have even one more case here:

If the application crash (e.g. exit with an exception in the main, JDK can not be loaded and so on) the the launcher shows a popup (see option --launcher.suppressErrors) I suspect this is using GTK too and would need a similar guard.

Please see #836 (comment) I specifically didn't put a guard on error messages because I thought it best that they still appear even is user does SWT_GTK4=1

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

@HannesWell HannesWell HannesWell left review comments

@laeubi laeubi laeubi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Disable launcher making GTK3 calls (splash) when GTK4 is in use
4 participants
@jonahgraham @merks @laeubi @HannesWell