Refactor packaging and remove `Fody.nuspec`. by teo-tsirpanis · Pull Request #1312 · Fody/Fody

teo-tsirpanis · 2025-03-15T22:37:53Z

You should already be a Patron

I am!

Description

The Fody package is currently being created with a .nuspec file which explicitly specifies the locations of every file in the package. This is not ideal because it relies in the exact output paths of the library and its dependencies, and also relies on the solution file to enforce a build order for some projects.

The solution

This PR removes Fody.nuspec and uses MSBuild facilities to define the structure of the Fody package. Some of the techniques used are described here.

Validated by manually inspecting that the files in the Fody package built from this branch are the same, with the following exceptions:

The task assemblies were moved from directories netclassictask and netstandardtask to tasks/net472 and tasks/netstandard2.0 for .NET Framework and .NET Standard respectively.
~~Next to the .NET Standard 2.0 task assembly there is a .deps.json file. This helps MSBuild resolve the task's dependencies.~~
~~FodyHelpers.xml was included in the package (I can remove it if I try more).~~

Todos

Related issues
Tests
Documentation

teo-tsirpanis · 2025-03-15T22:38:53Z


  <ItemGroup Condition="$(TargetFramework) == 'netstandard2.0'">
-    <PackageReference Include="System.Runtime.Loader" Version="4.3.0" />
+    <PackageReference Include="System.Runtime.Loader" Version="4.3.0" ExcludeAssets="Runtime" />


This makes System.Runtime.Loader.dll to not get included. It wasn't included before and it wasn't causing any problem.

teo-tsirpanis · 2025-03-15T23:40:10Z

I'm not sure why AzDo fails. 🤔

tom-englert · 2025-03-17T10:59:53Z

I'm not sure why AzDo fails. 🤔

Seems to be a server problem, after re-starting they all ran successful

tom-englert · 2025-03-17T11:09:04Z

@@ -0,0 +1,16 @@
+<Project>


What is the benefit from putting this into a global scope, and then only enabling for one single project?

It is enabled for both Fody and FodyHelpers.

ltrzesniewski

Here's a comparison of dotnet pack between the old and new:

That's obviously not good, and these changes seem more complicated than they should be. I'll do a second review after the package contains everything needed.

teo-tsirpanis · 2025-03-20T20:37:17Z

@ltrzesniewski the dotnet pack behavior seems to be by design (I cleaned-up the repo, ran it and it failed). We set GeneratePackageOnBuild=true in Fody.csproj, which for some reason means that the Build target does not implicitly run.

I've been making the packages on my machine with dotnet build -c Release and it works fine.

ltrzesniewski · 2025-03-21T18:32:46Z

Hmm, that's linked to NuGet/NuGet.Client#2822 and NuGet/Home#7801 which is supposed to be fixed... I think I'll play with the changes in this PR a bit and try to simplify them.

Do we even want to keep GeneratePackageOnBuild BTW?

teo-tsirpanis · 2025-03-22T00:18:09Z

With my latest commits, the list of files in Fody.nupkg is identical to the package released in NuGet, modulo the different folders of task assemblies of each framework.

tom-englert · 2025-03-22T16:26:50Z

+    <None Include="Fody.targets" Pack="true" PackagePath="build" />
+  </ItemGroup>
+
+  <!-- ExcludeAssets="compile" doesn't work on ProjectReference, and ReferenceOutputAssembly entirely


That's so much hacking with compiler internals. Wouldn't it be much clearer and easier to understand what's going on when we use `ReferenceOutputAssembly=false and then just copy the files manually?

e.g. like this:

<Target Name="CopyFodyIsolatedToOutput" AfterTargets="CopyFilesToOutputDirectory"> <ItemGroup> <_FodyIsolatedFile Include="..\FodyIsolated\bin\$(Configuration)\$(TargetFramework)\*.*" /> </ItemGroup> <Copy SourceFiles="@(_FodyIsolatedFile)" DestinationFolder="$(OutputPath)" /> </Target>

My proposed solution does precisely what we want to achieve (remove FodyIsolated from the references we pass to the compiler), and I believe it is less fragile and more maintainable down the road.

