[#1710] Don't touch the project again if it was already built

The strategy for the recovery build was changed such that we keep
an additional flag on the builder to indicate that the next regular
build is supposed to be a full build because the resource deltas cannot
be trusted.

Signed-off-by: Sebastian Zarnekow <[email protected]>

org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/BuildManagerAccess.java

@@ -99,5 +99,20 @@ public static void requestBuild() {
 			throw new RuntimeException("Unexpected workspace implementation");
 		}
 	}
+
+	/**
+	 * Schedule an auto build to be run.
+	 * 
+	 * @since 2.26
+	 */
+	public static void scheduleAutoBuild() {
+		IWorkspace workspace = ResourcesPlugin.getWorkspace();
+		if (workspace instanceof Workspace) {
+			BuildManager buildManager = ((Workspace) workspace).getBuildManager();
+			buildManager.endTopLevel(true);
+		} else {
+			throw new RuntimeException("Unexpected workspace implementation");
+		}
+	}
 
 }

org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/BuilderStateDiscarder.java

@@ -10,12 +10,8 @@
 
 import java.util.Map;
 
-import org.apache.log4j.Logger;
 import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IncrementalProjectBuilder;
-import org.eclipse.core.runtime.CoreException;
-import org.eclipse.core.runtime.Status;
-import org.eclipse.core.runtime.jobs.Job;
 
 import com.google.common.annotations.Beta;
 
