eclipse.platform icon indicating copy to clipboard operation
eclipse.platform copied to clipboard

Clean-up LaunchManager attempt 2

Open HannesWell opened this issue 1 year ago • 3 comments

Second attempt of https://github.com/eclipse-platform/eclipse.platform/pull/1487 after it was reverted in https://github.com/eclipse-platform/eclipse.platform/pull/1616.

This PR is split into two commits, the first one only containing clean-ups that should not change any logic. The second commit defers obtaining the IResource of a delta as long as possible.

HannesWell avatar Nov 14 '24 22:11 HannesWell

Test Results

 1 755 files  ±0   1 755 suites  ±0   1h 49m 31s ⏱️ + 10m 36s  4 170 tests ±0   4 146 ✅  - 2   22 💤 ±0  2 ❌ +2  13 107 runs  ±0  12 937 ✅  - 6  164 💤 ±0  6 ❌ +6 

For more details on these failures, see this check.

Results for commit 0d45dcf7. ± Comparison against base commit 6dd67323.

github-actions[bot] avatar Nov 14 '24 22:11 github-actions[bot]

The problem is still seen. This "fixes" the bug:

diff --git a/debug/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchManager.java b/debug/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchManager.java
index 80a62dfea1..35228b29ad 100644
--- a/debug/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchManager.java
+++ b/debug/org.eclipse.debug.core/core/org/eclipse/debug/internal/core/LaunchManager.java
@@ -551,8 +551,8 @@ public class LaunchManager extends PlatformObject implements ILaunchManager, IRe
                        }
                        return false;
                }
-               if (LAUNCH_CONFIG_FILE_EXTENSIONS.contains(delta.getFullPath().getFileExtension())) {
-                       if (delta.getResource() instanceof IFile file) {
+               if (delta.getResource() instanceof IFile file) {
+                       if (LAUNCH_CONFIG_FILE_EXTENSIONS.contains(delta.getFullPath().getFileExtension())) {
                                ILaunchConfiguration handle = new LaunchConfiguration(file);
                                switch (delta.getKind()) {
                                        case IResourceDelta.ADDED:

I'm not exactly sure why, probably the return false statement in the outer if block now does something differently.

Here are the reproduction steps:

  1. Create a Java project.
  2. Create a folder .launch in the project.
  3. Create a snippet with a main() method.
  4. Run the snippet.
  5. Go into the launch config dialog for the new launch (of the snippet created above).
  6. Go to the Common tab, make the launch shared, the directory of the shared launch file is .launch.
  7. Delete the shared launch file on command line.
  8. Refresh the project.
  9. Observe that the launch config dialog still lists the launch for the snippet, it should not.

Once you have the project and snippet, you can repeat only 4. and onward.

Also recorded:

platform_gh1618_reproduction_steps.webm

trancexpress avatar Nov 15 '24 08:11 trancexpress

May be this PR should have a test case?

iloveeclipse avatar Nov 15 '24 08:11 iloveeclipse