Implicitly set UserSecretsId for file-based apps #50783
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements implicit UserSecretsId setting for file-based applications to support user secrets functionality. This addresses a limitation where file-based apps couldn't use user secrets without explicit configuration.
- Automatically sets UserSecretsId as a hash of the entry point file path for file-based programs
- Adds comprehensive test coverage for user secrets functionality with both explicit and implicit ID scenarios
- Updates documentation to reflect the new implicit UserSecretsId behavior
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.props | Adds MSBuild property to implicitly set UserSecretsId for file-based apps |
| test/dotnet.Tests/CommandTests/Run/RunFileTests.cs | Adds test cases for user secrets functionality with both ID argument and file-based scenarios |
| documentation/general/dotnet-run-file.md | Documents the new implicit UserSecretsId behavior for file-based apps |
|
@RikkiGibson @333fred for reviews, thanks |
| @@ -31,6 +31,17 @@ public override int Execute() | |||
| var sourceFile = SourceFile.Load(file); | |||
| var directives = VirtualProjectBuildingCommand.FindDirectives(sourceFile, reportAllErrors: !_force, DiagnosticBag.ThrowOnFirst()); | |||
|
|
|||
| // Create a project instance for evaluation. | |||
| string entryPointFileDirectory = PathUtility.EnsureTrailingSlash(Path.GetDirectoryName(file)!); | |||
There was a problem hiding this comment.
Consider making this just a parameter to FindIncludedItems, in order to make things more clear. It feels unnecessary to rely on capture, when the variable is not used in the declaring method itself.
There was a problem hiding this comment.
The most straightforward thing might be to delete the declaration here and restore the declaration in FindIncludedItems().
There was a problem hiding this comment.
Agreed with Rikki, that this is only referenced via capture is somewhat confusing. I'd personally just pass this as a parameter where needed.
| } | ||
|
|
||
| [Fact] | ||
| public void UserSecretsId_Overridden_ViaDirectoryBuildProps() |
There was a problem hiding this comment.
The following are corner cases, but it may be worthwhile to test them, just so we know the behavior.
- UserSecretsId is specified via
#:propertyand the value is exactly the same as the one we determined implicitly. - UserSecretsId is specified via
Directory.Build.propsand the value is exactly the same as the one we determined implicitly.
| var testInstance = _testAssetsManager.CreateTestDirectory(); | ||
|
|
||
| string code = $""" | ||
| #:package Microsoft.Extensions.Configuration.UserSecrets@*-* |
There was a problem hiding this comment.
I am unsure if it is best to use a wildcard version here. For example, it could be disruptive if someone were PRing into a servicing branch, and some interaction with a newer version of the package, was causing the old branch version of this test to fail.
| @@ -31,6 +31,17 @@ public override int Execute() | |||
| var sourceFile = SourceFile.Load(file); | |||
| var directives = VirtualProjectBuildingCommand.FindDirectives(sourceFile, reportAllErrors: !_force, DiagnosticBag.ThrowOnFirst()); | |||
|
|
|||
| // Create a project instance for evaluation. | |||
| string entryPointFileDirectory = PathUtility.EnsureTrailingSlash(Path.GetDirectoryName(file)!); | |||
There was a problem hiding this comment.
Agreed with Rikki, that this is only referenced via capture is somewhat confusing. I'd personally just pass this as a parameter where needed.
Part of dotnet/aspnetcore#63440.
Limited to file-based apps only to alleviate the concerns raised in #50597 (comment).