Cannot update an imported file within a build

[rainersigwald]

This was reported offline by @kzu. Thanks!

In this situation:

1. A multi-project build with a "coordinating" project (like a .sln build).
2. The coordinating project calls into leaf projects with one set of global properties to extract some information.
3. The coordinating project updates files imported by leaf projects (think "NuGet restore").
4. The coordinating project then builds leaf projects again with a different set of global properties (so they get fully re-evaluated/rebuilt).

The builds in step 4 do not pick up the new versions of files updated by step 3--they continue to use the file contents from their original form read in step 2.

This breaks multi-step restore + build in a single invocation, even though the evaluation and builds are separated by being in different global property spaces.

This is happening because of caching of imported file contents done by ProjectRootElementCache. The cache can be set to invalidate its entries based on file timestamp checks by constructor argument, but this is not done by the relevant call in a command-line build, from this callstack:

    Microsoft.Build.dll!Microsoft.Build.Evaluation.ProjectRootElementCache.ProjectRootElementCache(bool autoReloadFromDisk) Line 137
    Microsoft.Build.dll!Microsoft.Build.Evaluation.ProjectCollection.ProjectCollection(System.Collections.Generic.IDictionary<string, string> globalProperties, System.Collections.Generic.IEnumerable<Microsoft.Build.Framework.ILogger> loggers, System.Collections.Generic.IEnumerable<Microsoft.Build.Logging.ForwardingLoggerRecord> remoteLoggers, Microsoft.Build.Evaluation.ToolsetDefinitionLocations toolsetDefinitionLocations, int maxNodeCount, bool onlyLogCriticalEvents) Line 256
    MSBuild.exe!Microsoft.Build.CommandLine.MSBuildApp.BuildProject(string projectFile, string[] targets, string toolsVersion, System.Collections.Generic.Dictionary<string, string> globalProperties, Microsoft.Build.Framework.ILogger[] loggers, Microsoft.Build.Framework.LoggerVerbosity verbosity, Microsoft.Build.CommandLine.DistributedLoggerRecord[] distributedLoggerRecords, bool needToValidateProject, string schemaFile, int cpuCount, bool enableNodeReuse, System.IO.TextWriter preprocessWriter, bool debugger, bool detailedSummary) Line 920
    MSBuild.exe!Microsoft.Build.CommandLine.MSBuildApp.Execute(string commandLine) Line 545
    MSBuild.exe!Microsoft.Build.CommandLine.MSBuildApp.Main() Line 202

There's no way to specify that the cache should check for updated targets within a build. Flipping that bit with a debugger produces the expected result (from the MSBUILDDEBUGXMLCACHE=1 debug log):

P 8232 | Satisfied from XML cache: C:\Program Files (x86)\MSBuild\Microsoft\NuGet\Microsoft.NuGet.targets
P 8232 | Forgetting: S:\NuGetizer\src\Build\Sample\Sample.nuget.targets
P 8232 | Out of date dropped from XML cache: S:\NuGetizer\src\Build\Sample\Sample.nuget.targets
P 8232 | Adding: S:\NuGetizer\src\Build\Sample\Sample.nuget.targets
P 8232 | Satisfied from XML cache: C:\Users\raines\.nuget\packages\GitInfo\1.1.32\build\GitInfo.targets

[rainersigwald]

I don't know why the default cache is constructed with autoReloadFromDisk = false. I can handwave at "perf reasons" but that doesn't seem like it's an automatic deal killer. And OutOfProcNode sets it to true.

It seems like the fix here is to check timestamps by default.

[rainersigwald]

@rohit21agrawal this might be relevant to you too

[rainersigwald]

Comments from standup today:

• Provide an escape-hatch environment variable
• Check on a large solution (Chromium?) to check perf impact of the extra up-to-date checks
• [if perf implications] Could this be set as a property on the MSBuild task somehow?

[kzu]

👍!

[rainersigwald]

Discovered additional considerations here after talking with some folks on the Live Unit Testing team. They are deliberately taking a snapshot of a set of projects by loading them into the cache, and relying on that to ensure that project-to-project references see a consistent view of the world, even while the user is typing.

Unfortunately, we now have conflicting desires: this issue, and the ability to take a snapshot. Since the legacy behavior allowed the LUT folks to get snapshots, I'm going to defer to that, meaning we can't solve this the "easy way" I proposed in the PR and need a more sophisticated design.

[kzu]

Hm. How about an explicit property (that doesn't sound as"scary" as
MSBUILDDEBUGXMLCACHE) that triggers this "live project updates" behavior?

On Mon, Nov 21, 2016, 2:21 PM Rainer Sigwald notifications@github.com
wrote:

Discovered additional considerations here after talking with some folks on
the Live Unit Testing
https://blogs.msdn.microsoft.com/visualstudio/2016/11/18/live-unit-testing-visual-studio-2017-rc/
team. They are deliberately taking a snapshot of a set of projects by
loading them into the cache, and relying on that to ensure that
project-to-project references see a consistent view of the world, even
while the user is typing.

Unfortunately, we now have conflicting desires: this issue, and the
ability to take a snapshot. Since the legacy behavior allowed the LUT folks
to get snapshots, I'm going to defer to that, meaning we can't solve this
the "easy way" I proposed in the PR and need a more sophisticated design.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1185 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAKW683ysxOVI8ciqHqQzP28dDhPzf2hks5rAdMRgaJpZM4KU5LR
.

/kzu from mobile

[livarcocc]

Team triage: this is not really feasible. Closing.