search

NuGet / Home

Repo for NuGet Client issues
Other
1.48k stars 249 forks source link

NuGet PackTask Does Not Include Xaml Theme Artifacts #5755

Open commonsensesoftware opened 6 years ago

commonsensesoftware commented 6 years ago

Background

Applications that target the Windows Runtime (WinRT) including Windows Store, Windows Phone (WP), and the Universal Windows Platform (UWP) have a number of post-build artifacts that must included in published NuGet packages. At present, the MSBuild NuGet PackTask only supports including Package Resource Index (*.pri) files.

Overview

WinRT applications that include custom controls often also include default styles and themes in the Generic.xaml file. The compiler also generates a few additional, related files that must be included in the NuGet package, including:

These files are required to have a very specific folder structure in the package:

lib
└─ [tfm]
   ├─ [package id]
   │  ├─ Themes
   │  │  ├─ Generic.xaml
   │  │  └─ Generic.xbf
   │  └─ [package id].xr.xml
   ├─ [package id].dll
   └─ [package id].pri

If these files are not included in this exact structure, then the default theme and other style information will fail to be picked up by package consumers.

Problem

Despite numerous attempts to specify the correct MSBuild items and metadata, I was unable to make the PackTask include these additional files using the required package paths. Upon closer examination of the MSBuildProjectFactory, it seems that only *.pri files are considered and all other uncognized files are ignored. The NuGet.Build.Tasks.Pack.targets also include a special build target for the _AddPriFileToPackBuildOutput.

I tried using the same approach for the other WinRT files, but I was unable to get the files to pack into the correct subfolders. It is possible to create the desired *.nuspec manually, but all of the great benefits from using MSBuild, in particular package dependencies, are lost and must also be managed manually.

Previous discussions on this topic include #3225 and #3293, which were solved by manually crafting the *.nuspec file.

Proposed Fix

The PackTask should automatically detect these files and correctly include them in the generated NuSpec metadata.

It should also be noted that this behavior does not apply to Windows Presentation Foundation (WPF), even though it too has Generic.xaml files. The compiler compiles WPF Generic.xaml files to a *.baml file, which is included in the output *.dll as an embedded resource.

Recommendations

While default behaviors and conventions should be supported by the PackTask, the task should also support explicitly specifying additional content to pack. Since such items are explicit, they should be included without filtering. This behavior allows packing build output that may not follow the expected conventions understood by the PackTask. Warnings may optionally be emmitted by package validation when unconventional content is added.

For example:

<ItemGroup>
 <!-- the "Pack" metadata item shouldn't be necessary since the mere
      existence of these items means that we want them packed -->
 <CustomPackageFile Include="bin\*.*">
  <PackagePath>bin\</PackagePath>
 </CustomPackageFile>
</ItemGroup>

As a practical example, you might have a build-only package that has varying *.targets by platform, but a single build task assembly. This assembly can be placed in an unconventional folder, which negates the need to include the same assembly or other content multiple times. In some cases, this can significantly reduce the final package size.

For example:

bin
├─ Tasks.dll
└─ BuildTool.exe

build
├─ net45
│  └─ MyPackage.targets
├─ netstandard1.0
│  └─ MyPackage.targets
└─ uap10.0
   └─ MyPackage.targets

Arguably, you could probably use the tools folder instead of the custom bin folder. As it stands, there is no way to pack custom folders using the MSBuild PackTask today (that I found find).

Workaround

I was able to come up with my own workaround, but it was tedious and time consuming. I'm sharing here in the hopes that it might help others until this issue is formally addressed and resolved.

Step 1

Make sure that your Generic.xaml items are correctly defined. To simplify things, you'll want to define the item twice, but as different build artifacts. This will ensure that things are both compiled (e.g. *.xbf) and included in the build output.

This is the normal build output, but it will not be in the format that you want. In order to make that work, you can use the Link item metadata to control where the output will go:

<Page Include="Themes\Generic.xaml">
 <Generator>MSBuild:Compile</Generator>
 <SubType>Designer</SubType>
 <!-- the compiler generates Generic.xbf -->
 <Link>$(PackageId)\Themes\Generic.xaml</Link>
</Page>

<Content Include="Themes\Generic.xaml">
 <Link>$(PackageId)\Themes\Generic.xaml</Link>
 <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
 <!-- this prevents the PackTask from putting it in the
      "content" package folder, which we don't want -->
 <Pack>false</Pack>
</Content>

Step 2

Now that the content is in the build output in the structure we want, you need to include it in the NuSpec metadata. There are several ways this can be done, but I chose to use a use a C# script (*.csx).

#r "System.Xml.Linq.dll"

using System;
using System.Linq;
using System.Xml.Linq;

var nuspec = Args[0];
var src = Args[1];
var target = Args[2];
var doc = XDocument.Load( nuspec );
var xmlns = (XNamespace) "http://schemas.microsoft.com/packaging/2013/01/nuspec.xsd";
var files = doc.Root.Element( xmlns + "files" );

