-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add Executable launch profile support to dotnet run and dotnet watch #51727
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
base: release/10.0.2xx
Are you sure you want to change the base?
Add Executable launch profile support to dotnet run and dotnet watch #51727
Conversation
Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
baronfel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Good initial attempt, but we can clean this up some. Please address my review comments.
src/Cli/dotnet/Commands/Run/LaunchSettings/ExecutableLaunchSettingsProvider.cs
Outdated
Show resolved
Hide resolved
- Created base LaunchSettingsModel with ProfileKind discriminator - Derived ProjectLaunchSettingsModel and ExecutableLaunchSettingsModel - Added JSON binding models for deserialization - Updated providers to use JsonSerializer.Deserialize instead of manual traversal - Exposed SupportedProfileTypes from LaunchSettingsManager - Updated LaunchSettingsProfile.cs to use array of supported types with comment - Updated RunCommand to use pattern matching on ProfileKind - Split Execute method into ExecuteWithProjectProfile and ExecuteWithExecutableProfile - Updated all method signatures to use base LaunchSettingsModel type Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Changed hardcoded error message to use CliCommandStrings.LaunchProfileHandlerCannotBeLocated for proper localization support. Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Simplified argument building logic to make it clearer that ApplicationArgs are properly escaped before being combined with launchSettings.CommandLineArgs. Added comments explaining that CommandLineArgs from JSON cannot be reliably parsed into tokens. Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
| } | ||
|
|
||
| private static ProjectLaunchSettingsModel? TryGetLaunchProfileSettings(string projectDirectory, string projectNameWithoutExtension, string appDesignerFolder, BuildOptions buildOptions, string? profileName) | ||
| private static LaunchSettingsModel? TryGetLaunchProfileSettings(string projectDirectory, string projectNameWithoutExtension, string appDesignerFolder, BuildOptions buildOptions, string? profileName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dotnet/dotnet-testing if we expand the 'launch settings' handlers that are known, how should the testing experience react? does test support launch settings profiles today?
| .WorkingDirectory(workingDirectory); | ||
|
|
||
| // Apply environment variables from launch profile | ||
| ApplyLaunchSettingsProfileToCommand(command, launchSettings); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like starting from here the code is the same as ExecuteWithProjectProfile.
Might be better to refactor these methods to methods that return a command based on the profile and then common code that sets env variables, sends telemetry and executes the command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot do this refactoring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored in 3704366. Extracted the common execution logic (applying launch settings, environment variables, telemetry, Ctrl-C handler, and command execution) into a new ExecuteCommand method. Both ExecuteWithProjectProfile and ExecuteWithExecutableProfile now call this method, eliminating the code duplication.
| internal void ApplyLaunchSettingsProfileToCommand(ICommand targetCommand, ProjectLaunchSettingsModel? launchSettings) | ||
| private int ExecuteWithExecutableProfile(ExecutableLaunchSettingsModel launchSettings) | ||
| { | ||
| Debug.Assert(launchSettings.ExecutablePath != null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be good idea to add more tests that validate the application of environment expansion and how command line args are constructed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot please add tests that verify that the executable command handler applies the launch profile settings to the generated command correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comprehensive tests in 33d9898. The new tests verify:
- Environment variables application: Tests that environment variables from the launch profile are correctly applied to the executed command
- Working directory settings: Verifies the working directory from the profile is used
- Command line arguments combination: Tests that arguments from the profile and command line are properly combined
- Environment variable expansion: Validates that executable paths are correctly resolved
Created two new test assets:
AppForExecutableProfile: Helper app that prints environment variables and argumentsAppWithDetailedExecutableProfile: Test project with multiple executable profiles testing different scenarios
All 6 executable profile tests now pass along with the 11 existing launch settings tests.
Extracted the common code from ExecuteWithProjectProfile and ExecuteWithExecutableProfile into a new ExecuteCommand method. This method handles: - Applying launch settings to the command - Applying environment variables from command line - Sending telemetry - Setting up Ctrl-C handler - Executing the command This reduces code duplication and makes the execution flow clearer. Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Added tests to verify that the executable command handler correctly applies: - Environment variables from launch profile - Working directory settings - Command line arguments combination (profile + command line) - Environment variable expansion in executable paths Created new test assets: - AppForExecutableProfile: Helper app to verify environment and arguments - AppWithDetailedExecutableProfile: Test project with various executable profiles All tests verify the correct application of launch profile settings to the generated command. Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Implementation Plan for Executable Launch Profile Support
ExecutableLaunchSettingsProviderwith typed models and JSON deserializationLaunchSettingsModelwith derived models for Project and Executable profilesProfileKinddiscriminator to enable type pattern matchingLaunchSettingsManagerSupportedProfileTypesfrom LaunchSettingsManagerLaunchSettingsProfile.csto use supported types arrayChanges Made:
Architecture
LaunchSettingsModelwithProfileKinddiscriminator enumProjectLaunchSettingsModelandExecutableLaunchSettingsModelfrom baseProjectLaunchProfileJson,ExecutableLaunchProfileJson)JsonSerializer.Deserializeinstead of manual JSON traversalSupportedProfileTypesproperty fromLaunchSettingsManagerExecution Flow
RunCommand.Execute()to use pattern matching:if (launchSettings is ExecutableLaunchSettingsModel execSettings)ExecuteWithProjectProfile()andExecuteWithExecutableProfile()methodsExecuteCommand()method to reduce duplicationLaunchSettingsModeltypedotnet-watch Support
LaunchSettingsProfile.csto use array of supported types with documentation commentTesting
AppForExecutableProfile: Helper app to verify environment and argumentsAppWithDetailedExecutableProfile: Test project with various executable profilesTests:
✅ All 6 Executable profile tests pass (3 basic + 4 comprehensive)
✅ All 11 existing launch settings tests pass
✅ Manual testing confirmed functionality
<issue_title>Allow
dotnet runto use launch profiles withcommandName: Executable</issue_title>><issue_description>This is a disparity between the dotnet CLI and Visual Studio.
> I was trying to use the dotnet CLI for running a 3rd party application, specified in launchSettings.json, which my project is a plugin for, however, the "Executable" type is not supported. It does however work with Visual studio.
>
> ### Describe the solution you'd like
> Allow
dotnet runto launch the 3rd party application and possibly take control of it's life cycle in some way so that hot reload can work withdotnet watch.></issue_description>
>
><agent_instructions>Make sure to add tests for the launch profile loading, and to review my comments in the thread before implementation.</agent_instructions>
>
> ## Comments on the Issue (you are @copilot in this section)
>
>
><comment_new>@marcpopMSFT
> Triage: VS has a few different launch targets we don't support currently. There is a similar ask for container support. Moving to backlog as we collect more feedback.</comment_new>
><comment_new>@baronfel
> No workaround as of yet, but the Project System team has docs about their profiles and how they are supported here. We'd need someone to contribute the handling for the "executable" type, as we're working on other things at this time. If you wanted to do that, we'd gladly help you get a PR merged!</comment_new>
><comment_new>@baronfel
> For
executableprofile support, there are two locations that would need to be updated:>
> * the
providerthat actually applies the launch settings values to the executable to run: src/Cli/dotnet/Commands/Run/LaunchSettings/LaunchSettingsManager.cs> * the 'default profile' detection logic: src/BuiltInTools/dotnet-watch/Process/LaunchSettingsProfile.cs
> * this default-profile detection logic should probably look for the first profile that is a type supported by the LaunchSettingsManager, if we're being honest. That'd make future profiles only require changing the one location
> </comment_new>
>
>
dotnet runto use launch profiles withcommandName: Executable#49131Original prompt
dotnet runto use launch profiles withcommandName: Executable#49131💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.