Implicitly set UserSecretsId #50597
Implicitly set UserSecretsId #50597
Conversation
|
This PR is targeting |
| @@ -133,6 +133,9 @@ Copyright (c) .NET Foundation. All rights reserved. | |||
|
|
|||
| <!-- Uncomment this once https://github.com/Microsoft/visualfsharp/issues/3207 gets fixed --> | |||
| <!-- <WarningsAsErrors>$(WarningsAsErrors);NU1605</WarningsAsErrors> --> | |||
|
|
|||
| <!-- Implicitly set UserSecretsId to hash of project file path. --> | |||
| <UserSecretsId Condition="'$(UserSecretsId)'==''">$([MSBuild]::StableStringHash($(MSBuildProjectFullPath), 'Sha256'))</UserSecretsId> | |||
There was a problem hiding this comment.
Is this going to make reproducible builds more difficult? I mean, if the project does not explicitly set UserSecretsId, and it ends up referencing Microsoft.Extensions.Configuration.UserSecrets so that an UserSecretsIdAttribute is generated, then the attribute will now depend on the full path of the project, so you'll need to place the project at the same path if you ever want to reproduce the build. The effects of the full path on debug information can be avoided by PathMap but that won't affect MSBuildProjectFullPath.
There was a problem hiding this comment.
Hm, interesting. I'm not sure what to do differently here though. You can always opt out from the attribute via GenerateUserSecretsAttribute=false.
I guess we could try using $(PathMap) here as well, but that seems like an overkill given you need to be referencing UserSecrets assembly and then there are opt outs.
There was a problem hiding this comment.
Eww yeah I strongly agree with @KalleOlaviNiemitalo here - we can't set this during eval time like this.
If we had a concept of the repo/workspace root then we could use the part of the project full path from that root safely - but we only know that data once the source link targets have run..
There was a problem hiding this comment.
Microsoft.Extensions.Configuration.UserSecrets is referenced by Microsoft.Extensions.Hosting, so I think this PR will cause UserSecretsIdAttribute to be generated in many services whose developers were not intending to use user secrets.
There was a problem hiding this comment.
cc @DamianEdwards - looks like setting UserSecretsId implicitly in SDK targets might be problematic.
I can instead implement this implicit UserSecretsId computation in the user-secrets tool, but that means normal builds won't see it (e.g., custom build targets that want to read UserSecretsId for some reason).
There was a problem hiding this comment.
@jjonescz yep fair enough. Side effects are impactful here so seems like the right place to do it.
There was a problem hiding this comment.
Realized that wouldn't work - the runtime side of user-secrets config wouldn't see the ID. What about limiting this to file-based apps only? #50783
| <!-- Implicitly set UserSecretsId to hash of project file path. --> | ||
| <UserSecretsId Condition="'$(UserSecretsId)'==''">$([MSBuild]::StableStringHash($(MSBuildProjectFullPath), 'Sha256'))</UserSecretsId> |
There was a problem hiding this comment.
Another effect of this change is that, if I find a user secrets file in my user profile and want to know which project is using it and whether the file would be safe to delete, I won't be able to identify the project by grepping for the user secrets ID in source trees.
However, the format of the user secrets ID will hint that it was generated by [MSBuild]::StableStringHash rather than dotnet user-secrets init, which generates a GUID.
There was a problem hiding this comment.
We could append the project name to the ID to help with this.
Part of dotnet/aspnetcore#63440.