Skip to content
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

Convert AspNetCore transport pkgproj to Pack task #56674

Merged
merged 2 commits into from
Aug 2, 2021

Conversation

ViktorHofer
Copy link
Member

@ViktorHofer ViktorHofer commented Jul 31, 2021

Converting the Microsoft.AspNetCore.Internal.Transport package to a proj
file which uses the NuGet Pack task.

Also moving some more packaging related targets into packaging.targets.

Diff: https://www.diffchecker.com/3qVruSQz

@ghost
Copy link

ghost commented Jul 31, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

Converting the Microsoft.AspNetCore.Internal.Transport package to a proj
file which uses the NuGet Pack task.

Also moving some more packaging related targets into packaging.targets.

Author: ViktorHofer
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

Converting the Microsoft.AspNetCore.Internal.Transport package to a proj
file which uses the NuGet Pack task.

Also moving some more packaging related targets into packaging.targets.
@ViktorHofer ViktorHofer force-pushed the AspNetCoreInternalPackage branch from a9d23fd to 156e0e1 Compare July 31, 2021 21:35
Copy link
Contributor

@Anipik Anipik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the nuspec diff here as well ?

@ViktorHofer
Copy link
Member Author

Just added it to the top post. As the package doesn't have any dependencies it doesn't really tell much though.

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Aug 2, 2021

Many unrelated failures because of flakiness in the test infrastructure. Merging.

@ViktorHofer ViktorHofer merged commit f440371 into dotnet:main Aug 2, 2021
@ViktorHofer ViktorHofer deleted the AspNetCoreInternalPackage branch August 2, 2021 18:53
Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Anipik
Copy link
Contributor

Anipik commented Aug 2, 2021

Diff: https://www.diffchecker.com/3qVruSQz

the diff here shows no files in the package.

@ViktorHofer
Copy link
Member Author

A nuspec generated by the NuGet Pack task doesn't list any files by design. I compared the files locally and those match.

@Anipik
Copy link
Contributor

Anipik commented Aug 2, 2021

A nuspec generated by the NuGet Pack task doesn't list any files by design. I compared the files locally and those match.

the extensions package use the nuget pack but still has the files

<?xml version="1.0" encoding="utf-8"?>
<package xmlns="http://schemas.microsoft.com/packaging/2012/06/nuspec.xsd">
  <metadata>
    <id>Microsoft.Extensions.DependencyInjection.Abstractions</id>
    <version>6.0.0-dev</version>
    <authors>Microsoft</authors>
    <license type="expression">MIT</license>
    <licenseUrl>https://licenses.nuget.org/MIT</licenseUrl>
    <icon>Icon.png</icon>
    <projectUrl>https://dot.net/</projectUrl>
    <description>Abstractions for dependency injection.

Commonly Used Types:
Microsoft.Extensions.DependencyInjection.IServiceCollection</description>
    <releaseNotes>https://go.microsoft.com/fwlink/?LinkID=799421</releaseNotes>
    <copyright>© Microsoft Corporation. All rights reserved.</copyright>
    <repository type="git" url="https://github.com/dotnet/runtime" commit="0000000000000000000000000000000000000000" />
    <dependencies>
      <group targetFramework=".NETFramework4.6.1">
        <dependency id="Microsoft.Bcl.AsyncInterfaces" version="6.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Threading.Tasks.Extensions" version="4.5.4" exclude="Build,Analyzers" />
      </group>
      <group targetFramework=".NETStandard2.0">
        <dependency id="Microsoft.Bcl.AsyncInterfaces" version="6.0.0-dev" exclude="Build,Analyzers" />
        <dependency id="System.Threading.Tasks.Extensions" version="4.5.4" exclude="Build,Analyzers" />
      </group>
      <group targetFramework=".NETStandard2.1" />
    </dependencies>
  </metadata>
  <files>
    <file src="C:\git\runtime\artifacts\bin\Microsoft.Extensions.DependencyInjection.Abstractions\net461-Debug\Microsoft.Extensions.DependencyInjection.Abstractions.dll" target="lib\net461\Microsoft.Ext
    <file src="C:\git\runtime\artifacts\bin\Microsoft.Extensions.DependencyInjection.Abstractions\netstandard2.0-Debug\Microsoft.Extensions.DependencyInjection.Abstractions.dll" target="lib\netstandard2
    <file src="C:\git\runtime\artifacts\bin\Microsoft.Extensions.DependencyInjection.Abstractions\netstandard2.1-Debug\Microsoft.Extensions.DependencyInjection.Abstractions.dll" target="lib\netstandard2
    <file src="C:\Users\anagniho\.nuget\packages\microsoft.private.intellisense\5.0.0-preview-20201009.2\IntellisenseFiles\net\1033\Microsoft.Extensions.DependencyInjection.Abstractions.xml" target="lib
    <file src="C:\Users\anagniho\.nuget\packages\microsoft.private.intellisense\5.0.0-preview-20201009.2\IntellisenseFiles\net\1033\Microsoft.Extensions.DependencyInjection.Abstractions.xml" target="lib
    <file src="C:\Users\anagniho\.nuget\packages\microsoft.private.intellisense\5.0.0-preview-20201009.2\IntellisenseFiles\net\1033\Microsoft.Extensions.DependencyInjection.Abstractions.xml" target="lib
    <file src="C:\git\runtime\eng\useSharedDesignerContext.txt" target="useSharedDesignerContext.txt" />
    <file src="C:\Users\anagniho\.nuget\packages\microsoft.dotnet.arcade.sdk\6.0.0-beta.21370.12\tools\Assets\DotNetPackageIcon.png" target="Icon.png" />
    <file src="C:\git\runtime\LICENSE.TXT" target="LICENSE.TXT" />
    <file src="C:\git\runtime\THIRD-PARTY-NOTICES.TXT" target="THIRD-PARTY-NOTICES.TXT" />
  </files>