Manually copying the output of FodyIsolated assumes that it gets written to a specific location (which could change in the future), and that the project targets the same frameworks as Fody. My proposed solution makes none of these assumptions. Hardcoding output paths also brings back what we were doing in the .nuspec file, the removal of which is the objective of this PR.

I will file a feature request to MSBuild to make this scenario easier (a CopyLocalOutputAssembly metadata on ProjectReference).

tom-englert · 2025-03-23T14:52:54Z

What about moving the NuGet generation to a separate project? This would make all the workarounds obsolete:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFrameworks>netstandard2.0;net472</TargetFrameworks>
    <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
    <IncludeBuildOutput>false</IncludeBuildOutput>
    <BuildOutputTargetFolder>tasks</BuildOutputTargetFolder>
    <SuppressDependenciesWhenPacking>true</SuppressDependenciesWhenPacking>
    <PackageId>Fody</PackageId>
  </PropertyGroup>

  <ItemGroup>
    <ProjectReference Include="..\FodyIsolated\FodyIsolated.csproj" />
    <ProjectReference Include="..\Fody\Fody.csproj" />
  </ItemGroup>

</Project>

ltrzesniewski · 2025-03-23T14:57:39Z

I thought about it, but didn't have time to try it yet. I have used this approach in a few projects of mine and it works well.

BTW I'd put <IsPackable>false</IsPackable> in Directory.Build.props - Fody produces lots of useless packages today.

tom-englert · 2025-03-23T14:59:58Z

Fody produces lots of useless packages today.

Really? Which ones?

ltrzesniewski · 2025-03-23T15:02:29Z

When I run dotnet pack, I get this:

tom-englert · 2025-03-23T15:04:13Z

I see, I never ran dotnet pack, since build already does the job

ltrzesniewski · 2025-03-23T15:05:06Z

Oh, and here are examples of how I use a separate project to produce a package:

In RazorBlade
In ZeroLog

tom-englert · 2025-03-23T15:06:16Z

The above sample does exactly what we need in terms of binaries, just metadata and Fody.targets need to be added.

ltrzesniewski · 2025-03-23T15:08:07Z

I haven't tried it yet, but doesn't that <PackageId>Fody</PackageId> property interfere with <ProjectReference Include="..\Fody\Fody.csproj" />?

tom-englert · 2025-03-23T15:11:04Z

Yes, there is a warning during restore, that project needs to be renamed.

tom-englert · 2025-03-23T15:12:56Z

It's only a POC that this is sufficient to output the binaries as we need them

ltrzesniewski · 2025-03-23T15:38:13Z

I like it if it gets rid of the more complex stuff.

Do you intend to work on it a bit more and push it to this PR?

tom-englert · 2025-03-23T16:13:36Z

@teo-tsirpanis what do you think? Maybe you can do an adaption, since I probably could continue earliest next weekend.

teo-tsirpanis · 2025-03-23T19:30:09Z

I took a first stab at this and created a separate Fody.Package project referencing Fody and FodyIsolated. I got an error : Ambiguous project name 'Fody'., which means that we would have to rename the Fody project to something like Fody.Tasks, which significantly expands the scope of the PR, and by doing this I feel I will spend more time on this PR than I originally planned.

@tom-englert, feel free to push on my branch whenever you have time, thanks.

Also the approach used by RazorBlade and ZeroLog is IMHO an overkill. These are "simple" packages that do not bundle their dependencies and have just an analyzer and some MSBuild files added on top, and the extra packaging project causes the library to be compiled twice as many times.

ltrzesniewski · 2025-03-23T21:19:25Z

Also the approach used by RazorBlade and ZeroLog is IMHO an overkill. These are "simple" packages that do not bundle their dependencies and have just an analyzer and some MSBuild files added on top, and the extra packaging project causes the library to be compiled twice as many times.

Yes, I'm aware. Both of these libraries have good reasons to be set up like that, and they're not as simple as they look like at first glance. The multiple compilation is not an issue at all (both of these projects even have some files be compiled 3 times).

Of course, I wouldn't use this approach for more standard libraries though.

