From 8e5cd4967eaca2698e8f7109461320e11a11d4d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20L=C3=A4ubrich?= Date: Tue, 28 Feb 2023 13:27:57 +0100 Subject: [PATCH] Only trigger (incremental) builds if there are relevant changes Currently any file change in a project can trigger a rebuild of that project, even if it is from the build-output or from child modules. This can produces "endless" loops of the build and produces a lot of build rush if automatic refresh is activated for the workspace or resources are refreshed on access. On the other hand, without autorefresh some changes are never detected. This changes the following: - check if a delta only contains irrelevant deltas and then do not trigger an incremental maven build - refresh the whole project first to discover new files generated - if auto refresh is enabled, do not manually refresh but use the native refresh events This can considerably reduce the number of rebuilds happening. Fix https://github.com/eclipse-m2e/m2e-core/issues/1275 --- .../internal/builder/MavenBuilderImpl.java | 63 ++++++++++++++++++- .../project/registry/MavenProjectFacade.java | 22 ++++--- .../m2e/core/project/IMavenProjectFacade.java | 6 ++ 3 files changed, 81 insertions(+), 10 deletions(-) diff --git a/org.eclipse.m2e.core/src/org/eclipse/m2e/core/internal/builder/MavenBuilderImpl.java b/org.eclipse.m2e.core/src/org/eclipse/m2e/core/internal/builder/MavenBuilderImpl.java index 60ab5af80e..de559785ac 100644 --- a/org.eclipse.m2e.core/src/org/eclipse/m2e/core/internal/builder/MavenBuilderImpl.java +++ b/org.eclipse.m2e.core/src/org/eclipse/m2e/core/internal/builder/MavenBuilderImpl.java @@ -25,15 +25,18 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.eclipse.core.resources.IFile; import org.eclipse.core.resources.IMarker; import org.eclipse.core.resources.IProject; import org.eclipse.core.resources.IResource; import org.eclipse.core.resources.IResourceDelta; import org.eclipse.core.resources.IncrementalProjectBuilder; +import org.eclipse.core.resources.ResourcesPlugin; import org.eclipse.core.runtime.CoreException; import org.eclipse.core.runtime.IPath; import org.eclipse.core.runtime.IProgressMonitor; @@ -102,7 +105,11 @@ public Set build(MavenSession session, IMavenProjectFacade projectFaca MavenProject mavenProject = projectFacade.getMavenProject(); IProject project = projectFacade.getProject(); - IResourceDelta delta = getDeltaProvider().getDelta(project); + DeltaProvider deltaProvider = getDeltaProvider(); + IResourceDelta delta = deltaProvider.getDelta(project); + if(!hasRelevantDelta(projectFacade, delta)) { + return Set.of(project); + } final BuildResultCollector participantResults = new BuildResultCollector(); List incrementalContexts = setupProjectBuildContext(project, kind, delta, participantResults); @@ -120,7 +127,7 @@ public Set build(MavenSession session, IMavenProjectFacade projectFaca mojoExecutionKey); participantResults.setParticipantId(mojoExecutionKey.getKeyString() + "-" + participant.getClass().getName()); participant.setMavenProjectFacade(projectFacade); - participant.setGetDeltaCallback(getDeltaProvider()); + participant.setGetDeltaCallback(deltaProvider); participant.setSession(session); participant.setBuildContext((AbstractEclipseBuildContext) incrementalContexts.get(0)); if(participant instanceof InternalBuildParticipant2 participant2) { @@ -164,7 +171,7 @@ public Set build(MavenSession session, IMavenProjectFacade projectFaca context.release(); } } - + // Refresh files modified by build participants/maven plugins refreshResources(project, participantResults.getFiles(), monitor); @@ -175,6 +182,44 @@ public Set build(MavenSession session, IMavenProjectFacade projectFaca return dependencies; } + private boolean hasRelevantDelta(IMavenProjectFacade projectFacade, IResourceDelta resourceDelta) + throws CoreException { + if(resourceDelta == null) { + return true; + } + IProject project = projectFacade.getProject(); + IPath buildOutputLocation = projectFacade.getBuildOutputLocation(); + if(project == null || buildOutputLocation == null) { + return true; + } + IPath projectPath = project.getFullPath(); + List moduleLocations = projectFacade.getMavenProjectModules().stream() + .map(module -> projectPath.append(module)).toList(); + AtomicBoolean hasRelevantDelta = new AtomicBoolean(); + resourceDelta.accept(delta -> { + IResource resource = delta.getResource(); + if(resource instanceof IFile) { + IPath fullPath = delta.getFullPath(); + if(buildOutputLocation.isPrefixOf(fullPath)) { + //anything in the build output is not interesting for a change as it is produced by the build + //lets see if there are more interesting parts... + return true; + } + for(IPath modulePath : moduleLocations) { + if(modulePath.isPrefixOf(fullPath)) { + //this is a change in a child module so this one is not really affected and the child will be (possibly) build directly. + return true; + } + } + //anything else has changed, so mark this as relevant an leave the loop + hasRelevantDelta.set(true); + return false; + } + return true; + }); + return hasRelevantDelta.get(); + } + private List setupProjectBuildContext(IProject project, int kind, IResourceDelta delta, IIncrementalBuildFramework.BuildResultCollector results) throws CoreException { List contexts = new ArrayList<>(); @@ -227,6 +272,13 @@ private void processMavenSessionErrors(MavenSession session, MojoExecutionKey mo private void refreshResources(IProject project, Collection resources, IProgressMonitor monitor) throws CoreException { + if(isAutoRefresh()) { + //if autorefresh is on, resources will be refreshed automatically + return; + } + //1st is to refresh all project resources, just to make sure if anything has changed during the build will become visible to eclipse + project.refreshLocal(IResource.DEPTH_INFINITE, monitor); + //2nd is to refresh all explicitly updated resources by named files... for(File file : resources) { IPath path = MavenProjectUtils.getProjectRelativePath(project, file.getAbsolutePath()); if(path == null) { @@ -260,6 +312,11 @@ private void refreshResources(IProject project, Collection resources, IPro } } + @SuppressWarnings("deprecation") + private boolean isAutoRefresh() { + return ResourcesPlugin.getPlugin().getPluginPreferences().getBoolean(ResourcesPlugin.PREF_AUTO_REFRESH); + } + private void processBuildResults(IProject project, MavenProject mavenProject, MavenExecutionResult result, BuildResultCollector results, Map buildErrors) { IMavenMarkerManager markerManager = MavenPluginActivator.getDefault().getMavenMarkerManager(); diff --git a/org.eclipse.m2e.core/src/org/eclipse/m2e/core/internal/project/registry/MavenProjectFacade.java b/org.eclipse.m2e.core/src/org/eclipse/m2e/core/internal/project/registry/MavenProjectFacade.java index 2621a24d1f..407ca81ae0 100644 --- a/org.eclipse.m2e.core/src/org/eclipse/m2e/core/internal/project/registry/MavenProjectFacade.java +++ b/org.eclipse.m2e.core/src/org/eclipse/m2e/core/internal/project/registry/MavenProjectFacade.java @@ -57,7 +57,6 @@ import org.eclipse.m2e.core.internal.Messages; import org.eclipse.m2e.core.internal.embedder.IMavenPlexusContainer; import org.eclipse.m2e.core.internal.embedder.MavenExecutionContext; -import org.eclipse.m2e.core.internal.embedder.MavenProperties; import org.eclipse.m2e.core.internal.embedder.PlexusContainerManager; import org.eclipse.m2e.core.lifecyclemapping.model.IPluginExecutionMetadata; import org.eclipse.m2e.core.project.IMavenProjectFacade; @@ -104,6 +103,8 @@ public class MavenProjectFacade implements IMavenProjectFacade, Serializable { private final List testCompileSourceLocations; + private IPath buildOutputLocation; + private final IPath outputLocation; private final IPath testOutputLocation; @@ -135,12 +136,11 @@ public MavenProjectFacade(ProjectRegistryManager manager, IFile pom, MavenProjec //TODO we currently always use the computed directory here // but https://github.com/eclipse-m2e/m2e-core/issues/904 will add support for a user to specify a custom root directory // and then we should really inherit this from the configuration! - this.resolverConfiguration = new MavenProjectConfiguration(resolverConfiguration, - MavenProperties.computeMultiModuleProjectDirectory(pomFile)); + this.resolverConfiguration = new MavenProjectConfiguration(resolverConfiguration); this.artifactKey = new ArtifactKey(mavenProject.getArtifact()); this.packaging = mavenProject.getPackaging(); - this.modules = mavenProject.getModules(); + this.modules = List.copyOf(mavenProject.getModules()); this.resourceLocations = MavenProjectUtils.getResourceLocations(getProject(), mavenProject.getResources()); this.testResourceLocations = MavenProjectUtils.getResourceLocations(getProject(), mavenProject.getTestResources()); @@ -153,6 +153,8 @@ public MavenProjectFacade(ProjectRegistryManager manager, IFile pom, MavenProjec IPath path = getProjectRelativePath(mavenProject.getBuild().getOutputDirectory()); this.outputLocation = (path != null) ? fullPath.append(path) : null; + path = getProjectRelativePath(mavenProject.getBuild().getDirectory()); + this.buildOutputLocation = (path != null) ? fullPath.append(path) : null; path = getProjectRelativePath(mavenProject.getBuild().getTestOutputDirectory()); this.testOutputLocation = path != null ? fullPath.append(path) : null; @@ -190,7 +192,7 @@ public MavenProjectFacade(MavenProjectFacade other) { this.artifactKey = other.artifactKey; this.packaging = other.packaging; - this.modules = new ArrayList<>(other.modules); + this.modules = other.modules; this.resourceLocations = List.copyOf(other.resourceLocations); this.testResourceLocations = List.copyOf(other.testResourceLocations); @@ -198,6 +200,7 @@ public MavenProjectFacade(MavenProjectFacade other) { this.testCompileSourceLocations = List.copyOf(other.testCompileSourceLocations); this.outputLocation = other.outputLocation; + this.buildOutputLocation = other.buildOutputLocation; this.testOutputLocation = other.testOutputLocation; this.finalName = other.finalName; @@ -245,6 +248,11 @@ public IPath getProjectRelativePath(String resourceLocation) { return MavenProjectUtils.getProjectRelativePath(getProject(), resourceLocation); } + @Override + public IPath getBuildOutputLocation() { + return buildOutputLocation; + } + /** * Returns the full, absolute path of this project maven build output directory relative to the workspace or null if * maven build output directory cannot be determined or outside of the workspace. @@ -656,12 +664,12 @@ private static final class MavenProjectConfiguration implements IProjectConfigur private List inactiveProfiles; - public MavenProjectConfiguration(IProjectConfiguration baseConfiguration, File multiModuleProjectDirectory) { + private MavenProjectConfiguration(IProjectConfiguration baseConfiguration) { if(baseConfiguration == null) { //we should really forbid this but some test seem to pass null! baseConfiguration = new ResolverConfiguration(); } - this.multiModuleProjectDirectory = multiModuleProjectDirectory; + this.multiModuleProjectDirectory = baseConfiguration.getMultiModuleProjectDirectory(); this.mappingId = baseConfiguration.getLifecycleMappingId(); this.properties = Map.copyOf(baseConfiguration.getConfigurationProperties()); this.userProperties = Map.copyOf(baseConfiguration.getUserProperties()); diff --git a/org.eclipse.m2e.core/src/org/eclipse/m2e/core/project/IMavenProjectFacade.java b/org.eclipse.m2e.core/src/org/eclipse/m2e/core/project/IMavenProjectFacade.java index 5f7d84c324..7920cf8e57 100644 --- a/org.eclipse.m2e.core/src/org/eclipse/m2e/core/project/IMavenProjectFacade.java +++ b/org.eclipse.m2e.core/src/org/eclipse/m2e/core/project/IMavenProjectFacade.java @@ -69,6 +69,12 @@ public interface IMavenProjectFacade extends IMavenExecutableLocation { */ IPath getProjectRelativePath(String resourceLocation); + /** + * @return the full, absolute path of this project where build results are placed relative to the workspace or + * null if the directory cannot be determined or is outside of the workspace. + */ + IPath getBuildOutputLocation(); + /** * Returns the full, absolute path of this project maven build output directory relative to the workspace or null if * maven build output directory cannot be determined or outside of the workspace.