if ( files == null )
{
    doc.Root.Add( files = new XElement( xmlns + "files" ) );
}

files.Add(
    new XElement(
        xmlns + "file",
        new XAttribute( "src", src ),
        new XAttribute( "target", target ) ) );

doc.Save( nuspec );

Step 3

With build output and script in hand, we need to hook the two together using a custom build target (based on VS2017):

<?xml version="1.0" encoding="utf-8"?>
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

 <PropertyGroup>
  <SolutionDir Condition=" '$(SolutionDir)' == '' ">$([System.IO.Path]::GetFullPath("$(MSBuildThisFileDirectory).."))\</SolutionDir>
  <CsiExe>$(MSBuildToolsPath)\Roslyn\csi.exe</CsiExe>
  <CsiLibPath>$(MSBuildToolsPath)</CsiLibPath>
 </PropertyGroup>

 <Target Name="AddXamlThemeFilesToNuspec">

  <PropertyGroup>
   <CsiScript>$(SolutionDir)tools\add-nuspec-xaml-content.csx</CsiScript>
  </PropertyGroup>

  <ItemGroup>
   <XamlThemeFile Include="$(BinPath)$(TargetFramework)\**\*.xaml;$(BinPath)$(TargetFramework)\**\*.xbf">
    <PackagePath>lib\$(TargetFramework)\$(PackageId)\Themes\</PackagePath>
   </XamlThemeFile>
   <XamlThemeFile Include="$(BinPath)$(TargetFramework)\**\*.xr.xml">
    <PackagePath>lib\$(TargetFramework)\$(PackageId)\</PackagePath>
   </XamlThemeFile>
  </ItemGroup>

  <Exec Command="&quot;$(CsiExe)&quot; /lib:&quot;$(CsiLibPath)&quot; &quot;$(CsiScript)&quot; &quot;$(NuspecFile)&quot; &quot;%(XamlThemeFile.Identity)&quot; &quot;@(XamlThemeFile->'%(PackagePath)%(Filename)%(Extension)')&quot;"
        Condition=" '@(XamlThemeFile)' != '' " />

 </Target>

 <Target Name="ForEachTargetFramework"
         AfterTargets="Build"
         DependsOnTargets="GenerateNuspec"
         Condition=" '$(GeneratePackageOnBuild)' == 'true' AND '$(IsInnerBuild)' != 'true' ">

  <!-- re-initial required properties that are otherwise no longer available -->
  <PropertyGroup>
   <ProjectDir Condition=" '$(ProjectDir)' == '' ">$([System.IO.Path]::GetDirectoryName('$(PackProjectInputFile)'))\</ProjectDir>
   <NuspecFile>$(ProjectDir)$(BaseIntermediateOutputPath)$(PackageId).$(PackageVersion).nuspec</NuspecFile>
   <AddXamlThemeFilesToNuspecProperties>
    PackageId=$(PackageId);
    NuspecFile=$(NuspecFile);
    SolutionDir=$(SolutionDir);
    CsiExe=$(CsiExe);
    CsiLibPath=$(CsiLibPath);
    BuildProjectReferences=false;
    BinPath=$(ProjectDir)$(OutputPath)
   </AddXamlThemeFilesToNuspecProperties>
  </PropertyGroup>

  <!-- run the *.csx script for each target framework with additional xaml artifacts -->
  <MSBuild Projects="$(MSBuildProjectFullPath)"
           Targets="AddXamlThemeFilesToNuspec"
           Properties="TargetFramework=%(_TargetFrameworks.Identity);$(AddXamlThemeFilesToNuspecProperties)" />

  <!-- HACK: now that the *.nuspec contains all of the required files,
       explicitly invoke the pack task using the finalized *.nuspec file -->
  <PackTask PackItem="$(PackProjectInputFile)"
            NuspecFile="$(NuspecFile)"
            PackageOutputPath="$(PackageOutputPath)"
            NuspecOutputPath="$(BaseIntermediateOutputPath)" />

 </Target>

</Project>

The name and location of your *.csx script may be different; update the paths accordingly. Save the *.targets file with whatever name you like. I used the name xaml.targets.

Putting It All Together

Once all of the preceeding steps have been completed, you just need to import the *.targets file into your project.

For example:

<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), my.sln))\build\xaml.targets" Label="Xaml" />

Whenever you build your project, the Generic.xaml, Generic.xbf, and [package id].xr.xml files will now be included.

mishra14 commented 6 years ago

tagging @rrelyea and @rohit21agrawal for visibility.

rohit21agrawal commented 6 years ago

@commonsensesoftware all the extensibility you want from pack is already supported. see https://docs.microsoft.com/en-us/nuget/schema/msbuild-targets#pack-scenarios under "Including Content in Package" to see how you can add any file to any desired output location in package.

If you have any questions, let me know.

