Fix sourcelink repository url

[michellemcdaniel]

When we updated the ScmRepositoryUrl to handle devdiv links, we never updated the version used in SourceRoot, which is used by sourcelink, and also needed to be updated to remove -trusted from the link. By not removing -trusted, the translation pattern can't identify the url as something that needs to be translated, and so just uses the azdo url. This change adds the code to remove -trusted to fix the issue.

To double check:

• The right tests are in and and the right validation has happened. Guidance: https://github.com/dotnet/core-eng/tree/master/Documentation/Validation

I tested this locally with the following MsBuild target:

<ItemGroup>
  <SourceRoot>
    <ScmRepositoryUrl>https://devdiv.visualstudio.com/DevDiv/_git/DotNet-msbuild-Trusted</ScmRepositoryUrl>
  </SourceRoot>
</ItemGroup>

<Message Importance="high" Text="%(SourceRoot.ScmRepositoryUrl)" /> // prints https://devdiv.visualstudio.com/DevDiv/_git/DotNet-msbuild-Trusted

<ItemGroup>
  <SourceRoot Update="@(SourceRoot)">
    <ScmRepositoryUrl Condition="$([System.String]::Copy(%(SourceRoot.ScmRepositoryUrl)).Contains(`devdiv.visualstudio`))">$([System.String]::Copy(%(SourceRoot.ScmRepositoryUrl)).ToLower().Replace(`-trusted`,``))</ScmRepositoryUrl>
  </SourceRoot>
</ItemGroup>

<Message Importance="high" Text="%(SourceRoot.ScmRepositoryUrl)" /> // prints https://devdiv.visualstudio.com/devdiv/_git/dotnet-msbuild

<ItemGroup>
  <SourceRoot Update="@(SourceRoot)">
    <ScmRepositoryUrl>$([System.Text.RegularExpressions.Regex]::Replace(%(SourceRoot.ScmRepositoryUrl), $(_TranslateUrlPattern), $(_TranslateUrlReplacement)))</ScmRepositoryUrl>
  </SourceRoot>
</ItemGroup>

<Message Importance="high" Text="%(SourceRoot.ScmRepositoryUrl)" /> // prints https://github.com/dotnet/msbuild

MsBuild quirks, as far as I can tell, require the two separate Updates.

[mmitche]

I think this will also need to go into the release/6.0 branch.

[ericstj]

This seems to have caused a regression https://dev.azure.com/dnceng/public/_build/results?buildId=1347728&view=logs&j=a83666f7-a1cf-595f-f6cb-b187860111f1&t=56bb721b-9eb1-5ff5-18e2-b390ddb4ced4

[ericstj]

Looks like it's due to one SourceRoot that is included by generated NuGet props that has no metadata:

