Skip to content

chore: Fix reference to Sentry.SourceGenerators #4290

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

chore: Fix reference to Sentry.SourceGenerators #4290

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7329400
chore: Fix reference to Sentry.SourceGenerators
jamescrosswell Jun 15, 2025
4f152c7
fix: build and pack Sentry Generators without dependency
Flash0ver Jun 19, 2025
ac399b3
Merge branch 'main' into fix-source-gen-ref
Flash0ver Jun 23, 2025
b3ca680
build: replace TfmSpecificPackageFile with custom Target
Flash0ver Jun 23, 2025
ee425d8
build: remve GenerateNuspec from BeforeTargets
Flash0ver Jun 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/Sentry/Sentry.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,8 @@
</ItemGroup>

<ItemGroup>
<None Include="..\Sentry.SourceGenerators\bin\Release\netstandard2.0\Sentry.SourceGenerators.dll"
Pack="true"
PackagePath="analyzers/dotnet/cs"
Visible="false" />
<ProjectReference Include="..\Sentry.SourceGenerators\Sentry.SourceGenerators.csproj"
Copy link
Member

@Flash0ver Flash0ver Jun 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a feeling that this won't work as expected.
I don't think we are packing Sentry.SourceGenerators.dll to the expected PackagePath.

Unfortunately, the NuGetPackageExplorer is Windows-only.
So I can't test it right away.

We need either

  • both items, where we use a proper $(OutputPath)-based path rather than hard-coding it
  • or something with the TfmSpecificPackageFile-item in a target instead, because Sentry is multi-targeted, but Roslyn-Compiler-Extensions should be netstandard2.0

Also OutputItemType="Analyzer" seems wrong in this context as we don't want to consume the Generator in Sentry, but pack it in the same package alongside the Sentry lib/.

Let me get back to that when I'm on a Windows machine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right... this would work if we only wanted to use the analyzer when building the SentrySDK:

But we want to package it and have it run when building projects that are using the Sentry SDK.

When I built a source generator myself, I simply put this in the csproj file for the analyzer itself:
https://github.com/mentaldesk/ducktype/blob/d06ab90bc698a30e33911b19901eb6833891eed9/MentalDesk.DuckType/MentalDesk.DuckType.csproj#L48-L52

Then other projects that wanted to use that source generator added a PackageReference to the analyzer.

If we published Sentry.SourcfeGenerators as a NuGet, would it be enough to add a package reference from Sentry to Sentry.SourceGenerators? Would that then ensure that anyone who added Sentry also got Sentry.SourceGenerators?

Copy link
Member

@Flash0ver Flash0ver Jun 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes ... this should work ... this is what xunit does.
But ... I don't think we necessarily want this: Our SourceGen emits code that is depending on Sentry. The SourceGen does not make sense without Sentry. And also, I think it may be "dangerous" - to some degree" - to use different versions of the SourceGen-Package and the Sentry package.
I prefer to have the SourceGen within the Sentry package, similar to Microsoft.Extensions.Logging.Abstractions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change:

  • ProjectReference to reliably build the project as a dependency in the correct order
    • with ReferenceOutputAssembly to include in $(OutputPath)
    • but with PrivateAssets="all" so that Sentry does not have a dependency on Sentry.SourceGenerators
  • utilize TargetsForTfmSpecificContentInPackage
    • with a custom Target that packs Sentry.SourceGenerators in the expected PackagePath for Analyzer
      • via PackagePath="analyzers/dotnet/cs"

dotnet pack (Release) and check via NuGet Package Explorer on Windows:

image

No package dependency to Sentry.SourceGenerators

image

Does not reference the assembly Sentry.SourceGenerators

image

PackagePath="analyzers/dotnet/cs"

Copy link
Member

@Flash0ver Flash0ver Jun 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build fails on CI though ... need to reiterate 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From failing on all Platforms now only failing on macOS ... I'm getting there 😉

OutputItemType="Analyzer"
ReferenceOutputAssembly="false" />
</ItemGroup>
</Project>
Loading