Skip to content

fix: Clean repo builds no longer fail when building native #4366

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 7 commits into from
Jul 18, 2025

Conversation

bitsandfoxes
Copy link
Contributor

@bitsandfoxes bitsandfoxes commented Jul 17, 2025

When building the SDK I keep having the build break on me with.

Sentry net8.0 failed with 2 error(s) (3.2s)
  EXEC : CMake error : CMAKE_CXX_COMPILER not set, after EnableLanguage
  /Users/bitfox/Workspace/sentry-unity/src/sentry-dotnet/src/Sentry/Platforms/Native/Sentry.Native.targets(93,5): error MSB3073: The command "pwsh ../../scripts/build-sentry-native.ps1 -Clean" exited with code 1.
Sentry net9.0 failed with 1 error(s) (3.0s)
  /Users/bitfox/Workspace/sentry-unity/src/sentry-dotnet/src/Sentry/Platforms/Native/Sentry.Native.targets(93,5): error MSB3073: The command "pwsh ../../scripts/build-sentry-native.ps1 -Clean" exited with code 1.

Digging through it I run into this

Sentry net9.0 failed with 1 error(s) (6.1s)
      -- The C compiler identification is AppleClang 17.0.0.17000013
      -- ... a bunch of other flags ...
      -- Build files have been written to: /Users/bitfox/Workspace/sentry-unity/src/sentry-dotnet/modules/sentry-native/build
      [  0%] Building C object CMakeFiles/sentry.dir/vendor/mpack.c.o
      [100%] Linking C static library libsentry.a
      [100%] Built target sentry
      Copying modules/sentry-native/build/libsentry.a to src/Sentry/Platforms/Native/sentry-native/osx/libsentry-native.a
      Copy-Item: /Users/bitfox/Workspace/sentry-unity/src/sentry-dotnet/scripts/build-sentry-native.ps1:88
      Line |
        88 |      Copy-Item -Force -Path $srcFile -Destination $outFile
           |      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
           | Cannot find path
           | '/Users/bitfox/Workspace/sentry-unity/src/sentry-dotnet/modules/sentry-native/build/libsentry.a' because it does not exist.
      /Users/bitfox/Workspace/sentry-unity/src/sentry-dotnet/src/Sentry/Platforms/Native/Sentry.Native.targets(93,5): error MSB3073: The command "pwsh /Users/bitfox/Workspace/sentry-unity/src/sentry-dotnet/scripts/build-sentry-native.ps1 -Clean" exited with code 1.
    Sentry net8.0 succeeded (6.7s) → src/Sentry/bin/Debug/net8.0/Sentry.dll
      -- The C compiler identification is AppleClang 17.0.0.17000013
      -- ... a bunch of other flags ...
      -- Build files have been written to: /Users/bitfox/Workspace/sentry-unity/src/sentry-dotnet/modules/sentry-native/build
      [ 23%] Building C object CMakeFiles/sentry.dir/vendor/mpack.c.o
      [ 96%] Building C object CMakeFiles/sentry.dir/src/transports/sentry_transport_none.c.o
      Linking C static library libsentry.a
      Built target sentry
      Copying modules/sentry-native/build/libsentry.a to src/Sentry/Platforms/Native/sentry-native/osx/libsentry-native.a

looks like building Sentry causes the build native target to run in parallel, corrupting the build output and failing.

So instead, we make use of this article explaining how to run a target really only once.

While debugging, the resolving the path to the build script kept breaking. So instead of ../.. we can go with $(MSBuildThisFileDirectory)_and then relative path from here_.

#skip-changelog

@bruno-garcia bruno-garcia requested review from jpnurmi and vaind July 17, 2025 14:18
<!-- We want to build the sentry-native only once, BEFORE building TFM specific verions of the SDK -->
<!-- How to run a target exactly once: https://learn.microsoft.com/visualstudio/msbuild/run-target-exactly-once-->
<Target Name="_BuildSentryNativeSDKBeforeOuterBuild"
Condition="'$(_SentryIsNet8OrGreater)' == 'true' and '$(CI)' != 'true'"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have to keep the condition on the "outer" target as the TF gets removed on line 108

@@ -85,11 +85,25 @@
<!-- Build the Sentry Native SDK (this only runs on local machines because in CI we expect the SDK to be
built already on each native platform and fetched for the final .net build. -->
<Target Name="_BuildSentryNativeSDK"
BeforeTargets="DispatchToInnerBuilds;BeforeBuild"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "when" now gets resolved by this target being "clamped" by the two new ones.

@@ -85,11 +85,25 @@
<!-- Build the Sentry Native SDK (this only runs on local machines because in CI we expect the SDK to be
built already on each native platform and fetched for the final .net build. -->
<Target Name="_BuildSentryNativeSDK"
BeforeTargets="DispatchToInnerBuilds;BeforeBuild"
Condition="'$(_SentryIsNet8OrGreater)' == 'true' and '$(CI)' != 'true'"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The condition needs to be added to the new one's as '$(_SentryIsNet8OrGreater)' == 'true' does not resolve due to TF being removed.

Copy link
Collaborator

@vaind vaind left a comment

Choose a reason for hiding this comment

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

I'm flabbergasted how the target change helps at all... but if it does... 🤷

Copy link
Member

@Flash0ver Flash0ver left a comment

Choose a reason for hiding this comment

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

Thanks for making our build a bit better.

I can imagine something similar will help me with #4307.
More pre-processed MSBuild project files for me to read,
and more .binlogs to explore get lost in, especially now with #4358.

@bitsandfoxes bitsandfoxes merged commit f1341ed into main Jul 18, 2025
28 checks passed
@bitsandfoxes bitsandfoxes deleted the fix/native-build-script-path branch July 18, 2025 06:58
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

Successfully merging this pull request may close these issues.

4 participants