</package>

@ViktorHofer
Copy link
Member Author

I don't know which NuGet.exe or dotnet sdk you are using but neither the tools on my machine nor the official builds produce nuspecs with files in it:

Package: https://dnceng.visualstudio.com/public/_packaging?_a=package&feed=dotnet6&package=Microsoft.Extensions.DependencyInjection.Abstractions&protocolType=NuGet&version=6.0.0-rc.1.21402.3

<?xml version="1.0" encoding="utf-8"?>
<package xmlns="http://schemas.microsoft.com/packaging/2012/06/nuspec.xsd">
  <metadata>
    <id>Microsoft.Extensions.DependencyInjection.Abstractions</id>
    <version>6.0.0-rc.1.21402.3</version>
    <authors>Microsoft</authors>
    <owners></owners>
    <requireLicenseAcceptance>false</requireLicenseAcceptance>
    <license type="expression">MIT</license>
    <icon>Icon.png</icon>
    <projectUrl>https://dot.net/</projectUrl>
    <description>Abstractions for dependency injection.

Commonly Used Types:
Microsoft.Extensions.DependencyInjection.IServiceCollection</description>
    <releaseNotes>https://go.microsoft.com/fwlink/?LinkID=799421</releaseNotes>
    <copyright>© Microsoft Corporation. All rights reserved.</copyright>
    <serviceable>true</serviceable>
    <repository type="git" url="https://github.com/dotnet/runtime" commit="aa4290f3f0aadf86882a439d124db3215851760b" />
    <dependencies>
      <group targetFramework=".NETFramework4.6.1">
        <dependency id="Microsoft.Bcl.AsyncInterfaces" version="6.0.0-rc.1.21402.3" exclude="Build,Analyzers" />
        <dependency id="System.Threading.Tasks.Extensions" version="4.5.4" exclude="Build,Analyzers" />
      </group>
      <group targetFramework="net6.0" />
      <group targetFramework=".NETStandard2.0">
        <dependency id="Microsoft.Bcl.AsyncInterfaces" version="6.0.0-rc.1.21402.3" exclude="Build,Analyzers" />
        <dependency id="System.Threading.Tasks.Extensions" version="4.5.4" exclude="Build,Analyzers" />
      </group>
      <group targetFramework=".NETStandard2.1" />
    </dependencies>
  </metadata>
</package>

@safern
Copy link
Member

safern commented Aug 2, 2021

I believe @Anipik might be looking at the nuspec generated by the task in order to generate the nupkg and the one that doesn't contain the files is the one inside the final package?

@Anipik
Copy link
Contributor

Anipik commented Aug 2, 2021

I believe @Anipik might be looking at the nuspec generated by the task in order to generate the nupkg and the one that doesn't contain the files is the one inside the final package

correct

@ViktorHofer
Copy link
Member Author

I believe @Anipik might be looking at the nuspec generated by the task in order to generate the nupkg and the one that doesn't contain the files is the one inside the final package?

Thanks for clarifying, that makes sense. I don't think diffing that intermediate nuspec is useful as it doesn't represent the final nuspec which is part of the package. Also the intermediate one if machine specific (ie absolute paths) and hence quite hard to diff.

@safern
Copy link
Member

safern commented Aug 3, 2021

This change introduced a build error when building some of the extensions package. i.e:

C:\repos\runtime\src\libraries\System.Text.Json\gen\System.Text.Json.SourceGeneration.csproj : error MSB4057: The target "GetTargetPath" does not exist in the project.
C:\repos\runtime\src\libraries\System.Text.Json\gen\System.Text.Json.SourceGeneration.csproj : error MSB4057: The target "GetTargetPath" does not exist in the project.

Should repro with:

 dotnet pack .\src\libraries\Microsoft.Extensions.DependencyModel\src\

The interesting thing is that it doesn't fail the build, that's why the build was green.

image

https://dev.azure.com/dnceng/public/_build/results?buildId=1272285&view=logs&j=81a0f8c2-d4db-561f-9b8e-3f045280d7fe&t=2a104984-c43b-5b93-1361-6bd8a1e96b04&l=2141

@safern
Copy link
Member

safern commented Aug 3, 2021

Seems to be related to ordering, haven't figured out why, but packaging.targets is imported afterwards. If I move what was moved from Directory.Build.targets down, back to its original location, everything works fine.

@ViktorHofer
Copy link
Member Author

Thanks for reporting. I will try to fix this immediately. Interestingly this is also showing in my other PR (windows compat pack).

@safern
Copy link
Member

safern commented Aug 3, 2021

I see. The problem is that these properties:

<BeforePack>IncludeAnalyzersInPackage;$(BeforePack)</BeforePack>
<BuildAnalyzerReferences>$(BuildProjectReferences)</BuildAnalyzerReferences>
<BuildAnalyzerReferences Condition="'$(BuildingInsideVisualStudio)' == 'true'">false</BuildAnalyzerReferences>

Where set unconditionally and now are set conditionally only when '$(IsPackable)' == 'true'. However not all project have moved to use the pack task, and in this case System.Text.Json which references an analyzer project, doesn't set IsPackable == true, so BuildAnalyzerReferences is not set, hence the ProjectReference we add for the AnalyzerReference is built as that property is empty by default. So when ProjectRefernces are resolved GetTargetPath is invoked on it.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants