Skip to content

Add the merge logic to BuildResult for handling RequestedProjectState, BuildFlags and ProjectInstance reliably #11765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

YuliiaKovalova
Copy link
Member

Fixes #11701

Context

The commit addresses an issue where Visual Studio hangs during build operations, due to inconsistent project state merging after build operations. The root cause appears to be how BuildResult instances handle merging of project state when multiple build operations are combined (per the same ConfigurationId).

Why These Changes Were Made

The core issue appears to be inconsistent handling of project state during build operations, particularly when:

  • Multiple build operations need to be merged

  • Different build operations request different subsets of project state

  • The merge operation doesn't preserve all necessary metadata, properties, or flags
    Prior to this change, when build results were merged using the MergeResults method, the logic didn't properly handle the merging of project state after build.
    The new implementation explicitly merges:

  • The project properties

  • The project items and their metadata

  • The requested state filters

  • The build flags

Changes Made

  1. Two significant methods were added to BuildResult:
    MergeProjectStateAfterBuildInstances: This method properly merges project instances, including their properties, items, and requested state filters. It handles cases where instances might be null, or when one instance has a full project state versus a partial state.
    MergeBuildFlags: This method provides explicit control over how build flags are merged, with special handling for the ProvideProjectStateAfterBuild and ProvideSubsetOfStateAfterBuild flags.
  2. Enhanced RequestedProjectState with Merge Capability
    Added a new Merge() method to RequestedProjectState that properly combines property filters and item filters from two different instances.
    Implemented sophisticated merging logic for item filters that preserves metadata from both sources.
  3. Improved Robustness in ProjectInstance
    Enhanced the FilteredCopy constructor to handle null collections gracefully.
    Updated code to use modern C# collection initializers and null-safe operations.
    Made the handling of various dictionaries (properties, items, etc.) more robust against null values.
  4. Changed the logic in ResultsCache.AreBuildResultFlagsCompatible
    If a subset of data is requested, we don't need to rely on the cache flag, but check if the available subset of the data can satisfy the request.

Testing

Added UTs to cover new Merge Logic

Notes

@Copilot Copilot AI review requested due to automatic review settings April 25, 2025 14:19
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements a robust merge logic for BuildResult by improving project state merging and build flag handling to resolve issues with inconsistent project state after build operations. Key changes include:

  • Enhancements in ProjectInstance to safely clone collections and properties, utilizing modern C# syntax.
  • New merge methods in BuildResult to combine build flags and project state, including properties, items, and filter states.
  • Expanded functionality and unit tests for RequestedProjectState to accurately merge property and item filters.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Build/Instance/ProjectInstance.cs Uses new C# constructs and null-safe operations when cloning collections and copying arrays.
src/Build/BackEnd/Shared/BuildResult.cs Adds merge methods for build flags and project state with local functions to merge properties and items.
src/Build/BackEnd/Components/Caching/ResultsCache.cs Refactors constructors and flag comparisons using concise lambda expressions.
src/Build/BackEnd/BuildManager/RequestedProjectState.cs Introduces a Merge() method to combine property and item filters using modern collection syntax.
src/Build.UnitTests/BackEnd/BuildResult_Tests.cs Provides thorough new tests validating the merged state behavior for project items, properties, and filters.
Comments suppressed due to low confidence (1)

src/Build/BackEnd/Shared/BuildResult.cs:844

  • [nitpick] Consider extracting the local merge functions (MergeProperties and MergeItems) into separate private methods to improve readability and maintainability.
void MergeProperties(ICollection<ProjectPropertyInstance> newProperties) { ... }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VS DTB Hangs
1 participant