<?xml version="1.0" encoding="utf-8" standalone="no"?>
<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <PropertyGroup Condition=" '$(ExcludeRestorePackageImports)' != 'true' ">
    <RestoreSuccess Condition=" '$(RestoreSuccess)' == '' ">True</RestoreSuccess>
    <RestoreTool Condition=" '$(RestoreTool)' == '' ">NuGet</RestoreTool>
    <ProjectAssetsFile Condition=" '$(ProjectAssetsFile)' == '' ">$(MSBuildThisFileDirectory)project.assets.json</ProjectAssetsFile>
    <NuGetPackageRoot Condition=" '$(NuGetPackageRoot)' == '' ">/__w/1/s/.packages</NuGetPackageRoot>
    <NuGetPackageFolders Condition=" '$(NuGetPackageFolders)' == '' ">/__w/1/s/.packages</NuGetPackageFolders>
    <NuGetProjectStyle Condition=" '$(NuGetProjectStyle)' == '' ">PackageReference</NuGetProjectStyle>
    <NuGetToolVersion Condition=" '$(NuGetToolVersion)' == '' ">6.0.0</NuGetToolVersion>
  </PropertyGroup>
  <ItemGroup Condition=" '$(ExcludeRestorePackageImports)' != 'true' ">
    <SourceRoot Include="/__w/1/s/.packages/" />
  </ItemGroup>
  <ImportGroup Condition=" '$(ExcludeRestorePackageImports)' != 'true' ">
    <Import Project="$(NuGetPackageRoot)/microsoft.sourcelink.common/1.1.0-beta-20206-02/build/Microsoft.SourceLink.Common.props" Condition="Exists('$(NuGetPackageRoot)/microsoft.sourcelink.common/1.1.0-beta-20206-02/build/Microsoft.SourceLink.Common.props')" />
    <Import Project="$(NuGetPackageRoot)/microsoft.build.tasks.git/1.1.0-beta-20206-02/build/Microsoft.Build.Tasks.Git.props" Condition="Exists('$(NuGetPackageRoot)/microsoft.build.tasks.git/1.1.0-beta-20206-02/build/Microsoft.Build.Tasks.Git.props')" />
    <Import Project="$(NuGetPackageRoot)/microsoft.sourcelink.github/1.1.0-beta-20206-02/build/Microsoft.SourceLink.GitHub.props" Condition="Exists('$(NuGetPackageRoot)/microsoft.sourcelink.github/1.1.0-beta-20206-02/build/Microsoft.SourceLink.GitHub.props')" />
    <Import Project="$(NuGetPackageRoot)/microsoft.sourcelink.azurerepos.git/1.1.0-beta-20206-02/build/Microsoft.SourceLink.AzureRepos.Git.props" Condition="Exists('$(NuGetPackageRoot)/microsoft.sourcelink.azurerepos.git/1.1.0-beta-20206-02/build/Microsoft.SourceLink.AzureRepos.Git.props')" />
    <Import Project="$(NuGetPackageRoot)/microsoft.net.illink.tasks/7.0.100-alpha.1.21426.2/build/Microsoft.NET.ILLink.Tasks.props" Condition="Exists('$(NuGetPackageRoot)/microsoft.net.illink.tasks/7.0.100-alpha.1.21426.2/build/Microsoft.NET.ILLink.Tasks.props')" />
    <Import Project="$(NuGetPackageRoot)/microsoft.net.illink.analyzers/7.0.100-alpha.1.21426.2/build/Microsoft.NET.ILLink.Analyzers.props" Condition="Exists('$(NuGetPackageRoot)/microsoft.net.illink.analyzers/7.0.100-alpha.1.21426.2/build/Microsoft.NET.ILLink.Analyzers.props')" />
    <Import Project="$(NuGetPackageRoot)/microsoft.net.compilers.toolset/4.0.0-4.21427.11/build/Microsoft.Net.Compilers.Toolset.props" Condition="Exists('$(NuGetPackageRoot)/microsoft.net.compilers.toolset/4.0.0-4.21427.11/build/Microsoft.Net.Compilers.Toolset.props')" />
    <Import Project="$(NuGetPackageRoot)/microsoft.codeanalysis.netanalyzers/7.0.0-preview1.21452.7/build/Microsoft.CodeAnalysis.NetAnalyzers.props" Condition="Exists('$(NuGetPackageRoot)/microsoft.codeanalysis.netanalyzers/7.0.0-preview1.21452.7/build/Microsoft.CodeAnalysis.NetAnalyzers.props')" />
    <Import Project="$(NuGetPackageRoot)/microsoft.codeanalysis.csharp.codestyle/3.10.0/build/Microsoft.CodeAnalysis.CSharp.CodeStyle.props" Condition="Exists('$(NuGetPackageRoot)/microsoft.codeanalysis.csharp.codestyle/3.10.0/build/Microsoft.CodeAnalysis.CSharp.CodeStyle.props')" />
  </ImportGroup>
  <PropertyGroup Condition=" '$(ExcludeRestorePackageImports)' != 'true' ">
    <PkgStyleCop_Analyzers_Unstable Condition=" '$(PkgStyleCop_Analyzers_Unstable)' == '' ">/__w/1/s/.packages/stylecop.analyzers.unstable/1.2.0.304</PkgStyleCop_Analyzers_Unstable>
    <PkgMicrosoft_SourceLink_Common Condition=" '$(PkgMicrosoft_SourceLink_Common)' == '' ">/__w/1/s/.packages/microsoft.sourcelink.common/1.1.0-beta-20206-02</PkgMicrosoft_SourceLink_Common>
    <PkgMicrosoft_Build_Tasks_Git Condition=" '$(PkgMicrosoft_Build_Tasks_Git)' == '' ">/__w/1/s/.packages/microsoft.build.tasks.git/1.1.0-beta-20206-02</PkgMicrosoft_Build_Tasks_Git>
    <PkgMicrosoft_SourceLink_GitHub Condition=" '$(PkgMicrosoft_SourceLink_GitHub)' == '' ">/__w/1/s/.packages/microsoft.sourcelink.github/1.1.0-beta-20206-02</PkgMicrosoft_SourceLink_GitHub>
    <PkgMicrosoft_SourceLink_AzureRepos_Git Condition=" '$(PkgMicrosoft_SourceLink_AzureRepos_Git)' == '' ">/__w/1/s/.packages/microsoft.sourcelink.azurerepos.git/1.1.0-beta-20206-02</PkgMicrosoft_SourceLink_AzureRepos_Git>
    <PkgMicrosoft_NET_ILLink_Tasks Condition=" '$(PkgMicrosoft_NET_ILLink_Tasks)' == '' ">/__w/1/s/.packages/microsoft.net.illink.tasks/7.0.100-alpha.1.21426.2</PkgMicrosoft_NET_ILLink_Tasks>
    <PkgMicrosoft_CodeAnalysis_NetAnalyzers Condition=" '$(PkgMicrosoft_CodeAnalysis_NetAnalyzers)' == '' ">/__w/1/s/.packages/microsoft.codeanalysis.netanalyzers/7.0.0-preview1.21452.7</PkgMicrosoft_CodeAnalysis_NetAnalyzers>
    <PkgMicrosoft_CodeAnalysis_CSharp_CodeStyle Condition=" '$(PkgMicrosoft_CodeAnalysis_CSharp_CodeStyle)' == '' ">/__w/1/s/.packages/microsoft.codeanalysis.csharp.codestyle/3.10.0</PkgMicrosoft_CodeAnalysis_CSharp_CodeStyle>
  </PropertyGroup>
</Project>

Probably the simple fix here is to just add single quotes around %(SourceRoot.ScmRepositoryUrl)

[MattGal]

Probably the simple fix here is to just add single quotes around %(SourceRoot.ScmRepositoryUrl)

I'll get a fresh FR issue up to do this... it's slightly tricky to do since the targets file comes from within a nupkg so can't just fix up this PR to test that theory.

[ericstj]

One trick I use to test this sort of thing is to build a private package, put that on myget, and create a draft PR in runtime that consumes it. https://github.com/dotnet/runtime/pull/58059/files Not the cleanest but it can help when you want that sort of coverage.

[ericstj]

That said in this case I think it should be easy to repro on a local build of runtime that updates the targets (even manually) the same flags as PR validation.

[MattGal]

That said in this case I think it should be easy to repro on a local build of runtime that updates the targets (even manually) the same flags as PR validation.

Agreed, though I think in the case of a simple bug with items and batching a boiled-down repro is frequently enough as long as it gets the identical errors.