Fix issue in FastCheckNeedsBuild

The FastCheckNeedsBuild method now takes an OperationContext as argument.
This is necessary because when building a project, OperationContext can
contain custom msbuild properties that can have an effect on the build
(for example, a build can be specific to the target device currently selected
in the IDE).

OnFastCheckNeedsBuild now keeps track of the global msbuild properties
specified in the context, and uses them when checking for changes.

6 files changed, 186 insertions, 14 deletions

diff --git a/main/src/core/MonoDevelop.Core/MonoDevelop.Projects.SharedAssetsProjects/SharedAssetsProject.cs b/main/src/core/MonoDevelop.Core/MonoDevelop.Projects.SharedAssetsProjects/SharedAssetsProject.cs
index 7461135756..447484027d 100644
--- a/main/src/core/MonoDevelop.Core/MonoDevelop.Projects.SharedAssetsProjects/SharedAssetsProject.cs
+++ b/main/src/core/MonoDevelop.Core/MonoDevelop.Projects.SharedAssetsProjects/SharedAssetsProject.cs
@@ -274,7 +274,7 @@ namespace MonoDevelop.Projects.SharedAssetsProjects
 			return ProjectFeatures.None;
 		}
 
-		protected override bool OnFastCheckNeedsBuild (ConfigurationSelector configuration)
+		protected override bool OnFastCheckNeedsBuild (ConfigurationSelector configuration, TargetEvaluationContext context)
 		{
 			return false;
 		}
diff --git a/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/OperationContext.cs b/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/OperationContext.cs
index 96594b3302..41f0d3f92d 100644
--- a/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/OperationContext.cs
+++ b/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/OperationContext.cs
@@ -25,6 +25,7 @@
 // THE SOFTWARE.
 using System;
 using System.Collections.Generic;
+using MonoDevelop.Core.Execution;
 
 namespace MonoDevelop.Projects
 {
@@ -56,7 +57,13 @@ namespace MonoDevelop.Projects
 				customData = new Dictionary<object, object> (other.customData);
 			else
 				customData = null;
+			ExecutionTarget = other.ExecutionTarget;
 		}
+
+		/// <summary>
+		/// Execution target for which the operation is being executed
+		/// </summary>
+		public ExecutionTarget ExecutionTarget { get; set; }
 	}
 }
 
diff --git a/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/Project.cs b/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/Project.cs
index a1333466c8..866af7a0df 100644
--- a/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/Project.cs
+++ b/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/Project.cs
@@ -1067,7 +1067,12 @@ namespace MonoDevelop.Projects
 		/// </param>
 		public Task<TargetEvaluationResult> RunTarget (ProgressMonitor monitor, string target, ConfigurationSelector configuration, TargetEvaluationContext context = null)
 		{
-			return ProjectExtension.OnRunTarget (monitor, target, configuration, context ?? new TargetEvaluationContext ());
+			// Initialize the evaluation context. This initialization is shared with FastCheckNeedsBuild.
+			// Extenders will override OnConfigureTargetEvaluationContext to add custom properties and do other
+			// initializations required by MSBuild.
+			context = ProjectExtension.OnConfigureTargetEvaluationContext (target, configuration, context ?? new TargetEvaluationContext ());
+
+			return ProjectExtension.OnRunTarget (monitor, target, configuration, context);
 		}
 
 		public bool SupportsTarget (string target)
@@ -1086,6 +1091,24 @@ namespace MonoDevelop.Projects
 		}
 
 		/// <summary>
+		/// Initialize the evaluation context that is going to be used to execute an MSBuild target.
+		/// </summary>
+		/// <returns>The updated context.</returns>
+		/// <param name="target">Target.</param>
+		/// <param name="configuration">Configuration.</param>
+		/// <param name="context">Context.</param>
+		/// <remarks>
+		/// This method can be overriden to add custom properties and do other initializations on the evaluation
+		/// context. The method is always called before executing OnRunTarget and other methods that do
+		/// target evaluations. The method can modify the provided context instance and return it, or it can
+		/// create a new instance.
+		/// </remarks>
+		protected virtual TargetEvaluationContext OnConfigureTargetEvaluationContext (string target, ConfigurationSelector configuration, TargetEvaluationContext context)
+		{
+			return context;
+		}
+
+		/// <summary>
 		/// Runs a build or execution target.
 		/// </summary>
 		/// <returns>
@@ -1687,7 +1710,7 @@ namespace MonoDevelop.Projects
 			if (UsingMSBuildEngine (configuration)) {
 				var result = await RunMSBuildTarget (monitor, "Build", configuration, context);
 				if (!result.BuildResult.Failed)
-					SetFastBuildCheckClean (configuration);
+					SetFastBuildCheckClean (configuration, context);
 				return result;			
 			}
 			