tom-englert · 2025-04-13T13:59:48Z

Superseded by #1313

teo-tsirpanis added 2 commits March 12, 2025 04:08

Refactor packaging and remove Fody.nuspec.

20e005e

Use robust way to exclude FodyIsolated reference from Fody.

13c9e71

teo-tsirpanis commented Mar 15, 2025

View reviewed changes

teo-tsirpanis added 2 commits March 16, 2025 00:43

Add comment.

17ed73f

Fix typo.

8aa9267

teo-tsirpanis marked this pull request as ready for review March 15, 2025 23:40

ltrzesniewski self-requested a review March 16, 2025 14:55

tom-englert requested changes Mar 17, 2025

View reviewed changes

ltrzesniewski requested changes Mar 17, 2025

View reviewed changes

Comment thread Fody/Fody.targets

teo-tsirpanis requested review from ltrzesniewski and tom-englert March 20, 2025 20:39

teo-tsirpanis added 2 commits March 21, 2025 20:33

Do not pack .deps.json file.

a032b83

Do not pack XML documentation files in the Fody package.

3c3749b

Remove PrivateAssets from ProjectReferences.

c362fe2

tom-englert requested changes Mar 22, 2025

View reviewed changes

Comment thread FodyHelpers/FodyHelpers.csproj Outdated

teo-tsirpanis added 2 commits March 22, 2025 18:04

Remove more PrivateAssets.

a66ed35

Remove all PrivateAssets from Fody and FodyHelpers.

b4217d6

tom-englert requested changes Mar 22, 2025

View reviewed changes

tom-englert mentioned this pull request Apr 8, 2025

Refactor packaging and remove Fody.nuspec #2. #1313

Merged

tom-englert closed this Apr 13, 2025

teo-tsirpanis mentioned this pull request Jun 21, 2025

Target .NET 8 instead of .NET Standard 2.0 and support unloading the IsolatedAssemblyLoadContext. #1317

Closed

Uh oh!

Uh oh!

Conversation

teo-tsirpanis commented Mar 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

You should already be a Patron

Description

The solution

Todos

Uh oh!

teo-tsirpanis Mar 15, 2025

Choose a reason for hiding this comment

Uh oh!

teo-tsirpanis commented Mar 15, 2025

Uh oh!

tom-englert commented Mar 17, 2025

Uh oh!

Uh oh!

tom-englert Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

teo-tsirpanis Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ltrzesniewski left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

teo-tsirpanis commented Mar 20, 2025

Uh oh!

ltrzesniewski commented Mar 21, 2025

Uh oh!

teo-tsirpanis commented Mar 22, 2025

Uh oh!

Uh oh!

tom-englert Mar 22, 2025

Choose a reason for hiding this comment

Uh oh!

tom-englert Mar 22, 2025

Choose a reason for hiding this comment

Uh oh!

teo-tsirpanis Mar 22, 2025

Choose a reason for hiding this comment

Uh oh!

tom-englert commented Mar 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ltrzesniewski commented Mar 23, 2025

Uh oh!

tom-englert commented Mar 23, 2025

Uh oh!

ltrzesniewski commented Mar 23, 2025

Uh oh!

tom-englert commented Mar 23, 2025

Uh oh!

ltrzesniewski commented Mar 23, 2025

Uh oh!

tom-englert commented Mar 23, 2025

Uh oh!

ltrzesniewski commented Mar 23, 2025

Uh oh!

tom-englert commented Mar 23, 2025

Uh oh!

tom-englert commented Mar 23, 2025

Uh oh!

ltrzesniewski commented Mar 23, 2025

Uh oh!

tom-englert commented Mar 23, 2025

Uh oh!

teo-tsirpanis commented Mar 23, 2025

Uh oh!

ltrzesniewski commented Mar 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tom-englert commented Apr 13, 2025

Uh oh!

Reviewers

Assignees

teo-tsirpanis commented Mar 15, 2025 •

edited

Loading

ltrzesniewski left a comment •

edited

Loading

tom-englert commented Mar 23, 2025 •

edited

Loading

ltrzesniewski commented Mar 23, 2025 •

edited

Loading