-
-
Notifications
You must be signed in to change notification settings - Fork 225
fix: Native AOT linking #4298
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
Merged
Merged
fix: Native AOT linking #4298
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
19b741f
ci: use safer non-cone-mode sparse checkout for integration tests
jpnurmi 2a82dff
top-level Directory.Build.props is required
jpnurmi 5be8490
retain src/**/Release/*.(s)nupkg
jpnurmi 9c8f845
dump preprocessed msbuild for integration test
jpnurmi 2c84157
temp skip tests to speed up things
jpnurmi e900d6c
reorder
jpnurmi e1b1b72
diag Sentry.Native.targets
jpnurmi 090070f
try binlog
jpnurmi 5484bc8
fix binlog path
jpnurmi 65ce81c
undo everything
jpnurmi 199cb8c
ci: native-aot-docker
jpnurmi 6474302
dotnet 9.0 & name
jpnurmi 0903986
composite
jpnurmi 1ec3d8c
test AOT on all platforms
jpnurmi 49a3f8c
check if docker is available
jpnurmi 51e9778
grouped and detailed output
jpnurmi 28d776a
clean up docker containers
jpnurmi a6489dd
allow scripts for mac
jpnurmi 1086327
try turning it into a pester integration test
jpnurmi 44d819a
fix nuget.config
jpnurmi 8a8315b
set NUGET_PACKAGES
jpnurmi 81b051f
finally, apply the copy-paste fix
jpnurmi be9d67e
fix /var vs. /private/var mismatch on macos
jpnurmi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| # This file contains test cases for https://pester.dev/ | ||
| Set-StrictMode -Version Latest | ||
| $ErrorActionPreference = 'Stop' | ||
|
|
||
| Describe 'Publish' { | ||
| BeforeAll { | ||
| $package = Get-ChildItem -Path "$PSScriptRoot/../src/Sentry/bin/Release/Sentry.*.nupkg" -File | Select-Object -First 1 | ||
| if (-not $package) | ||
| { | ||
| throw "No NuGet package found in src/Sentry/bin/Release." | ||
| } | ||
|
|
||
| $tempDir = Resolve-Path ([System.IO.Path]::GetTempPath()) | ||
| $tempDir = Join-Path $tempDir ([System.IO.Path]::GetRandomFileName()) | ||
| New-Item -ItemType Directory -Path $tempDir -Force | Out-Null | ||
| Set-Location $tempDir | ||
| Write-Host "Testing $package in $tempDir" | ||
|
|
||
| Write-Host "::group::Setup local NuGet source" | ||
| $localPackages = Join-Path $tempDir "packages" | ||
| New-Item -ItemType Directory -Path $localPackages -Force | Out-Null | ||
| Copy-Item $package $localPackages | ||
| $localConfig = Join-Path $tempDir "nuget.config" | ||
| Copy-Item $PSScriptRoot/nuget.config $localConfig | ||
| (Get-Content $localConfig) -replace '\./packages', $localPackages | Set-Content $localConfig | ||
| $env:NUGET_PACKAGES = Join-Path $tempDir "nuget" | ||
| New-Item -ItemType Directory -Path $env:NUGET_PACKAGES -Force | Out-Null | ||
| dotnet nuget list source | Write-Host | ||
| Write-Host "::endgroup::" | ||
|
|
||
| Write-Host "::group::Create test project" | ||
| dotnet new console --aot --name hello-sentry --output . | Write-Host | ||
| dotnet add package Sentry --prerelease --source $localPackages | Write-Host | ||
| @" | ||
| SentrySdk.Init(options => | ||
| { | ||
| options.Dsn = "https://[email protected]/42"; | ||
| options.Debug = true; | ||
| }); | ||
| Console.WriteLine("Hello, Sentry!"); | ||
| "@ | Set-Content Program.cs | ||
| Write-Host "::endgroup::" | ||
| } | ||
|
|
||
| AfterAll { | ||
| if ($tempDir -and (Test-Path $tempDir)) | ||
| { | ||
| Remove-Item $tempDir -Recurse -Force -ErrorAction SilentlyContinue | ||
| } | ||
| } | ||
|
|
||
| It 'Aot' { | ||
| dotnet publish -c Release | Write-Host | ||
| $LASTEXITCODE | Should -Be 0 | ||
|
|
||
| $tfm = (Get-ChildItem -Path "bin/Release" -Directory | Select-Object -First 1).Name | ||
| $rid = (Get-ChildItem -Path "bin/Release/$tfm" -Directory | Select-Object -First 1).Name | ||
| & "bin/Release/$tfm/$rid/publish/hello-sentry" | Write-Host | ||
| $LASTEXITCODE | Should -Be 0 | ||
| } | ||
|
|
||
| It 'Container' -Skip:(!$IsLinux -or !(Get-Command docker -ErrorAction SilentlyContinue)) { | ||
| dotnet publish -p:EnableSdkContainerSupport=true -t:PublishContainer | Write-Host | ||
| $LASTEXITCODE | Should -Be 0 | ||
|
|
||
| docker run --rm hello-sentry | Write-Host | ||
| $LASTEXITCODE | Should -Be 0 | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
We really need to get to the bottom of why these props are not being read from:
sentry-dotnet/src/Sentry/buildTransitive/Sentry.props
Lines 5 to 7 in be9d67e
... and the closely related:
sentry-dotnet/Directory.Build.props
Lines 44 to 45 in be9d67e
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.
Sentry.propsis read but beforeTargetFrameworkis set 🙁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.
I don't think so no. See:
I think MS Build runs a couple of passes though... once to do stuff that is framework specific (when the
TargetFrameworkis set) and once to do stuff that is framework agnostic. That's why this condition was added to the props file:sentry-dotnet/src/Sentry/buildTransitive/Sentry.props
Line 4 in be9d67e
In the experimenting that I did, the agnostic run seems to happen after the specific stuff.
Any code in the props/targets/csproj files that depends on the
TargetFrameworkbeing set really needs to be guarded by that condition then, since otherwise it will get executed with an effective target framework version of0(and likely do something unexpected).All of this is just from experimenting and trying to reverse engineer it though... I'd feel way more comfortable if there were some docs somewhere explaining what the heck is going on. Whatever it is, it's not very intuitive.