@@ -1720,16 +1743,31 @@ namespace MonoDevelop.Projects
 
 		bool disableFastUpToDateCheck;
 
-		//the configuration of the last build that completed successfully
-		//null if any file in the project has since changed
+		// The configuration of the last build that completed successfully,
+		// null if any file in the project has since changed
 		string fastUpToDateCheckGoodConfig;
+
+		// The global properties used in the last build
+		IPropertySet fastUpToDateCheckGlobalProperties;
+
+		// Timestamp of the last build
 		DateTime fastUpToDateTimestamp;
 
 		public bool FastCheckNeedsBuild (ConfigurationSelector configuration)
 		{
-			return ProjectExtension.OnFastCheckNeedsBuild (configuration);
+			return FastCheckNeedsBuild (configuration, new TargetEvaluationContext ());
 		}
-		
+
+		public bool FastCheckNeedsBuild (ConfigurationSelector configuration, TargetEvaluationContext context)
+		{
+			// Initialize the evaluation context. This initialization is shared with RunTarget.
+			// Extenders will override OnConfigureTargetEvaluationContext to add custom properties and do other
+			// initializations required by MSBuild.
+			context = ProjectExtension.OnConfigureTargetEvaluationContext ("Build", configuration, context ?? new TargetEvaluationContext ());
+			return ProjectExtension.OnFastCheckNeedsBuild (configuration, context);
+		}
+
+		[Obsolete ("Use OnFastCheckNeedsBuild (configuration, TargetEvaluationContext)")]
 		protected virtual bool OnFastCheckNeedsBuild (ConfigurationSelector configuration)
 		{
 			if (disableFastUpToDateCheck || fastUpToDateCheckGoodConfig == null)
@@ -1738,15 +1776,52 @@ namespace MonoDevelop.Projects
 			if (cfg == null || cfg.Id != fastUpToDateCheckGoodConfig)
 				return true;
 
+			return false;
+		}
+
+		/// <summary>
+		/// Checks if this project needs to be built.
+		/// </summary>
+		/// <returns><c>true</c>, if the project is dirty and needs to be rebuilt, <c>false</c> otherwise.</returns>
+		/// <param name="configuration">Build configuration.</param>
+		/// <param name="context">Evaluation context.</param>
+		/// <remarks>
+		/// This method can be overriden to provide custom logic for checking if a project needs to be built, either
+		/// due to changes in the content or in the configuration.
+		/// </remarks>
+		protected virtual bool OnFastCheckNeedsBuild (ConfigurationSelector configuration, TargetEvaluationContext context)
+		{
+			// Chain the new OnFastCheckNeedsBuild override to the old one, so that extensions
+			// using the old API keep working
+#pragma warning disable 618
+			if (ProjectExtension.OnFastCheckNeedsBuild (configuration))
+				return true;
+#pragma warning restore 618
+
 			// Shouldn't need to build, but if a dependency was changed since this project build flag was reset,
 			// the project needs to be rebuilt
 
 			foreach (var dep in GetReferencedItems (configuration).OfType<Project> ()) {
-				if (dep.FastCheckNeedsBuild (configuration) || dep.fastUpToDateTimestamp >= fastUpToDateTimestamp) {
+				if (dep.FastCheckNeedsBuild (configuration, context) || dep.fastUpToDateTimestamp >= fastUpToDateTimestamp) {
 					fastUpToDateCheckGoodConfig = null;
 					return true;
 				}
 			}
+
+			// Check if global properties have changed
+
+			var cachedCount = fastUpToDateCheckGlobalProperties != null ? fastUpToDateCheckGlobalProperties.GetProperties ().Count () : 0;
+
+			if (cachedCount != context.GlobalProperties.GetProperties ().Count ())
+				return true;
+
+			if (cachedCount == 0)
+				return false;
+			
+			foreach (var p in context.GlobalProperties.GetProperties ()) {
+				if (fastUpToDateCheckGlobalProperties.GetValue (p.Name) != p.Value)
+					return true;
+			}
 			return false;
 		}
 
@@ -1755,10 +1830,11 @@ namespace MonoDevelop.Projects
 			fastUpToDateCheckGoodConfig = null;
 		}
 		
-		void SetFastBuildCheckClean (ConfigurationSelector configuration)
+		void SetFastBuildCheckClean (ConfigurationSelector configuration, TargetEvaluationContext context)
 		{
 			var cfg = GetConfiguration (configuration);
 			fastUpToDateCheckGoodConfig = cfg != null ? cfg.Id : null;
+			fastUpToDateCheckGlobalProperties = context.GlobalProperties;
 			fastUpToDateTimestamp = DateTime.Now;
 		}
 
@@ -3862,6 +3938,11 @@ namespace MonoDevelop.Projects
 				Project.OnWriteRunConfiguration (monitor, runConfig, properties);
 			}
 