@@ -33,50 +29,42 @@
 @Beta
 public class BuilderStateDiscarder {
 
-	private static final Logger logger = Logger.getLogger(BuilderStateDiscarder.class);
-
 	/**
 	 * Returns true, if the given builderArguments indicate that we should discard the built state
 	 * for the given projects.
 	 */
 	public boolean forgetLastBuildState(Iterable<IProject> toUpdate, Map<String, String> builderArguments) {
+		/*
+		 * Implementation note:
+		 * This is an unsafe operation. There no guarantee that any lock is being acquired by the current thread
+		 * while performing this operation. It is very well possible that this is running concurrently with a build.
+		 * 
+		 * When we come here, we might, or might not
+		 * - hold the AbstractBuilderState.loadLock
+		 * - hold the org.eclipse.core.internal.resources.WorkManager.lock
+		 * - come from a build and see a situation with org.eclipse.core.internal.resources.Workspace.treeLocked being the case
+		 * - might or might not have a rule stack available on org.eclipse.core.internal.jobs.ThreadJob.ruleStack
+		 * 
+		 * It is desired that we only discard the build information of the XtextBuilder and not of all the builders
+		 * of a project. Therefore, we try to keep the interaction with the resource model to a minimum.
+		 * 
+		 * The strategy is coupled to the internals of the XtextBuilder where state is maintained
+		 * that allows to schedule an initial full build for a project.
+		 */
 		if (canHandleBuildFlag(builderArguments)) {
 			for (IProject project : toUpdate) {
-				XtextBuilder builder = BuildManagerAccess.findBuilder(project);
-				if (builder != null) {
-					builder.forgetLastBuiltState();
-					touchProject(project);
+				if (project.isAccessible()) {
+					XtextBuilder builder = BuildManagerAccess.findBuilder(project);
+					if (builder != null) {
+						builder.requestFullBuild(IBuildFlag.FORGET_BUILD_STATE_ONLY.isSet(builderArguments));
+					}
 				}
 			}
 			return true;
 		}
 		return false;
 	}
-
-	/**
-	 * Touch a given project such that the auto-build knows it should be rebuild.
-	 *
-	 * @since 2.26
-	 */
-	protected void touchProject(IProject project) {
-		Job touchJob = Job.create("Touch project " + project.getName(), progressMonitor -> {
-			try {
-				if (project.isAccessible()) {
-					project.touch(progressMonitor);
-				}
-				return Status.OK_STATUS;
-			} catch (CoreException e) {
-				logger.error("Failed to refresh project while forgetting its builder state", e);
-				return Status.CANCEL_STATUS;
-			}
-		});
-		touchJob.setRule(project);
-		touchJob.setUser(false);
-		touchJob.setSystem(true);
-		touchJob.schedule(0);
-
-	}
-
+
 	protected boolean canHandleBuildFlag(Map<String, String> builderArguments) {
 		return IBuildFlag.FORGET_BUILD_STATE_ONLY.isSet(builderArguments)
 				|| IBuildFlag.RECOVERY_BUILD.isSet(builderArguments);

org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/ProjectOpenedOrClosedListener.java

@@ -214,11 +214,18 @@ else if (!XtextProjectHelper.hasNature(project)) {
 	}
 
 	protected void scheduleRemoveProjectJob(final IProject project) {
-		final ToBeBuilt toBeBuilt = getToBeBuiltComputer().removeProject(project, new NullProgressMonitor());
-		if (toBeBuilt.getToBeDeleted().isEmpty() && toBeBuilt.getToBeUpdated().isEmpty()) {
-			return;
+		try {
+			final ToBeBuilt toBeBuilt = getToBeBuiltComputer().removeProject(project, new NullProgressMonitor());
+			if (toBeBuilt.getToBeDeleted().isEmpty() && toBeBuilt.getToBeUpdated().isEmpty()) {
+				return;
+			}
+			scheduleJob(project.getName(), toBeBuilt);
+		} finally {
+			XtextBuilder builder = BuildManagerAccess.findBuilder(project);
+			if (builder != null) {
+				builder.forgetLastBuiltState();
+			}
 		}
-		scheduleJob(project.getName(), toBeBuilt);
 	}
 
 	/**

org.eclipse.xtext.builder/src/org/eclipse/xtext/builder/impl/XtextBuilder.java

@@ -8,6 +8,7 @@
  *******************************************************************************/
 package org.eclipse.xtext.builder.impl;
 
+import java.lang.reflect.Field;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Map;
@@ -158,25 +159,149 @@ public static enum SchedulingOption {
 		 * all projects with Xtext nature in the workspace, extended by JDTs External Folder Project
 		 * @see org.eclipse.jdt.internal.core.ExternalFoldersManager
 		 * */
+		@SuppressWarnings("restriction")
 		ALL_XTEXT_PROJECTS_AND_JDTEXTFOLDER,
 		/** the currently building project */
 		PROJECT,
 		/** null scheduling rule */
 		NULL;
 	}
 
+	/**
+	 * Used to reflectively set the flag that will instruct the Eclipse infrastructure to also
+	 * consider this builder if the delta is empty. An XtextBuilder might need to run a build even
+	 * if no resource changes have been recorded, e.g. if the index wasn't deserialized successfully
+	 * but the workspace state is still consistent from Eclipse's POV.
+	 * 
+	 * @since 2.26
+	 */
+	private static final Field callOnEmptyData;
+	static {
+		try {
+			@SuppressWarnings("restriction")
+			Field fld = org.eclipse.core.internal.events.InternalBuilder.class.getDeclaredField("callOnEmptyDelta");
+			fld.setAccessible(true);
+			callOnEmptyData = fld;
+		} catch (ReflectiveOperationException e) {
+			throw new ExceptionInInitializerError(e);
+		}
+	}
+
+	/**
+	 * True if the next regular build (not a clean build) that will be executed, will be treated as a full build request.
+	 * 
+	 * @since 2.26
+	 */
+	private boolean nextBuildIsFullBuild = false;
+
+	/**
+	 * True if a regular request for a full build from an external service is allowed.
+	 * 
+	 * @since 2.26
+	 */
+	private boolean allowRequestFullBuild = true;
+
+	/**
+	 * The lock protects against concurrent access to the boolean flags {@link #nextBuildIsFullBuild} and {@link #allowRequestFullBuild}.
+	 * 
+	 * @since 2.26
+	 */
+	private final Object requestFullBuildLock = new Object();
+
+	/**
+	 * An external (to this builder and the Eclipse BuildManager) service may want to trigger an initial
+	 * full build for this project if it was detected that the project state is not in sync with the builder
+	 * state. This can be done once in the life-cycle of an XtextBuilder which is bound to an IProject or, if {@code force} 
+	 * is set to true, when a configuration change to the project happened that warrants a full build.
+	 * 
+	 * @since 2.26
+	 */
+	protected void requestFullBuild(boolean force) {
+		/*
+		 * Synchronize here to avoid a build being triggered while a build was running and completed successfully.
+		 * Maintaining the flags must be coherent.
+		 */
+		synchronized (requestFullBuildLock) {
+			if (allowRequestFullBuild || force) {
+				allowRequestFullBuild = false;
+				nextBuildIsFullBuild = true;
+				try {
+					callOnEmptyData.set(this, true);
+				} catch (IllegalArgumentException | IllegalAccessException e) {
+					log.error(e.getMessage(), e);
+				}
+				BuildManagerAccess.scheduleAutoBuild();
+			}
+		}
+	}
+
+	/**
+	 * Answer if a full build was already requested externally and not yet done.
+	 * 
+	 * @since 2.26
+	 */
+	protected boolean wasFullBuildRequested() {
+		synchronized (requestFullBuildLock) {
+			return nextBuildIsFullBuild;
+		}
+	}
+
+	/**
+	 * After a full build was run, reset the externally requested full build.
+	 * Don't reset, if the current build was an incremental build but a full build was requested
+	 * in the meantime.
+	 * 
+	 * @since 2.26
+	 */
+	protected void unsetWasFullBuildRequested(boolean wasFullBuildRequested) {
+		synchronized (requestFullBuildLock) {
+			if (nextBuildIsFullBuild && !wasFullBuildRequested) {
+				// an intermediate operation requested an enforced full build, e.g. due to a class-path change
+				// make the next build a full build
+				return;
+			}
+			allowRequestFullBuild = false;
+			nextBuildIsFullBuild = false;
+			try {
+				callOnEmptyData.set(this, false);
+			} catch (IllegalArgumentException | IllegalAccessException e) {
+				log.error(e.getMessage(), e);
+			}
+		}
+	}
+
+	/**
+	 * Attempt to access the builder state. If it wasn't loaded yet, an attempt to load it is made.
+	 * This may trigger the {@link BuilderStateDiscarder#forgetLastBuildState(Iterable, Map)} if there
+	 * is no build state available.
+	 * Since we are currently building this project, we turn the current build into a full build if it
+	 * was an auto build or an incremental build instead. 
+	 * 
+	 * @since 2.26
+	 */
+	protected void ensureBuilderStateLoaded() {
+		// ensure the state was loaded or load it now or wait until it was successfully loaded
+		builderState.isEmpty();
+	}
+
 	@SuppressWarnings("rawtypes")
 	@Override
-	protected IProject[] build(final int kind, Map args, IProgressMonitor monitor) throws CoreException {
-		if (IBuildFlag.FORGET_BUILD_STATE_ONLY.isSet(args)) {
+	protected IProject[] build(int kind, Map args, IProgressMonitor monitor) throws CoreException {
+		ensureBuilderStateLoaded();
+
+		boolean wasFullBuildRequested = wasFullBuildRequested();
+		if (!wasFullBuildRequested && IBuildFlag.FORGET_BUILD_STATE_ONLY.isSet(args)) {
 			forgetLastBuiltState();
 			return getReferencedProjects();
 		}
+		int effectiveKind = wasFullBuildRequested ? FULL_BUILD : kind;
+
 		long startTime = System.currentTimeMillis();
-		StoppedTask task = Stopwatches.forTask(String.format("XtextBuilder.build[%s]", getKindAsString(kind)));
+		StoppedTask task = Stopwatches.forTask(String.format("XtextBuilder.build[%s]", getKindAsString(effectiveKind)));
 		try {
 			queuedBuildData.createCheckpoint();
-			if(shouldCancelBuild(kind)) {
+			if(shouldCancelBuild(effectiveKind)) {
+				buildLogger.log("interrupted");
 				throw new OperationCanceledException("Build has been interrupted");
 			}
 			task.start();
@@ -190,16 +315,16 @@ public void subTask(String name) {
 
 					@Override
 					public boolean isCanceled() {
-						boolean shouldCancelBuild = shouldCancelBuild(kind);
+						boolean shouldCancelBuild = shouldCancelBuild(effectiveKind);
 						if (shouldCancelBuild)
 							buildLogger.log("interrupted");
 						return shouldCancelBuild || super.isCanceled();
 					}
 				};
 			}
 			SubMonitor progress = SubMonitor.convert(monitor, 1);
-			if (kind == FULL_BUILD) {
-				fullBuild(progress.split(1), IBuildFlag.RECOVERY_BUILD.isSet(args));
+			if (effectiveKind == FULL_BUILD) {
+				fullBuild(progress.split(1), !wasFullBuildRequested && IBuildFlag.RECOVERY_BUILD.isSet(args));
 			} else {
 				IResourceDelta delta = getDelta(getProject());
 				if (delta == null || isOpened(delta)) {
@@ -208,6 +333,8 @@ public boolean isCanceled() {
 					incrementalBuild(delta, progress.split(1));
 				}
 			}
+			// If no exceptions have been thrown, fix the information about the requested full builds
+			unsetWasFullBuildRequested(wasFullBuildRequested);
 		} catch (CoreException e) {
 			log.error(e.getMessage(), e);
 			throw e;
@@ -251,7 +378,7 @@ private boolean shouldCancelBuild(int buildKind) {
 	}
 
 	private void handleCanceled(Throwable t) {
-		// If the cancelation happens due to an external interruption, don't pass an 
+		// If the cancellation happens due to an external interruption, don't pass an 
 		// OperationCanceledException on to the BuildManager as it would force a full 
 		// build in the next round. Instead, save the resource deltas to be reprocessed 
 		// next time.
@@ -376,7 +503,7 @@ protected void doBuild(ToBeBuilt toBeBuilt, IProgressMonitor monitor, BuildType
 	protected void doBuild(ToBeBuilt toBeBuilt, Set<String> removedProjects, IProgressMonitor monitor, BuildType type) throws CoreException {
 		buildLogger.log("Building " + getProject().getName());
 		// return early if there's nothing to do.
-		// we reuse the isEmpty() impl from BuildData assuming that it doesnT access the ResourceSet which is still null 
+		// we reuse the isEmpty() implementation from BuildData assuming that it doesnT access the ResourceSet which is still null 
 		// and would be expensive to create.
 		boolean indexingOnly = type == BuildType.RECOVERY;
 		if (new BuildData(getProject().getName(), null, toBeBuilt, queuedBuildData, indexingOnly, this::needRebuild, removedProjects).isEmpty())
@@ -488,6 +615,7 @@ protected void clean(IProgressMonitor monitor) throws CoreException {
 				task.reschedule();
 				throw e;
 			}
+			unsetWasFullBuildRequested(true);
 		} finally {
 			if (monitor != null)
 				monitor.done();