As for the bigger problem of doing this by default, Pack target cannot support each individual project types that exist - it's designed to work with certain msbuild output groups, and as long as the project systems add their files/items to the correct msbuild output groups and set the right metadata on them, the pack task will do the right thing.

rohit21agrawal commented 6 years ago

also, please note that the PackTask is not meant to be called directly from a target. Only the Pack target should be called.

clairernovotny commented 6 years ago

Related: https://github.com/NuGet/Home/issues/5379

clairernovotny commented 6 years ago

@rohit21agrawal I don't think they have to support all project types. There's only one additional glob that has to be included and that's a directory (and everything in it) that matches the library name in the output directory.

rrelyea commented 6 years ago

@rohit21agrawal - is the plan to open an internal issue on the xaml project system team?

rohit21agrawal commented 6 years ago

@onovotny we don't rely on globbing paths or patterns to get outputs, instead we rely on well known msbuild output groups to get the output for a nuget package.

@rrelyea @zhili1208 is currently engaging with the UWP team and XAML Compiler team on email thread to discuss this issue.

clairernovotny commented 6 years ago

Here is a fix for the UWP issue that relies on output groups: https://github.com/onovotny/MSBuildSdkExtras/blob/master/src/build/platforms/Windows.targets#L54-L66

@rrelyea there is already a target in the XAML targets that populates a variable from an output group PrepareLibraryLayout. That fills in the _layout items that I'm using, but it's based on the items set in that target.

rohit21agrawal commented 6 years ago

thanks @onovotny ! i'll get this reviewed with XAML folks, and see if we can check this in.

clairernovotny commented 6 years ago

Probably needs a bit of polish, but that's the general idea

commonsensesoftware commented 6 years ago

@onovotny so is TargetPath custom metadata or is that already generated by PrepareLibraryLayout? If isn't not custom metadata, is there way to influence it?

I know you're very familiar with the required build layout for packaging, so no need to rehash it. I have a scenario where I'm multi-targeting with different XAML by platform. You can't have two Generic.xaml files, so I have things split by platform. For example:

Platforms
├─ net45
│  └─ Themes\Generic.xaml
├─ win81
│  └─ Themes\Generic.xaml
└─ wpa
│  └─ Themes\Generic.xaml
└─ uap10.0
   └─ Themes\Generic.xaml

Without any other influence to the metadata, the output ends up being bin\$(Configuration)\Platforms\win81\Themes\Generic.xaml instead of bin\$(Configuration)\win81\$(PackageId)\Themes\Generic.xaml. At least for XAML content, the source file and folder conventions may be incongruent with the required output, which is always well-known and consistent.

You can get a sense of my scenario from one of my working branches here.

@onovotny, @rohit21agrawal on a related note, which version of Visual Studio is this recommendation running against? I'm using 15.3.2 and I'm having issues building UWP apps. MSBuild keeps reporting that the targets are being resolved from the .NET SDK folder, which doesn't contain the UWP targets. This only started happening after .NET SDK 2.0 and 15.3.0 were released. I've engaged separately with some folks on the .NET SDK team, but I'm not sure the cause so it's difficult to find the right team. My build only continues to work if I force a version of the .NET SDK < 2.0 (ex: 1.1.0 or 1.0.4) in the global.json. Unfortunately, that doesn't help with this issue because @rohit21agrawal suggestion to use TfmSpecificPackageFile is only available starting in the most recent release. If you've observed this, know of a workaround, or know how to determine which team should diagnose this issue, please share.

rohit21agrawal commented 6 years ago

@commonsensesoftware have suggested some contacts on skype for the sdk related issues you are facing.

commonsensesoftware commented 6 years ago

@rohit21agrawal it would nice if the authors of the XAML targets make this easier out-of-the-box since it's so common, but the issue is fully mitigated by the use of TargetsForTfmSpecificContentInPackage. Thanks.

clairernovotny commented 6 years ago

Can you please re-open this? This really should be handled by the default pack targets. I have the solution in the Extras:

https://github.com/onovotny/MSBuildSdkExtras/blob/fa415e0af7e7897b5b5bbb60e02e6a13621e8775/MSBuild.Sdk.Extras/build/Platform.targets#L36-L47

If it's a matter of submitting that as a PR here, I'm happy to.

commonsensesoftware commented 6 years ago

Done. Hopefully this will get some attention - soon.

rohit21agrawal commented 6 years ago

@onovotny I will accept a PR

clairernovotny commented 6 years ago

@rohit21agrawal can you please take a look at those lines in the extras and let me know if a) it looks like the right thing, and b) where in the nuget targets it should go?

rohit21agrawal commented 6 years ago

the lines in extras look good, but you don't need TargetsForTfmSpecificContent since you will be writing this directly into Pack.

This is the target that would need to depend an additional target "_GetXamlLibraryLayoutFiles" conditioned on the property "PrepareLibraryLayout" , and return an ItemGroup which will get added to BuildOutputInPackage . Should be pretty straight forward.

https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Build.Tasks.Pack/NuGet.Build.Tasks.Pack.targets#L381