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

Is MSBuild truly necesary? #69

Open
masonwheeler opened this issue Nov 2, 2023 · 6 comments
Open

Is MSBuild truly necesary? #69

masonwheeler opened this issue Nov 2, 2023 · 6 comments

Comments

@masonwheeler
Copy link

The MSBuild dependencies make this code generator very heavyweight, transitively dragging over 30 other dependencies into the project, including bizarre things like System.Drawing.Common and a whole host of security/permissions related things, none of which feel like they have anything to do with running a simple code generator. (Or even a not-so-simple one that has to hand the code generation work off to a Java process.)

Would it be possible to produce a lightweight code generator package for those of us whose only use case is running the code generation and not any of the build-system stuff? Or is MSBuild somehow integral to that?

@masonwheeler
Copy link
Author

@KalleOlaviNiemitalo
Copy link

Copying my comments from dotnet/msbuild#9389 as requested by @rainersigwald:

Compare to the Microsoft.Build.Tasks.Git package, which provides MSBuild tasks and does not depend on any NuGet packages -- it instead assumes that the process has already loaded the MSBuild DLLs.

https://nuget.info/packages/Antlr4BuildTasks/12.4.0 shows lib/netstandard2.0/Antlr4BuildTasks.dll depending on these:

  • Microsoft.Build.Framework, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
  • Microsoft.Build.Utilities.Core, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a
  • netstandard, Version=2.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
  • System.Buffers, Version=4.0.2.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
  • System.Memory, Version=4.0.1.2, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
  • System.Text.Encoding.CodePages, Version=7.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a

The System.Text.Encoding.CodePages assembly dependency seems a bit suspect; I'm not sure that's always available in the MSBuild process, and I don't think the NuGet dependencies of Antlr4BuildTasks matter for that purpose at all.

In other words, Antlr4BuildTasks should

  • mark dependencies as private so that the resulting NuGet package does not depend on the packages that were used for building Antlr4BuildTasks
  • bundle dependencies into the package so that System.Buffers, System.Memory, and System.Text.Encoding.CodePages can be loaded at build time even if MSBuild does not depend on the same versions

@GeorgDangl
Copy link

@kaby76, we've discovered some problems related to System.Text.Encoding.CodePages as well. When we're updating to the lastest Antlr4BuildTasks dependency, we got a conflict because we had only referenced version 4.5.0 ourselves, but 7.0.0 was required. So we went ahead and updated our dependency as well.

This has worked fine on .NET 8, but we quickly got reports from users that it failed at runtime in .NET 6 environments due to not finding the System.Text.Encoding.CodePages dependency.

I didn't really find a workaround with the newer versions, so I switched back to 12.2.0, since all versions after seemed to produce the bug.

If I understood it correctly, it basically caused runtime errors for packages that referenced Antlr4BuildTasks and also internally relied on ´System.Text.Encoding.CodePages`. Setting all assets to private and only including build assets did also not resolve the issue.

@kaby76
Copy link
Owner

kaby76 commented Dec 13, 2023

System.Text.Encoding.CodePages

This is caused by Antlr4BuildTasks requiring Microsoft.Build.Utilities.Core which requires System.Text.Encoding.CodePages. Assuming that changes in the API are backward compatible, try adding <NoWarn>NU1605</NoWarn> to the first/top <PropertyGroup> in the .csproj and test.

@GeorgDangl
Copy link

Thanks! I did try it with <NoWarn>NU1605</NoWarn>, that seemed to work. But I'm not sure if it's correct to suppress a warning😀

I'm not really sure how to solve that, either. I don't have a lot of experience with building build tools for .NET. My first idea would probably be to try to somehow inline the dependencies, so the required dlls for Antlr4BuildTasks are copied to the NuGet package and resolved during the build execution via some custom assembly load logic, but that doesn't sound very clean.

@jbartlau
Copy link

Citing @masonwheeler's very helpful change from the other referenced thread in case anyone lands here - this fixed the issue of unneeded dependencies being copied to the output folder. I didn't notice any unwanted side effects.

- <PackageReference Include="Antlr4BuildTasks" Version="12.4.0" />
+ <PackageReference Include="Antlr4BuildTasks" Version="12.4.0" PrivateAssets="all" IncludeAssets="build" />

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants