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

dotnet pack puts net5.0-android in lib/net5.0 instead of lib/net5.0-android #14042

Closed
jonathanpeppers opened this issue Oct 8, 2020 · 12 comments
Assignees
Labels
untriaged Request triage from a team member

Comments

@jonathanpeppers
Copy link
Member

I'm testing an Android library project similar to what we have setup here:

https://github.com/xamarin/net6-samples

I notice that the output of dotnet pack on a TargetFramework=net5.0-android library results in:

> 7z l .\bin\Release\UnnamedProject.1.0.0.nupkg
...
   Date      Time    Attr         Size   Compressed  Name
------------------- ----- ------------ ------------  ------------------------
2020-10-08 10:55:52 .....          508          287  _rels\.rels
2020-10-08 10:55:52 .....          456          263  UnnamedProject.nuspec
2020-10-08 15:55:50 .....         5632         2450  lib\net5.0\UnnamedProject.dll
2020-10-08 15:55:52 .....         3345         3116  lib\net5.0-android\UnnamedProject.aar

NOTE: we have an additional .aar file here that is added via <None Include="$(OutputPath)$(TargetName).aar" Pack="true" PackagePath="lib\$(TargetFramework)" />. This works fine.

The PackTask emits a warning, that is probably related:

C:\Program Files\dotnet\sdk\5.0.100-rc.2.20480.7\Sdks\NuGet.Build.Tasks.Pack\build\NuGet.Build.Tasks.Pack.targets(207,5): warning NU5128: Some target frameworks declared in the dependencies group of the nuspec and the lib/ref folder do not have exact matches in the other location. Consult the list of actions below:
- Add a dependency group for net5.0-android to the nuspec

The PackTask generates a .nuspec such as :

> cat .\obj\Release\UnnamedProject.1.0.0.nuspec
<?xml version="1.0" encoding="utf-8"?>
<package xmlns="http://schemas.microsoft.com/packaging/2012/06/nuspec.xsd">
  <metadata>
    <id>UnnamedProject</id>
    <version>1.0.0</version>
    <authors>UnnamedProject</authors>
    <requireLicenseAcceptance>false</requireLicenseAcceptance>
    <description>Package Description</description>
    <dependencies>
      <group targetFramework="net5.0" />
    </dependencies>
  </metadata>
  <files>
    <file src="C:\src\xamarin-android\bin\TestDebug\temp\DotNetPack\bin\Release\net5.0-android\UnnamedProject.dll" target="lib\net5.0\UnnamedProject.dll" />
    <file src="C:\src\xamarin-android\bin\TestDebug\temp\DotNetPack\bin\Release\net5.0-android\UnnamedProject.aar" target="lib\net5.0-android\UnnamedProject.aar" />
  </files>
</package>

This might need to be filed against the NuGet repo, not sure.

/cc @dsplaisted

@dsplaisted
Copy link
Member

What version of the .NET SDK are you using?

@nkolev92 @zkat

@jonathanpeppers
Copy link
Member Author

This was using 5.0.100-rc.2.20480.7.

It seems like something might have changed recently, I think this may have been working on RC 1.

@marcpopMSFT marcpopMSFT added the untriaged Request triage from a team member label Oct 8, 2020
@nkolev92
Copy link
Contributor

Can you please attach a binlog?

NuGet/NuGet.Client@ba87e7e was recently added to correctly handle the TargetFrameworkMoniker + TargetPlatformMoniker to tfm in the package translation.

Sounds like you need NuGet/Home#10063, same as the other issue.

Keep in mind that target platforms have versions and those have to be included in the package.
NuGet/NuGet.Client#3691 is where that gap gets closed.

@jonathanpeppers
Copy link
Member Author

@nkolev92 I retested with .NET 5.0.100-rtm.20509.5. I'm not sure how to tell which NuGet commits are in a .NET build.

Here is the .binlog, hopefully it's just these fixes haven't landed yet:

msbuild.zip

@nkolev92
Copy link
Contributor

Took a quick look.
There's no TargetPlatformMoniker set for the framework. So NuGet generates the framework name based on TargetFrameworkMoniker only, which leads to net5.0.
Here's a SS of how I observed that:

