From d8c30d15ae0d8e2c11f5829a28299ca92a342e7c Mon Sep 17 00:00:00 2001 From: Stephan Herrmann Date: Tue, 30 Jan 2024 18:08:02 +0100 Subject: [PATCH 1/4] Inconsistent handling of files in an output folder - full vs. incremental build fixes #2920 Signed-off-by: Stephan Herrmann --- .../src/org/eclipse/xtext/builder/impl/XtextBuilder.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/XtextBuilder.java b/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/XtextBuilder.java index 33604f160fc..af0ad36cde7 100644 --- a/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/XtextBuilder.java +++ b/org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/XtextBuilder.java @@ -17,6 +17,7 @@ import java.util.Set; import org.apache.log4j.Logger; +import org.eclipse.core.resources.IFolder; import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.IResourceDelta; import org.eclipse.core.resources.IResourceDeltaVisitor; @@ -588,6 +589,8 @@ public boolean visit(IResourceDelta delta) throws CoreException { } else if (delta.getKind() == IResourceDelta.ADDED || delta.getKind() == IResourceDelta.CHANGED) { return toBeBuiltComputer.updateStorage(null, toBeBuilt, (IStorage) delta.getResource()); } + } else if (delta.getResource() instanceof IFolder) { + return toBeBuiltComputer.isHandled((IFolder) delta.getResource()); } return true; } From b41d4c4b7df5df19692ca99a9336c36ee37ba249 Mon Sep 17 00:00:00 2001 From: Stephan Herrmann Date: Tue, 30 Jan 2024 23:58:17 +0100 Subject: [PATCH 2/4] Test case that "almost" demonstrates the problem Signed-off-by: Stephan Herrmann --- .../xtext/builder/impl/GH2920Test.java | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/impl/GH2920Test.java diff --git a/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/impl/GH2920Test.java b/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/impl/GH2920Test.java new file mode 100644 index 00000000000..479a750959d --- /dev/null +++ b/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/impl/GH2920Test.java @@ -0,0 +1,67 @@ +/******************************************************************************* + * Copyright (c) 2024 GK Software SE and others. + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.xtext.builder.impl; + +import org.eclipse.core.resources.IFile; +import org.eclipse.core.resources.IFolder; +import org.eclipse.core.resources.IProject; +import org.eclipse.core.runtime.CoreException; +import org.eclipse.core.runtime.IProgressMonitor; +import org.eclipse.jdt.core.IJavaProject; +import org.eclipse.xtext.resource.IResourceDescriptions; +import org.eclipse.xtext.resource.IResourceDescriptionsProvider; +import org.eclipse.xtext.ui.XtextProjectHelper; +import org.eclipse.xtext.util.StringInputStream; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertFalse; + +import com.google.inject.Inject; + +/** + * @author sherrmann - Initial contribution and API + */ +public class GH2920Test extends AbstractParticipatingBuilderTest { + private IJavaProject javaProject; + + @Inject private IResourceDescriptionsProvider descriptionsProvider; + + @Before + public void createProjectUnderTest() throws Exception { + javaProject = createJavaProject("NoResourceFromOutput"); + addNature(javaProject.getProject(), XtextProjectHelper.NATURE_ID); + build(); + } + + @Override + public void build(IBuildContext context, IProgressMonitor monitor) throws CoreException { + IResourceDescriptions resourceDescriptions = descriptionsProvider.getResourceDescriptions(context.getResourceSet()); + resourceDescriptions.getAllResourceDescriptions().forEach(rd -> assertFalse(rd.getURI().toString().contains("bin"))); + super.build(context, monitor); + } + + @Test + public void testIncremental() throws Exception { + IProject project = javaProject.getProject(); + createSomeBuilderRelatedFile(project, "Foo"); + build(); + startLogging(); + createSomeBuilderRelatedFile(project, "Bar"); + } + + private IFile createSomeBuilderRelatedFile(IProject project, String name) throws CoreException { + IFolder folder = project.getProject().getFolder("src"); + IFile file = folder.getFile(name + F_EXT); + file.create(new StringInputStream("object "+name), true, monitor()); + build(); + return file; + } +} + From 5ee75f083dae8b46d18c35111b716b3f911341ad Mon Sep 17 00:00:00 2001 From: Lorenzo Bettini Date: Sat, 3 Feb 2024 21:44:43 +0100 Subject: [PATCH 3/4] Recreated test for the issue https://github.com/eclipse/xtext/issues/2920 (#1) Test case that effectively demonstrates the problem the customization happens at the level of the executable extension factory --- .../META-INF/MANIFEST.MF | 2 +- org.eclipse.xtext.builder.tests/plugin.xml | 17 +++++ .../xtext/builder/impl/GH2920Test.java | 31 ++++++-- .../tests/internal/TestsActivatorCustom.java | 71 +++++++++++++++++++ ...guageExecutableExtensionFactoryGH2920.java | 29 ++++++++ 5 files changed, 143 insertions(+), 7 deletions(-) create mode 100644 org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/tests/internal/TestsActivatorCustom.java create mode 100644 org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/tests/ui/BuilderTestLanguageExecutableExtensionFactoryGH2920.java diff --git a/org.eclipse.xtext.builder.tests/META-INF/MANIFEST.MF b/org.eclipse.xtext.builder.tests/META-INF/MANIFEST.MF index ca063b9510e..e6db26378a6 100644 --- a/org.eclipse.xtext.builder.tests/META-INF/MANIFEST.MF +++ b/org.eclipse.xtext.builder.tests/META-INF/MANIFEST.MF @@ -40,7 +40,7 @@ Import-Package: org.apache.log4j;version="1.2.24", org.junit.rules;version="4.13.2", org.junit.runner;version="4.13.2", org.junit.runners.model;version="4.13.2" -Bundle-Activator: org.eclipse.xtext.builder.tests.internal.TestsActivator +Bundle-Activator: org.eclipse.xtext.builder.tests.internal.TestsActivatorCustom Bundle-ActivationPolicy: lazy Export-Package: org.eclipse.xtext.builder.tests;version="2.34.0", org.eclipse.xtext.builder.tests.builderTestLanguage;version="2.34.0", diff --git a/org.eclipse.xtext.builder.tests/plugin.xml b/org.eclipse.xtext.builder.tests/plugin.xml index 4286158cb06..6e883f0689e 100644 --- a/org.eclipse.xtext.builder.tests/plugin.xml +++ b/org.eclipse.xtext.builder.tests/plugin.xml @@ -132,14 +132,31 @@ type="buildertestlanguage"> + + + + + + + + + + + diff --git a/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/impl/GH2920Test.java b/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/impl/GH2920Test.java index 479a750959d..574c9c7f0a4 100644 --- a/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/impl/GH2920Test.java +++ b/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/impl/GH2920Test.java @@ -8,6 +8,12 @@ *******************************************************************************/ package org.eclipse.xtext.builder.impl; +import static org.junit.Assert.*; + +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IFolder; import org.eclipse.core.resources.IProject; @@ -21,18 +27,19 @@ import org.junit.Before; import org.junit.Test; -import static org.junit.Assert.assertFalse; - import com.google.inject.Inject; /** * @author sherrmann - Initial contribution and API + * @author Lorenzo Bettini - make it appropriate for reproducing https://github.com/eclipse/xtext/issues/2920 */ public class GH2920Test extends AbstractParticipatingBuilderTest { private IJavaProject javaProject; - + @Inject private IResourceDescriptionsProvider descriptionsProvider; + private List descriptionsInOutputFolder = new ArrayList<>(); + @Before public void createProjectUnderTest() throws Exception { javaProject = createJavaProject("NoResourceFromOutput"); @@ -43,7 +50,12 @@ public void createProjectUnderTest() throws Exception { @Override public void build(IBuildContext context, IProgressMonitor monitor) throws CoreException { IResourceDescriptions resourceDescriptions = descriptionsProvider.getResourceDescriptions(context.getResourceSet()); - resourceDescriptions.getAllResourceDescriptions().forEach(rd -> assertFalse(rd.getURI().toString().contains("bin"))); + resourceDescriptions.getAllResourceDescriptions().forEach( + rd -> { + String uriString = rd.getURI().toString(); + if (uriString.contains("bin")) + descriptionsInOutputFolder.add(uriString); + }); super.build(context, monitor); } @@ -54,14 +66,21 @@ public void testIncremental() throws Exception { build(); startLogging(); createSomeBuilderRelatedFile(project, "Bar"); + checkNoUrisInOutputFolder(); } private IFile createSomeBuilderRelatedFile(IProject project, String name) throws CoreException { IFolder folder = project.getProject().getFolder("src"); - IFile file = folder.getFile(name + F_EXT); + IFile file = folder.getFile(name + F_EXT + "GH2920"); file.create(new StringInputStream("object "+name), true, monitor()); build(); return file; } -} + private void checkNoUrisInOutputFolder() { + if (!descriptionsInOutputFolder.isEmpty()) + fail("unexpected resources in output folder:\n" + + descriptionsInOutputFolder.stream() + .collect(Collectors.joining("\n", " ", ""))); + } +} diff --git a/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/tests/internal/TestsActivatorCustom.java b/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/tests/internal/TestsActivatorCustom.java new file mode 100644 index 00000000000..c6d437d427d --- /dev/null +++ b/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/tests/internal/TestsActivatorCustom.java @@ -0,0 +1,71 @@ +/******************************************************************************* + * Copyright (c) 2024 itemis AG (http://www.itemis.eu) and others. + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.xtext.builder.tests.internal; + +import org.eclipse.xtext.Constants; +import org.eclipse.xtext.builder.tests.BuilderTestLanguageRuntimeModule; +import org.eclipse.xtext.builder.tests.ui.BuilderTestLanguageUiModule; +import org.eclipse.xtext.ui.shared.JdtHelper; +import org.eclipse.xtext.ui.util.IJdtHelper; + +import com.google.inject.Binder; +import com.google.inject.Module; +import com.google.inject.Provider; +import com.google.inject.name.Names; + +/** + * This allows customizations in the UI, specific for some test scenarions, without creating new languages. + * + * @author Lorenzo Bettini - Initial contribution and API + */ +public class TestsActivatorCustom extends TestsActivator { + + public static final String ORG_ECLIPSE_XTEXT_BUILDER_TESTS_BUILDERTESTLANGUAGE_GH2920 = ORG_ECLIPSE_XTEXT_BUILDER_TESTS_BUILDERTESTLANGUAGE + + "GH2920"; + + @Override + protected Module getRuntimeModule(String grammar) { + if (ORG_ECLIPSE_XTEXT_BUILDER_TESTS_BUILDERTESTLANGUAGE_GH2920.equals(grammar)) + return new BuilderTestLanguageRuntimeModule() { + @Override + public void configureFileExtensions(Binder binder) { + if (properties == null || properties.getProperty(Constants.FILE_EXTENSIONS) == null) + binder.bind(String.class).annotatedWith(Names.named(Constants.FILE_EXTENSIONS)) + .toInstance("buildertestlanguageGH2920"); + } + }; + return super.getRuntimeModule(grammar); + } + + @Override + protected Module getUiModule(String grammar) { + if (ORG_ECLIPSE_XTEXT_BUILDER_TESTS_BUILDERTESTLANGUAGE_GH2920.equals(grammar)) + return new BuilderTestLanguageUiModule(this) { + /** + * By default, the {@link IJdtHelper} is bound by the {@link org.eclipse.xtext.ui.shared.SharedStateModule}, but since the + * UI module is mixed as the last one in {@link TestsActivator}, we can "override" its binding here. + */ + @SuppressWarnings("unused") + public Provider provideJdtHelper() { + return new Provider<>() { + @Override + public IJdtHelper get() { + return new JdtHelper() { + @Override + protected boolean computeJavaCoreAvailable() { + return false; + } + }; + } + }; + } + }; + return super.getUiModule(grammar); + } +} diff --git a/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/tests/ui/BuilderTestLanguageExecutableExtensionFactoryGH2920.java b/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/tests/ui/BuilderTestLanguageExecutableExtensionFactoryGH2920.java new file mode 100644 index 00000000000..5ea6b04073f --- /dev/null +++ b/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/tests/ui/BuilderTestLanguageExecutableExtensionFactoryGH2920.java @@ -0,0 +1,29 @@ +/******************************************************************************* + * Copyright (c) 2024 itemis AG (http://www.itemis.eu) and others. + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.xtext.builder.tests.ui; + +import org.eclipse.xtext.builder.tests.internal.TestsActivator; +import org.eclipse.xtext.builder.tests.internal.TestsActivatorCustom; + +import com.google.inject.Injector; + +/** + * This allows customizations in the UI, specific for some test scenarions, without creating new languages. + * + * @author Lorenzo Bettini - Initial contribution and API + */ +public class BuilderTestLanguageExecutableExtensionFactoryGH2920 extends BuilderTestLanguageExecutableExtensionFactory { + + @Override + protected Injector getInjector() { + TestsActivator activator = TestsActivator.getInstance(); + return activator != null ? activator.getInjector(TestsActivatorCustom.ORG_ECLIPSE_XTEXT_BUILDER_TESTS_BUILDERTESTLANGUAGE_GH2920) + : null; + } +} From 4b43a2103fbd3775c4bde5f46dfff5cbe53dc486 Mon Sep 17 00:00:00 2001 From: Stephan Herrmann Date: Sat, 3 Feb 2024 21:54:15 +0100 Subject: [PATCH 4/4] Modify the test customization to be even closer to the original problem Signed-off-by: Stephan Herrmann --- .../tests/internal/TestsActivatorCustom.java | 48 ++++++++++++------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/tests/internal/TestsActivatorCustom.java b/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/tests/internal/TestsActivatorCustom.java index c6d437d427d..afdab13ec47 100644 --- a/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/tests/internal/TestsActivatorCustom.java +++ b/org.eclipse.xtext.builder.tests/src/org/eclipse/xtext/builder/tests/internal/TestsActivatorCustom.java @@ -8,15 +8,18 @@ *******************************************************************************/ package org.eclipse.xtext.builder.tests.internal; +import org.eclipse.core.resources.IStorage; +import org.eclipse.emf.common.util.URI; +import org.eclipse.jface.viewers.ILabelProvider; import org.eclipse.xtext.Constants; import org.eclipse.xtext.builder.tests.BuilderTestLanguageRuntimeModule; import org.eclipse.xtext.builder.tests.ui.BuilderTestLanguageUiModule; -import org.eclipse.xtext.ui.shared.JdtHelper; -import org.eclipse.xtext.ui.util.IJdtHelper; +import org.eclipse.xtext.resource.impl.DefaultResourceServiceProvider; +import org.eclipse.xtext.ui.editor.IURIEditorOpener; +import org.eclipse.xtext.ui.resource.IResourceUIServiceProvider; import com.google.inject.Binder; import com.google.inject.Module; -import com.google.inject.Provider; import com.google.inject.name.Names; /** @@ -48,24 +51,35 @@ protected Module getUiModule(String grammar) { if (ORG_ECLIPSE_XTEXT_BUILDER_TESTS_BUILDERTESTLANGUAGE_GH2920.equals(grammar)) return new BuilderTestLanguageUiModule(this) { /** - * By default, the {@link IJdtHelper} is bound by the {@link org.eclipse.xtext.ui.shared.SharedStateModule}, but since the - * UI module is mixed as the last one in {@link TestsActivator}, we can "override" its binding here. + * {@link IResourceUIServiceProvider} specifies its default implementation to be {@link org.eclipse.xtext.ui.resource.DefaultResourceUIServiceProvider} + * with no explicit binding in any module, so we can customize the binding here. */ @SuppressWarnings("unused") - public Provider provideJdtHelper() { - return new Provider<>() { - @Override - public IJdtHelper get() { - return new JdtHelper() { - @Override - protected boolean computeJavaCoreAvailable() { - return false; - } - }; - } - }; + public Class bindIResourceUIServiceProvider() { + return ResourceNonUIServiceProvider.class; } }; return super.getUiModule(grammar); } + /** A provider that essentially mimics the behavior of {@link DefaultResourceServiceProvider} in the guise of a {@link IResourceUIServiceProvider}. */ + static class ResourceNonUIServiceProvider extends DefaultResourceServiceProvider implements IResourceUIServiceProvider { + @Override + public ILabelProvider getLabelProvider() { + return null; + } + @Override + public boolean canHandle(URI uri, IStorage storage) { + // if we wouldn't implement IResourceUIServiceProvider then this method would be invoked in the first place: + return canHandle(uri); + } + @Override + public IURIEditorOpener getURIEditorOpener() { + return null; + } + @SuppressWarnings({ "deprecation", "restriction" }) + @Override + public org.eclipse.xtext.ui.refactoring.IReferenceUpdater getReferenceUpdater() { + return null; + } + } }