Support laxer isolation mode `-isolate:Message` #8249

DmitriyShepelev · 2022-12-15T21:34:16Z

Context

Support a laxer isolation mode -isolate:MessageUponIsolationViolation (or its short form -isolate:Message) for builds unable to adhere to the restrictive isolation mode -isolate:True, piggybacking on the existing skipStaticGraphIsolationConstraints flag. Isolation violations are logged as messages.

Changes

Allow a new value for the -isolate switch: MessageUponIsolationViolation (short form Message).
In this new mode, only the TargetResults for top-level targets are serialized into the output cache. This is to mitigate the chances of isolation-violating dependency targets relying on state potentially mutated by cached dependency targets. (E.g., the definition of a property.) As such, it is still possible for a dependent project to have a config entry C in its override caches and still need to obtain BuildResults from C in isolation violation, requiring placing them in the override result cache instead of the current cache to maintain the invariant of no overlap configs across the override and current caches.
Preserve the IsolateProjects getter/setter for API backwards compat.

src/Build/BackEnd/BuildManager/IsolateProjects.cs

src/Build.UnitTests/BackEnd/MockHost.cs

src/Build/PublicAPI/netstandard/PublicAPI.Unshipped.txt

src/Build/Resources/Strings.resx

src/Build/PublicAPI/netstandard/PublicAPI.Shipped.txt

src/Build.UnitTests/Graph/IsolateProjects_Tests.cs

src/Build.UnitTests/BackEnd/TaskBuilder_Tests.cs

DmitriyShepelev · 2022-12-19T17:45:52Z

Discussed offline with @Forgind and @dfederm:

Avert making breaking change to the BuildParameters.IsolateProjects.get API by wrapping existing isolateProjects boolean with the created ProjectIsolationMode enum.
2. When dependent project P1 needs to obtain the cache results for targets t1, t2, ..., tm from a dependency project P2 for which P1 already has cache results for targets tm+1, tm+2, ..., tn such that m > 0 and m < n, the cache results for tm+1, tm+2, ..., tn are invalidated and all targets in P2 are built as if -isolate was set to False. This is to prevent incorrect behavior, such as that covered by the no-longer-necessary MSB4047. Note that this change means that the other bug fix regarding not placing any isolation-violating target results into the current cache if the corresponding project's configuration already exists in the override cache is no longer necessary.
Placing the bug fix for excluding the cache results for dependency projects built in violation of isolation mode from the output cache file into a separate PR.

…4386) (#8257) Fixes #4386 Context Any cache entries from projects excluded from isolation constraints should be excluded from the output results cache file. Changes Made Only the cache entry with the smallest configuration ID (that of the project to be built in isolation) should be serialized into the output results cache file. As described in #4386, this prevents the case where dependency projects pass down the same cache entry obtained through skipping/violating isolation constraints to a dependent project, creating duplicate input cache entries. Testing Added a UT. Notes Addressing this issue since it came up frequently when testing #8249.

src/Build/BackEnd/BuildManager/ProjectIsolationMode.cs

src/Build/BackEnd/Shared/BuildResult.cs

src/Build/BackEnd/BuildManager/BuildManager.cs

src/Build/BackEnd/BuildManager/BuildParameters.cs

src/Build.UnitTests/BackEnd/MockHost.cs

rainersigwald

I'm very confused by the Targets list and would like clarity on that before digging back in.

src/MSBuild/XMake.cs

src/Build/BackEnd/BuildManager/BuildParameters.cs

rainersigwald

Overall LGTM but some comments to clarify the fairly complicated flow and tests here would be highly appreciated.

src/Build/BackEnd/Components/Scheduler/Scheduler.cs

src/Build/BackEnd/BuildManager/BuildManager.cs

src/Build.UnitTests/Graph/IsolateProjects_Tests.cs

src/Build.UnitTests/BackEnd/CacheSerialization_Tests.cs

src/Build.UnitTests/BackEnd/Scheduler_Tests.cs

…ojects' result caches

…ng state

…ied to build

src/Shared/UnitTests/ObjectModelHelpers.cs

src/Build/BackEnd/Components/RequestBuilder/RequestBuilder.cs

src/Build/BackEnd/Components/RequestBuilder/TaskBuilder.cs

src/Build/BackEnd/Shared/BuildResult.cs

src/MSBuild/Resources/Strings.resx

src/MSBuild/XMake.cs

…#8330) Fixes #4252 Context This PR extracts the relevant logic from the closed #5297, which adds support for the SkipNonexistentTargets metadatum on the ProjectReferenceTargets item: <ProjectReferenceTargets Include='<some-target>' Targets='<some-target1>;<some-target2>' SkipNonexistentTargets='<boolean>'> If SkipNonexistentTargets is true, then any targets in Targets are skipped if they're nonexistent. SkipNonexistentTargets cannot be added to ProjectReferenceTargets items whose Targets contain .default or .projectReferenceTargetsOrDefaultTargets, which represent the default targets and targets specified on the ProjectReference item (with fallback to default targets if none are specified), respectively. Changes Made GetTargetLists filters out skippable nonexistent targets on referenced projects. Determining whether a target is skippable required storing the defined project targets in BuildResults to ensure that corresponding BuildRequestConfigurations on the build manager node have set project targets if the build manager node created a configuration based on a request from an external node but hadn't received a result (since the project may not have been loaded locally and thus the project targets would be unknown). Added SkipNonexistentTargets='true' to GetTargetFrameworks since in the non-graph case it is added to the relevant MSBuild task. Testing UTs and manual testing with /graph and /graph /isolate (the latter run without restore being called due to #6856) on the erroring repos I saw when testing #8249. Notes Addressing this since it came up when testing #8249.

Update documentation given the merge of #8249, #8257, and #8330. --------- Co-authored-by: Rainer Sigwald <[email protected]>

dfederm reviewed Dec 15, 2022

View reviewed changes

src/Build/BackEnd/BuildManager/IsolateProjects.cs Outdated Show resolved Hide resolved