Condition source-link arguments #11153
Conversation
| <InnerBuildArgs>$(InnerBuildArgs) /p:EnableSourceLink=false</InnerBuildArgs> | ||
| <InnerBuildArgs>$(InnerBuildArgs) /p:DeterministicSourcePaths=false</InnerBuildArgs> | ||
| <InnerBuildArgs Condition="'$(DisableSourceLink)' == 'true'">$(InnerBuildArgs) /p:EnableSourceControlManagerQueries=false</InnerBuildArgs> | ||
| <InnerBuildArgs Condition="'$(DisableSourceLink)' == 'true'">$(InnerBuildArgs) /p:EnableSourceLink=false</InnerBuildArgs> |
There was a problem hiding this comment.
Why both and EnableSourceLink and DisableSourceLink? This is confusing IMO. Would it make sense to toggle EnableSourceControlManagerQueries and DeterministicSourcePaths based on EnableSourceLink and just use EnableSourceLink to control the behavior?
There was a problem hiding this comment.
I guess this DisableSourceLink is something that already exists. 😞
There was a problem hiding this comment.
In light of this change: https://github.com/dotnet/runtime/pull/76685/files - would it actually make sense to completely remove these 3 lines instead?
DisableSourceLink seems to be a higher-level property, controlling the values of these 3 source-link related properties, like in here:
arcade/src/Microsoft.DotNet.Arcade.Sdk/tools/RepositoryInfo.targets
Lines 13 to 17 in 48e76fb
Similarly, in installer repo:
https://github.com/dotnet/installer/blob/e9549ff1d3412a94f101be476cec553f0bf858e8/Directory.Build.props#L35-L37
As a result, we would also remove the same 3 lines from: https://github.com/dotnet/installer/blob/e9549ff1d3412a94f101be476cec553f0bf858e8/src/SourceBuild/tarball/content/ArcadeOverrides/SourceBuildArcadeBuild.targets#L89-L91
There was a problem hiding this comment.
Yes, removing these would be good. They appear completely redundant now.
|
Should we port this to main? |
* Condition source-link arguments (#11153) * Remove redundant arguments - these properties are set by repos based on value of DisableSourceLink property
Fixes: dotnet/source-build#2883
This change allows source-build to disable sourcelink, but it will not do it by default anymore.