Skip to content

Enhance IBuildEngine3 Yield/Reacquire documentation with task requirements #12014

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

Merged
merged 4 commits into from
Jun 30, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 13, 2025

The documentation for IBuildEngine3.Yield() and IBuildEngine3.Reacquire() methods was too minimal and didn't explain the critical requirements and burdens that using these methods imposes on tasks.

Changes Made

Enhanced the XML documentation for both methods to include detailed <remarks> sections that explain:

  1. Global process state changes: After calling Yield(), global process state like environment variables and current working directory can change arbitrarily until Reacquire() returns.

  2. Task requirements: If tasks depend on any global state (e.g., opening files by relative path rather than calling ITaskItem.GetMetadata("FullPath")), they must complete that work before calling Yield().

  3. Common pattern: The recommended approach is to figure out what all the long-running work is and start it before yielding.

Before

/// <summary>
/// Informs the system that this task has a long-running out-of-process component and other work can be done in the
/// build while that work completes.
/// </summary>
void Yield();

/// <summary>
/// Waits to reacquire control after yielding.
/// </summary>
void Reacquire();

After

/// <summary>
/// Informs the system that this task has a long-running out-of-process component and other work can be done in the
/// build while that work completes.
/// </summary>
/// <remarks>
/// After calling <see cref="Yield"/>, global process state like environment variables and current working directory 
/// can change arbitrarily until <see cref="Reacquire"/> returns. As a result, if you are going to depend on any of 
/// that state, for instance by opening files by relative path rather than calling 
/// <c>ITaskItem.GetMetadata("FullPath")</c>, you must do so before calling <see cref="Yield"/>. 
/// The common pattern is to figure out what all the long-running work is and start it before yielding.
/// </remarks>
void Yield();

/// <summary>
/// Waits to reacquire control after yielding.
/// </summary>
/// <remarks>
/// This method must be called to regain control after <see cref="Yield"/> has been called. 
/// After <see cref="Yield"/> is called, global process state like environment variables and current 
/// working directory can change arbitrarily until this method returns. Tasks should complete any work 
/// that depends on global process state before calling <see cref="Yield"/>.
/// </remarks>
void Reacquire();

This documentation enhancement will help task authors understand the proper usage patterns and avoid common pitfalls when implementing long-running tasks that need to yield control.

Fixes #12013.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dnceng.pkgs.visualstudio.com
    • Triggering command: dotnet build src/Framework/Microsoft.Build.Framework.csproj (dns block)
    • Triggering command: dotnet build src/Utilities.UnitTests/Microsoft.Build.Utilities.UnitTests.csproj (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copy link
Contributor

Hello @@copilot, I noticed that you’re changing an .swr file or any file under src/Package/MSBuild.VSSetup.. Please make sure to validate this change by an experimental VS insertion. This is accomplished by pushing to an exp/* branch, which requires write permissions to this repo.

…requirements

Co-authored-by: rainersigwald <3347530+rainersigwald@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Yield/Reacquire docs don't talk about task requirements Enhance IBuildEngine3 Yield/Reacquire documentation with task requirements Jun 13, 2025
@Copilot Copilot AI requested a review from rainersigwald June 13, 2025 15:29
Copilot finished work on behalf of rainersigwald June 13, 2025 15:29
Comment on lines 61 to 64
/// This method must be called to regain control after <see cref="Yield"/> has been called.
/// After <see cref="Yield"/> is called, global process state like environment variables and current
/// working directory can change arbitrarily until this method returns. Tasks should complete any work
/// that depends on global process state before calling <see cref="Yield"/>.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is interesting--just say it for Yield.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simplified the Reacquire() documentation to remove the redundant global state details, keeping only the essential requirement to call it after Yield(). The detailed explanation is now only in the Yield() method documentation. (72f4132)

Co-authored-by: rainersigwald <3347530+rainersigwald@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jun 13, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dnceng.pkgs.visualstudio.com
    • Triggering command: dotnet build src/Framework/Microsoft.Build.Framework.csproj --no-restore (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot finished work on behalf of rainersigwald June 13, 2025 15:50
Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

@ghogen if you get a second could you look at this doc change?

Co-authored-by: Gordon Hogenson <ghogen@microsoft.com>
@YuliiaKovalova YuliiaKovalova merged commit 822ba23 into main Jun 30, 2025
10 checks passed
@YuliiaKovalova YuliiaKovalova deleted the copilot/fix-12013 branch June 30, 2025 09:37
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.

Yield/Reacquire docs don't talk about task requirements
4 participants