paint

@jonathanpeppers
Copy link
Member Author

For some reason this <PropertyGroup/> isn't evaluating for Microsoft.Android.Sdk as an "SDK":

<!-- Import workload targets -->
<Import Project="Microsoft.NET.Sdk.ImportWorkloads.targets" Condition="'$(MSBuildEnableWorkloadResolver)' == 'true'" />
<!-- TargetPlatformMoniker and TargetPlatformDisplayName would normally be set in Microsoft.Common.CurrentVersion.targets. However, the
TargetPlatformVersion may have been set later than that in evaluation by platform-specific targets. So fix up those properties here -->
<PropertyGroup Condition="'$(TargetPlatformMoniker)' == '' and '$(TargetPlatformIdentifier)' != '' and '$(TargetPlatformVersion)' != ''">
<TargetPlatformMoniker>$(TargetPlatformIdentifier),Version=$(TargetPlatformVersion)</TargetPlatformMoniker>
<TargetPlatformDisplayName>$([Microsoft.Build.Utilities.ToolLocationHelper]::GetPlatformSDKDisplayName($(TargetPlatformIdentifier), $(TargetPlatformVersion)))</TargetPlatformDisplayName>
</PropertyGroup>

So I tried our prototype Microsoft.Android "workload", and the problem is fixed:

image

So I think we can close this. I believe the new dotnet/sdk workload changes caused this, but everything seems to be working for an actual "workload". We will hopefully have this in place soon.

@pjcollins
Copy link
Member

pjcollins commented Oct 19, 2020

I have one question with respect to the $(TargetPlatformVersion) usage here. With our latest workload prototype and .NET 5.0.100-rtm.20509.5 the following .nuspec is generated for an Android library project as described above:

<?xml version="1.0" encoding="utf-8"?>
<package xmlns="http://schemas.microsoft.com/packaging/2012/06/nuspec.xsd">
  <metadata>
    <id>UnnamedProject</id>
    <version>1.0.0</version>
    <authors>UnnamedProject</authors>
    <requireLicenseAcceptance>false</requireLicenseAcceptance>
    <description>Package Description</description>
    <dependencies>
      <group targetFramework="net5.0-android30.0" />
    </dependencies>
  </metadata>
</package>

I'm wondering where the trailing .0 is coming from in this targetFramework attribute? Is it expected for this to be automatically included? Our SDK generates and includes the following $(TargetPlatformVersion) :

<TargetPlatformVersion Condition=" '$(TargetPlatformVersion)' == '' ">30</TargetPlatformVersion>

I think Android is unique in the fact that this will always be an integer. Should this property generation be updated to always include a trailing .0 to be more consistent with other platforms? Or should we prevent the inclusion of this extra .0 for the Android platform wherever this is being set? Some more context around this was discussed in dotnet/runtime#43531

@jonathanpeppers
Copy link
Member Author

Reopening this, as we have a related question.

We are having to do this to put our .aar file in the same directory as the .dll:

<None Include="$(_AarOutputPath)" Pack="true" PackagePath="lib\$(TargetFramework)$(TargetPlatformVersion).0" />

Looking through the .binlog, we don't see an MSBuild property that would tell us the path where NuGet is going to put the .NET assembly. We could also consider moving the .aar to a different path, let us know.

@dsplaisted
Copy link
Member

@nkolev92 @zkat Can you comment on where the .0 is getting appended to the folder name and whether that's intentional?

@nkolev92
Copy link
Contributor

Not sure at what part exactly the 0 gets appended, but it is there "as written".

NuGet itself doesn't understand the platform names and how they correlate with the numbers, so the logic kept is at the minimum a 2 part version similar to the target framework moniker part.

@dsplaisted
Copy link
Member

@jonathanpeppers Can you try adapting this solution (from here)?

<PropertyGroup>
  <TargetsForTfmSpecificContentInPackage>$(TargetsForTfmSpecificContentInPackage);IncludeReferencedProjectInPackage</TargetsForTfmSpecificContentInPackage>
