Skip to content

Migrate AspNetCompiler to TaskEnvironment API#13424

Merged
OvesN merged 30 commits intodotnet:mainfrom
OvesN:dev/veronikao/Migrate-AspNetCompiler-task-to-TaskEnvironment
Apr 8, 2026
Merged

Migrate AspNetCompiler to TaskEnvironment API#13424
OvesN merged 30 commits intodotnet:mainfrom
OvesN:dev/veronikao/Migrate-AspNetCompiler-task-to-TaskEnvironment

Conversation

@OvesN
Copy link
Copy Markdown
Contributor

@OvesN OvesN commented Mar 20, 2026

Fixes #13421

Context

The AspNetCompiler task was made thread-safe.

Changes Made

AspNetCompiler.cs

  • Routed file existence checks and path resolution through TaskEnvironment.GetAbsolutePath().

Testing

AspNetCompiler_Tests.cs

  • GenerateFullPathToTool_ReturnsAbsolutePathOrNull — validates the TaskEnvironment.GetAbsolutePath() integration by constructing a MultiThreadedTaskEnvironmentDriver with a custom project directory and verifying the returned path is absolute (or null when the tool is not found).
  • GetProcessStartInfo_UsesTaskEnvironmentWorkingDirectory — verifies that the working directory from MultiThreadedTaskEnvironmentDriver is propagated to the child process ProcessStartInfo.

Notes

@OvesN OvesN self-assigned this Mar 20, 2026
@OvesN OvesN changed the base branch from main to dev/veronikao/migrate-Al-task-to-TaskEnvironment-API March 20, 2026 16:22
@OvesN OvesN changed the title Migrate aspNetCompiler task to taskEnvironment API Migrate AspNetCompilertask to TaskEnvironment API Mar 20, 2026
@OvesN OvesN marked this pull request as ready for review March 20, 2026 16:36
@OvesN OvesN changed the title Migrate AspNetCompilertask to TaskEnvironment API Migrate AspNetCompiler to TaskEnvironment API Mar 20, 2026
OvesN and others added 2 commits March 23, 2026 13:54
…nto dev/veronikao/Migrate-AspNetCompiler-task-to-TaskEnvironment
@OvesN OvesN requested review from AR-May and JanProvaznik March 23, 2026 12:59
Copy link
Copy Markdown
Member

@AR-May AR-May left a comment

Choose a reason for hiding this comment

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

Looks good, but might require some changes after merging #13423.

@OvesN OvesN marked this pull request as draft March 26, 2026 16:03
@OvesN OvesN marked this pull request as ready for review March 26, 2026 16:15
OvesN and others added 5 commits March 26, 2026 17:22
The test relied on FileAttributes.ReadOnly to force File.Delete to throw,
but this is not cross-platform - Unix read-only attributes don't prevent deletion.
ApplyEnvironmentOverrides  moved inside SetUpProcessStartInfo.
Added override for DeleteTempFile using AbsolutePath.
Made Driver private again, expose only enum value to decided if driver is multitheaded.
Made GetProcessStartInfoMultithreadable private.
add overload for DeleteTempFile with AbsolutePath argument
…nto dev/veronikao/Migrate-AspNetCompiler-task-to-TaskEnvironment
Comment thread src/Tasks/AspNetCompiler.cs Outdated
Comment thread src/Tasks/AspNetCompiler.cs Outdated
Comment thread src/Tasks/AspNetCompiler.cs
@JanProvaznik
Copy link
Copy Markdown
Member

JanProvaznik commented Apr 7, 2026

oh it needs retarget to msbuild main, and please check there are no AspNetCompiler warnings in Linux Core Multithreaded Mode CI job

@OvesN OvesN changed the base branch from dev/veronikao/migrate-Al-task-to-TaskEnvironment-API to main April 8, 2026 06:37
Copilot AI review requested due to automatic review settings April 8, 2026 09:23
OvesN and others added 2 commits April 8, 2026 11:24
Co-authored-by: Jan Provazník <janpro@janpro.dev>
Copy link
Copy Markdown
Contributor

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 continues the TaskEnvironment migration by marking AspNetCompiler as multi-threadable and routing tool-path handling through TaskEnvironment APIs, with accompanying unit test updates to validate behavior in a multithreaded environment.

Changes:

  • Marked AspNetCompiler as [MSBuildMultiThreadableTask] and updated GenerateFullPathToTool() to return a TaskEnvironment-absolutized tool path.
  • Updated AspNetCompiler unit tests to use ITestOutputHelper-backed engines/loggers and added NETFRAMEWORK-only tests for TaskEnvironment integration.
  • Added TaskEnvironment.DriverKind plumbing and updated ToolTask.GetProcessStartInfo() flow to branch for multithreaded drivers.

Reviewed changes

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

File Description
src/Utilities/ToolTask.cs Adjusts GetProcessStartInfo flow to account for multithreaded TaskEnvironment execution.
src/Tasks/AspNetCompiler.cs Marks task multi-threadable and absolutizes resolved tool path via TaskEnvironment.
src/Tasks.UnitTests/AspNetCompiler_Tests.cs Adds TaskEnvironment-focused tests (NETFRAMEWORK) and plumbs xUnit output into engines/loggers.
src/Framework/TaskEnvironment.cs Introduces internal driver-kind identification used by Utilities to branch behavior.

Comment thread src/Tasks.UnitTests/AspNetCompiler_Tests.cs
@OvesN OvesN requested a review from SimaTian April 8, 2026 11:57
@OvesN OvesN merged commit fe7af3a into dotnet:main Apr 8, 2026
10 checks passed
dfederm pushed a commit to dfederm/msbuild that referenced this pull request Apr 9, 2026
Fixes dotnet#13421

## Context

The AspNetCompiler task was made thread-safe.

## Changes Made

### AspNetCompiler.cs
- Routed file existence checks and path resolution through
TaskEnvironment.GetAbsolutePath().
## Testing

### AspNetCompiler_Tests.cs

- **`GenerateFullPathToTool_ReturnsAbsolutePathOrNull`** — validates the
`TaskEnvironment.GetAbsolutePath()` integration by constructing a
`MultiThreadedTaskEnvironmentDriver` with a custom project directory and
verifying the returned path is absolute (or null when the tool is not
found).
- **`GetProcessStartInfo_UsesTaskEnvironmentWorkingDirectory`** —
verifies that the working directory from
`MultiThreadedTaskEnvironmentDriver` is propagated to the child process
`ProcessStartInfo`.
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.

Migrate AspNetCompiler task to TaskEnvironment API

5 participants