helix-publishing-pipeline icon indicating copy to clipboard operation
helix-publishing-pipeline copied to clipboard

Support publish of content and contentFiles with NuGet PackageReference format

Open coreyasmith opened this issue 6 years ago • 13 comments

For Sitecore 9.1 and beyond, it's recommended to convert the ASP.NET Web Application projects in your solution to use the NuGet PackageReference format for reasons I won't expound here. After you convert an ASP.NET Web Application project to use the PackageReference format, config files in NuGet packages (e.g., Unicorn, Synthesis, Glass Mapper) will no longer be automatically added in to your project when those NuGet packages are installed or updated. Instead, you have to track down the configs in the %USERPROFILE%\.nuget\packages folder and manually copy them in to your project. This makes it easy to miss critical config changes when upgrading NuGet packages.

The ASP.NET Core Web Application project type does not suffer from this problem. When an ASP.NET Core Web Application project is published, all assets in the contentFiles directory of the NuGet packages that the project references are automatically published to the publish target. Due to this mechanism, it is not necessary for assets in contentFiles to be added to ASP.NET Core Web Application project types.

It would be awesome if HPP handled publishing of assets in the content and contentFiles folders of referenced NuGet packages when using the PackageReference format with the ASP.NET Web Application similar to the way they are handled for ASP.NET Core Web Application projects. This feature should not be enabled for ASP.NET Web Application projects still using the packages.config format. I think there should be some intelligence for packages with configs/assets duplicated in both the content and contentFiles folders for backwards compatibility (e.g., Synthesis) so that a double publish doesn't occur, if possible.

coreyasmith avatar Mar 04 '19 19:03 coreyasmith

Great idea. I propose the following rules:

  • Only run when the website project contains PackageReferences
  • Prefer contentFiles/ over content/
  • Prefer a project file to a package file (to allow overrides)
  • Allow .Helix transform files to transform package config files

Sound good?

richardszalay avatar Mar 04 '19 20:03 richardszalay

Sounds great.

coreyasmith avatar Mar 04 '19 20:03 coreyasmith

hey guys, the solution I'm working with right now is configured with PackageReferences and I've hit the same problem @coreyasmith described above. I'm wondering if this feature request is already being worked on as it sounds so much better than building my own custom build steps. If not, I'd be happy to take a stab at it

ezlateva avatar May 02 '19 14:05 ezlateva

@ezlateva just to clarify, if the PackageReference with the content a reference of Website or one of the modules?

richardszalay avatar May 03 '19 09:05 richardszalay

in my case, both actually

ezlateva avatar May 03 '19 13:05 ezlateva

Interesting. Looking the NuGet.targets, the content files should get published. I wonder if it doesn't follow that path when running via a legacy (non-SDK) csproj

richardszalay avatar May 03 '19 21:05 richardszalay

Yes I think that's the case @richardszalay.

coreyasmith avatar May 03 '19 21:05 coreyasmith

In which case, solving it for Website should be simple enough as we can trigger the same target and/or Task that gets used by sdk projects.

Modules will be more difficult. Need to think it through a little more.

richardszalay avatar May 03 '19 21:05 richardszalay

I'll look into it again as soon as I can. If either of you want to do the same, to figure it what's executing I recommend the MSBuild binary log viewer.

richardszalay avatar May 03 '19 21:05 richardszalay

I've had a little time to investigate this further:

Files in content/ are not supported at all in the PackageReference model. Since the're used by Unicorn and pretty much every other legacy-supporting package, we should definitely support this.

Files in contentFiles/ are explicitly skipped by both Visual Studio and NuGet CLI restores when a legacy project is using PackageReference`. I propose we do not support contentFiles due to:

  1. It's clearly intended only for .NET Core projects
  2. It supports additional complexities like language specific transforms

If/when Sitecore supports .NET Core, we can review the above.

Thoughts?

In both cases, adding support for content/ in PackageReferences on the Website project is on the lower end of effort but supporting PackageReferences of referenced modules is a bit more complex.

NB. We should definitely use the project lock file rather than the PackageReference items, since the former includes downstream references and the latter does not.

richardszalay avatar May 08 '19 04:05 richardszalay

I think that by the time all of Sitecore is running on .NET Core, we'll be using project formats that support all of the Package Reference features natively and HPP won't need to support contentFiles.

Focusing on publishing only files in the content folder will definitely be a big win. However, it'd still be nice to support publishing from the contentFiles folder just because I think it promotes better development habits.

coreyasmith avatar May 09 '19 01:05 coreyasmith

My only issue is that supporting it properly (ie. without breaking a bunch of packages), we'd have to reimplement all of the tokenisation transform support. content/ is much easier to support since it only supports a bunch of files. It does support config transforms, but we can leave for a future release.

richardszalay avatar May 09 '19 01:05 richardszalay

Here's my plan for the implementation:

  1. Create a custom task that:
  2. Parses the lock file (from $(ProjectLockFile))
  3. Finds packages that haven't been excluded (possibly via a custom ExcludeAssets metadata value?)
  4. Outputs a list of items as long as they don't exist in the current project
  5. Create a target that uses the new task to add the located content files into FilesForPackagingFromProject
  6. Create another target (in it's own file) that runs the task and adds the content files to Content, and configure this target with BeforeTargets="ContentFilesProjectOutputGroup"
  7. Update CollectFilesFromHelixModulesContent to inject the new targets file into the ContentFilesProjectOutputGroup invocation by using CustomBeforeMicrosoftCommonTargets

There will be no support for prioritising content from multiple projects that both include the same package. As with regular content files, this will generate a publish-time warning via WPP.

richardszalay avatar May 09 '19 02:05 richardszalay