-
-
Notifications
You must be signed in to change notification settings - Fork 226
fix: BuildProperty-Generator and AOT compilation detection #4333
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
Conversation
|
A couple of the integration tests are failing since they're erroneously detecting the the app has been compiled AOT when this is not the case. I believe it's due to this: sentry-dotnet/integration-test/runtime.Tests.ps1 Lines 94 to 105 in 24ab509
The test itself is valid. The sentry-dotnet/src/Sentry/Internal/AotHelper.cs Lines 20 to 28 in 0514bfe
A couple of things would need to change in that logic then:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refines the build‐property source generator to only emit code for executable outputs, renames and enhances the generated initializer, expands AOT detection, and updates related tests and MSBuild targets.
- Generator now uses
OutputKindExtensions.IsExe()and dynamic tool/version metadata to emitSentry.Generated.BuildPropertyInitializer.g.cs. AotHelpergains_IsPublishing&PublishSelfContainedchecks with diagnostic logging; called early inSentrySdk.InitHub.- MSBuild
.targetsadd the_IsPublishingandPublishSelfContainedproperties; tests and verified baselines updated.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/VerifySettingsInitializer.cs | Adds scrubber initializer to normalize version in snapshots. |
| test/BuildPropertySourceGeneratorTests.cs | Refactors driver setup to use OutputKind, updates hint name, and adds new scenarios. |
| src/Sentry/buildTransitive/Sentry.targets | Includes _IsPublishing and PublishSelfContained properties. |
| src/Sentry/buildTransitive/Sentry.SourceGenerators.targets | Mirrors the new MSBuild properties for the source‐generator. |
| src/Sentry/SentrySdk.cs | Invokes AotHelper.CheckIsTrimmed with logger. |
| src/Sentry/Internal/AotHelper.cs | Enhances trimmed detection with _IsPublishing and PublishSelfContained, adds logging. |
| src/Sentry.SourceGenerators/OutputKindExtensions.cs | New helper to test executable outputs. |
| src/Sentry.SourceGenerators/GeneratedCodeText.cs | Introduces dynamic tool/version values for generation. |
| src/Sentry.SourceGenerators/BuildPropertySourceGenerator.cs | Sealed class, raw string literal, indentation constant, renamed output, and metadata injection. |
| CHANGELOG.md | Notes fix for build‐setting detection in AOT contexts. |
Comments suppressed due to low confidence (2)
test/Sentry.SourceGenerators.Tests/BuildPropertySourceGeneratorTests.cs:52
- There’s no test covering the new
PublishSelfContainedpath inAotHelper.CheckIsTrimmed. Add a test scenario where_IsPublishing = trueandPublishSelfContained = trueto verify trimmed detection behaves as expected.
[SkippableFact]
test/Sentry.SourceGenerators.Tests/VerifySettingsInitializer.cs:1
- The scrubber method uses StringBuilder but there's no
using System.Text;directive. Add it to ensure the code compiles.
using System.CodeDom.Compiler;
Resolves #4331:
Proposing a bunch of changes to the Generated-code, considering the Generator so far never emitted any sources to the consuming compilation
<CompilerVisibleProperty Include="OutputType" />in packedSentry.targets, so the Generator never emitted any sourcesCompilation.Options.OutputKindnew Dictionary<string, string>tonew Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)#if NET8_0_OR_GREATERto#if NET5_0_OR_GREATERnet5.0net5.0#nullable enablenet5.0has a defaultLangVersionof9.0<Nullable>enable</Nullable>#nullable enableindividuallyinternal[ModuleInitializer]must be accessible from the containing modulepublicorinternalin a non-file-local-type in a non-local-functionSystem.Runtime.CompilerServices.CompilerGeneratedAttributewithSystem.CodeDom.Compiler.GeneratedCodeAttributeCompilerGeneratedAttributeattribute is reserved for Compiler usage onlyrecord-typesGeneratedCodeAttributeis intended for Tools and Roslyn-Generator authorsBuildVariableInitializertoBuildPropertyInitializer__BuildProperties.g.cstoSentry.Generated.BuildPropertyInitializer.g.csAdditionally, proposing changes to the Generator itself and our MSBuild / Packaging
Sentry.SourceGenerators.targetsSentry.targetsis automatically picked up from Sentry's NuGet-PackageSentry.SourceGenerators.targetswas neverImported in any shipped Build-filesBuildPropertySourceGeneratorNot-InheritableTests
GeneratedCodeAttributeto the string"Version"for test stability between releases