Add support and tests for interpolating MSBuild properties and environment variables in launchProfile commandLineArgs #50159
Conversation
…nment variables in launchProfile commandLineArgs
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for interpolating MSBuild properties and environment variables in launch profile commandLineArgs, addressing issue #50157. The implementation follows the same property interpolation approach used by the project system.
- Adds property expansion functionality to
RunCommandwith a three-tier precedence system - Updates method signatures to pass
ProjectInstancefor property resolution - Includes comprehensive test coverage for the new interpolation feature
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/Cli/dotnet/Commands/Run/RunCommand.cs |
Core implementation of MSBuild property interpolation with regex-based expansion and precedence handling |
src/Cli/dotnet/Commands/Run/Api/RunApiCommand.cs |
Updates method calls to accommodate new signature requiring ProjectInstance parameter |
test/dotnet.Tests/CommandTests/Run/GivenDotnetRunBuildsCsProj.cs |
Adds comprehensive test for property interpolation functionality |
test/TestAssets/TestProjects/TestAppWithLaunchSettings/Program.cs |
Simplifies argument handling to support multiple interpolated arguments in tests |
.vscode/launch.json |
Minor correction to SDK version path format |
Co-authored-by: Copilot <[email protected]>
Youssef1313
left a comment
Youssef1313
left a comment
There was a problem hiding this comment.
This probably creates a divergence between dotnet run and dotnet test :/
If hard to address in this PR, can you please open an issue to track this?
|
|
||
| (var targetCommand, var project) = runCommand.GetTargetCommand(buildCommand.CreateProjectInstance); | ||
| runCommand.ApplyLaunchSettingsProfileToCommand(targetCommand, launchSettings, project); | ||
There was a problem hiding this comment.
nit: unnecessary whitespace on a blank line
| /// </summary> | ||
| /// <param name="projectFactory"></param> | ||
| /// <returns></returns> | ||
| /// <exception cref="GracefulException"></exception> |
There was a problem hiding this comment.
Consider removing empty doc comments.
| } | ||
|
|
||
| [GeneratedRegex(@"\$\((?<token>[^\)]+)\)", RegexOptions.IgnoreCase, "en-US")] | ||
| private static partial Regex MSBuildPropertyUsagePattern(); |
There was a problem hiding this comment.
Consider using a partial property instead:
| private static partial Regex MSBuildPropertyUsagePattern(); | |
| private static partial Regex MSBuildPropertyUsagePattern { get; } |
| // So we can skip project evaluation to continue the optimized path. | ||
| Debug.Assert(EntryPointFileFullPath is not null); | ||
| return CreateCommandForCscBuiltProgram(EntryPointFileFullPath); | ||
| return (CreateCommandForCscBuiltProgram(EntryPointFileFullPath), null); |
There was a problem hiding this comment.
It's worth noting that the direct-Csc usage of dotnet run doesn't support this feature because there is no ProjectInstance. It's possible that this gap could be filled with a proxy or a static set of Properties or something.
Another option would be to fall back to MSBuild if there are any $(...) holes in the launch profile to ensure correctness.
I think we should file a follow up issue to track this.
4 participants
Fixes #50157 by doing the same kind of property-interpolation as the project-system does.
We choose the following 'layering':
-eenvironment variables specified by the userIt's worth noting that the direct-Csc usage of
dotnet rundoesn't support this feature because there is no ProjectInstance. It's possible that this gap could be filled with a proxy or a static set of Properties or something.