+			internal protected override TargetEvaluationContext OnConfigureTargetEvaluationContext (string target, ConfigurationSelector configuration, TargetEvaluationContext context)
+			{
+				return Project.OnConfigureTargetEvaluationContext (target, configuration, context);
+			}
+
 			internal protected override Task<TargetEvaluationResult> OnRunTarget (ProgressMonitor monitor, string target, ConfigurationSelector configuration, TargetEvaluationContext context)
 			{
 				return Project.DoRunTarget (monitor, target, configuration, context);
@@ -3978,10 +4059,17 @@ namespace MonoDevelop.Projects
 				Project.OnPrepareForEvaluation (project);
 			}
 
+#pragma warning disable 672, 618
 			internal protected override bool OnFastCheckNeedsBuild (ConfigurationSelector configuration)
 			{
 				return Project.OnFastCheckNeedsBuild (configuration);
 			}
+#pragma warning restore 672, 618
+
+			internal protected override bool OnFastCheckNeedsBuild (ConfigurationSelector configuration, TargetEvaluationContext context)
+			{
+				return Project.OnFastCheckNeedsBuild (configuration, context);
+			}
 
 			internal protected override Task<ProjectFile []> OnGetSourceFiles (ProgressMonitor monitor, ConfigurationSelector configuration)
 			{
diff --git a/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/ProjectExtension.cs b/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/ProjectExtension.cs
index 5d32a28e6a..3544528240 100644
--- a/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/ProjectExtension.cs
+++ b/main/src/core/MonoDevelop.Core/MonoDevelop.Projects/ProjectExtension.cs
@@ -85,6 +85,19 @@ namespace MonoDevelop.Projects
 			next.OnWriteRunConfiguration (monitor, config, properties);
 		}
 
+		/// <summary>
+		/// Called to initialize a TargetEvaluationContext instance required by RunTarget()
+		/// and other methods that invoke MSBuild targets
+		/// </summary>
+		/// <returns>The initialized evaluation context (it can be just the provided context)</returns>
+		/// <param name="target">The MSBuild target that is going to be invoked</param>
+		/// <param name="configuration">Build configuration</param>
+		/// <param name="context">Execution context</param>
+		internal protected virtual TargetEvaluationContext OnConfigureTargetEvaluationContext (string target, ConfigurationSelector configuration, TargetEvaluationContext context)
+		{
+			return next.OnConfigureTargetEvaluationContext (target, configuration, context);
+		}
+
 		internal protected virtual Task<TargetEvaluationResult> OnRunTarget (ProgressMonitor monitor, string target, ConfigurationSelector configuration, TargetEvaluationContext context)
 		{
 			return next.OnRunTarget (monitor, target, configuration, context);
@@ -218,11 +231,27 @@ namespace MonoDevelop.Projects
 			}
 		}
 
+		[Obsolete ("Use OnFastCheckNeedsBuild (ConfigurationSelector,TargetEvaluationContext)")]
 		internal protected virtual bool OnFastCheckNeedsBuild (ConfigurationSelector configuration)
 		{
 			return next.OnFastCheckNeedsBuild (configuration);
 		}
 
+		/// <summary>
+		/// Checks if this project needs to be built.
+		/// </summary>
+		/// <returns><c>true</c>, if the project is dirty and needs to be rebuilt, <c>false</c> otherwise.</returns>
+		/// <param name="configuration">Build configuration.</param>
+		/// <param name="context">Evaluation context.</param>
+		/// <remarks>
+		/// This method can be overriden to provide custom logic for checking if a project needs to be built, either
+		/// due to changes in the content or in the configuration.
+		/// </remarks>
+		internal protected virtual bool OnFastCheckNeedsBuild (ConfigurationSelector configuration, TargetEvaluationContext context)
+		{
+			return next.OnFastCheckNeedsBuild (configuration, context);
+		}
+
 		#endregion
 
 		#region Events
