Skip to content

Commit

Permalink
Only trigger (incremental) builds if there are relevant changes
Browse files Browse the repository at this point in the history
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 #1275
  • Loading branch information
laeubi committed Feb 28, 2023
1 parent e466baf commit 8e5cd49
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -102,7 +105,11 @@ public Set<IProject> 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<BuildContext> incrementalContexts = setupProjectBuildContext(project, kind, delta, participantResults);
Expand All @@ -120,7 +127,7 @@ public Set<IProject> 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) {
Expand Down Expand Up @@ -164,7 +171,7 @@ public Set<IProject> build(MavenSession session, IMavenProjectFacade projectFaca
context.release();
}
}

// Refresh files modified by build participants/maven plugins
refreshResources(project, participantResults.getFiles(), monitor);

Expand All @@ -175,6 +182,44 @@ public Set<IProject> 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<IPath> 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<IIncrementalBuildFramework.BuildContext> setupProjectBuildContext(IProject project, int kind,
IResourceDelta delta, IIncrementalBuildFramework.BuildResultCollector results) throws CoreException {
List<IIncrementalBuildFramework.BuildContext> contexts = new ArrayList<>();
Expand Down Expand Up @@ -227,6 +272,13 @@ private void processMavenSessionErrors(MavenSession session, MojoExecutionKey mo

private void refreshResources(IProject project, Collection<File> 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) {
Expand Down Expand Up @@ -260,6 +312,11 @@ private void refreshResources(IProject project, Collection<File> 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<Throwable, MojoExecutionKey> buildErrors) {
IMavenMarkerManager markerManager = MavenPluginActivator.getDefault().getMavenMarkerManager();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -104,6 +103,8 @@ public class MavenProjectFacade implements IMavenProjectFacade, Serializable {

private final List<IPath> testCompileSourceLocations;

private IPath buildOutputLocation;

private final IPath outputLocation;

private final IPath testOutputLocation;
Expand Down Expand Up @@ -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());
Expand All @@ -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;
Expand Down Expand Up @@ -190,14 +192,15 @@ 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);
this.compileSourceLocations = List.copyOf(other.compileSourceLocations);
this.testCompileSourceLocations = List.copyOf(other.testCompileSourceLocations);

this.outputLocation = other.outputLocation;
this.buildOutputLocation = other.buildOutputLocation;
this.testOutputLocation = other.testOutputLocation;
this.finalName = other.finalName;

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -656,12 +664,12 @@ private static final class MavenProjectConfiguration implements IProjectConfigur

private List<String> 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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
* <code>null</code> 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.
Expand Down

0 comments on commit 8e5cd49

Please sign in to comment.