</PropertyGroup>
<Target Name="IncludeReferencedProjectInPackage" Condition="'$(IncludeBuildOutput)' != 'false'">
    <GetNuGetShortFolderName
      TargetFrameworkMoniker="$(TargetFrameworkMoniker)"
      TargetPlatformMoniker="$(TargetPlatformMoniker)">
      <Output TaskParameter="NuGetShortFolderName" PropertyName="_NuGetShortFolderName" />
    </GetNuGetShortFolderName>
  <ItemGroup>
    <TfmSpecificPackageFile Include="$(OutputPath)\ReferencedLib.dll" PackagePath="lib/$(_NuGetShortFolderName)" />
  </ItemGroup>
</Target>

jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this issue Oct 27, 2020
Context: dotnet/sdk#14042 (comment)

We had been using an unfortunate hack to get the proper directory for
placing `.aar` files in NuGet packages:

    <None Include="$(_AarOutputPath)" Pack="true" PackagePath="lib\$(TargetFramework)$(TargetPlatformVersion).0" />

Not only was the `.0` weird, but it would also produce the wrong
result if the `$(TargetFramework)` was `net5.0-android30`:

    lib\net5.0-android3030.0

Using an example from the NuGet team, we can run a run a target by
setting `$(TargetsForTfmSpecificContentInPackage)`:

    <PropertyGroup>
      <TargetsForTfmSpecificContentInPackage>$(TargetsForTfmSpecificContentInPackage);_IncludeAarInNuGetPackage</TargetsForTfmSpecificContentInPackage>
    </PropertyGroup>

Then we can run the `<GetNuGetShortFolderName/>` MSBuild task to get
the NuGet folder name:

    <GetNuGetShortFolderName
        TargetFrameworkMoniker="$(TargetFrameworkMoniker)"
        TargetPlatformMoniker="$(TargetPlatformMoniker)">
      <Output TaskParameter="NuGetShortFolderName" PropertyName="_NuGetShortFolderName" />
    </GetNuGetShortFolderName>

Lastly, we add to the `@(TfmSpecificPackageFile)` item group:

    <ItemGroup>
      <TfmSpecificPackageFile Include="$(_AarOutputPath)" PackagePath="lib\$(_NuGetShortFolderName)" />
    </ItemGroup>

Using these changes we can remove the `@(None)` item we were using
originally. I also updated a test to verify that `net5.0-android30`
works.
@jonathanpeppers
Copy link
Member Author

This appears to work well: dotnet/android#5242

We can close this, thanks!

jonathanpeppers added a commit to dotnet/android that referenced this issue Oct 27, 2020
Context: dotnet/sdk#14042 (comment)

We had been using an unfortunate hack to get the proper directory for
placing `.aar` files in NuGet packages:

    <None Include="$(_AarOutputPath)" Pack="true" PackagePath="lib\$(TargetFramework)$(TargetPlatformVersion).0" />

Not only was the `.0` weird, but it would also produce the wrong
result if the `$(TargetFramework)` was `net5.0-android30`:

    lib\net5.0-android3030.0

Using an example from the NuGet team, we can run a run a target by
setting `$(TargetsForTfmSpecificContentInPackage)`:

    <PropertyGroup>
      <TargetsForTfmSpecificContentInPackage>$(TargetsForTfmSpecificContentInPackage);_IncludeAarInNuGetPackage</TargetsForTfmSpecificContentInPackage>
    </PropertyGroup>

Then we can run the `<GetNuGetShortFolderName/>` MSBuild task to get
the NuGet folder name:

    <GetNuGetShortFolderName
        TargetFrameworkMoniker="$(TargetFrameworkMoniker)"
        TargetPlatformMoniker="$(TargetPlatformMoniker)">
      <Output TaskParameter="NuGetShortFolderName" PropertyName="_NuGetShortFolderName" />
    </GetNuGetShortFolderName>

Lastly, we add to the `@(TfmSpecificPackageFile)` item group:

    <ItemGroup>
      <TfmSpecificPackageFile Include="$(_AarOutputPath)" PackagePath="lib\$(_NuGetShortFolderName)" />
    </ItemGroup>

Using these changes we can remove the `@(None)` item we were using
originally. I also updated a test to verify that `net5.0-android30`
works.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
untriaged Request triage from a team member
Projects
None yet
Development

No branches or pull requests

5 participants