diff --git a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide/ProjectOperations.cs b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide/ProjectOperations.cs
index 19e3b067ca..a88c448dfc 100644
--- a/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide/ProjectOperations.cs
+++ b/main/src/core/MonoDevelop.Ide/MonoDevelop.Ide/ProjectOperations.cs
@@ -1182,7 +1182,7 @@ namespace MonoDevelop.Ide
 			BuildResult res = null;
 			try {
 				tt.Trace ("Cleaning item");
-				res = await entry.Clean (monitor, IdeApp.Workspace.ActiveConfiguration, operationContext);
+				res = await entry.Clean (monitor, IdeApp.Workspace.ActiveConfiguration, InitOperationContext (entry, operationContext));
 			} catch (Exception ex) {
 				monitor.ReportError (GettextCatalog.GetString ("Clean failed."), ex);
 			} finally {
@@ -1435,13 +1435,13 @@ namespace MonoDevelop.Ide
 
 			var sei = target as Project;
 			if (sei != null) {
-				if (sei.FastCheckNeedsBuild (configuration))
+				if (sei.FastCheckNeedsBuild (configuration, InitOperationContext (target, new TargetEvaluationContext ())))
 					return true;
 				//TODO: respect solution level dependencies
 				var deps = new HashSet<SolutionItem> ();
 				CollectReferencedItems (sei, deps, configuration);
 				foreach (var dep in deps.OfType<Project> ()) {
-					if (dep.FastCheckNeedsBuild (configuration))
+					if (dep.FastCheckNeedsBuild (configuration, InitOperationContext (target, new TargetEvaluationContext ())))
 						return true;
 				}
 				return false;
@@ -1450,7 +1450,7 @@ namespace MonoDevelop.Ide
 			var sln = target as Solution;
 			if (sln != null) {
 				foreach (var item in sln.GetAllProjects ()) {
-					if (item.FastCheckNeedsBuild (configuration))
+					if (item.FastCheckNeedsBuild (configuration, InitOperationContext (target, new TargetEvaluationContext ())))
 						return true;
 				}
 				return false;
@@ -1517,7 +1517,7 @@ namespace MonoDevelop.Ide
 
 				if (skipPrebuildCheck || result.ErrorCount == 0) {
 					tt.Trace ("Building item");
-					result = await entry.Build (monitor, IdeApp.Workspace.ActiveConfiguration, true, operationContext);
+					result = await entry.Build (monitor, IdeApp.Workspace.ActiveConfiguration, true, InitOperationContext (entry, operationContext));
 				}
 			} catch (Exception ex) {
 				monitor.ReportError (GettextCatalog.GetString ("Build failed."), ex);
@@ -1534,6 +1534,25 @@ namespace MonoDevelop.Ide
 
 			return result;
 		}
+
+		/// <summary>
+		/// Initializes the context to be used for build operations. It currently just initializes
+		/// it with the currently selected execution target.
+		/// </summary>
+		T InitOperationContext<T> (IBuildTarget target, T context) where T:OperationContext
+		{
+			OperationContext ctx = context;
+			if (ctx == null)
+				ctx = new OperationContext ();
+			if (ctx.ExecutionTarget == null) {
+				var item = target as SolutionItem;
+				if (item != null)
+					ctx.ExecutionTarget = IdeApp.Workspace.GetActiveExecutionTarget (item);
+				else
+					ctx.ExecutionTarget = IdeApp.Workspace.ActiveExecutionTarget;
+			}
+			return (T)ctx;
+		}
 		
 		// Note: This must run in the main thread
 		async Task PromptForSave (BuildResult result)
diff --git a/main/tests/UnitTests/MonoDevelop.Projects/ProjectTests.cs b/main/tests/UnitTests/MonoDevelop.Projects/ProjectTests.cs
index 1eee00dde6..182934fd1a 100644
--- a/main/tests/UnitTests/MonoDevelop.Projects/ProjectTests.cs
+++ b/main/tests/UnitTests/MonoDevelop.Projects/ProjectTests.cs
@@ -1154,6 +1154,35 @@ namespace MonoDevelop.Projects
 			var asms = await p.GetReferencedAssemblies (p.Configurations [0].Selector);
 			Assert.IsTrue (asms.Any (r => r.FilePath.FileName == "System.Runtime.dll"));
 		}
+
+		[Test]
+		public async Task FastCheckNeedsBuildWithContext ()
+		{
+			string solFile = Util.GetSampleProject ("fast-build-test", "FastBuildTest.sln");
+			Solution sol = (Solution)await Services.ProjectService.ReadWorkspaceItem (Util.GetMonitor (), solFile);
+			var app = (DotNetProject)sol.Items [0];
+
+			var cs = new SolutionConfigurationSelector ("Debug");
+
+			var ctx = new TargetEvaluationContext ();
+			ctx.GlobalProperties.SetValue ("Foo", "Bar");
+
+			Assert.IsTrue (app.FastCheckNeedsBuild (cs, ctx));
+
+			ctx = new TargetEvaluationContext ();
+			ctx.GlobalProperties.SetValue ("Foo", "Bar");
+
+			var res = await sol.Build (Util.GetMonitor (), cs, ctx);
+			Assert.IsFalse (res.HasErrors);
+
+			ctx = new TargetEvaluationContext ();
+			ctx.GlobalProperties.SetValue ("Foo", "Bar");
+			Assert.IsFalse (app.FastCheckNeedsBuild (cs, ctx));
+
+			ctx = new TargetEvaluationContext ();
+			ctx.GlobalProperties.SetValue ("Foo", "Modified");
+			Assert.IsTrue (app.FastCheckNeedsBuild (cs, ctx));
+		}
 	}
 
 	class SerializedSaveTestExtension: SolutionItemExtension