-
-
Notifications
You must be signed in to change notification settings - Fork 221
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
Conversation
<!-- 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'" |
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 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" |
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.
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'" |
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.
The condition needs to be added to the new one's as '$(_SentryIsNet8OrGreater)' == 'true'
does not resolve due to TF being removed.
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'm flabbergasted how the target change helps at all... but if it does... 🤷
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.
When building the SDK I keep having the build break on me with.
Digging through it I run into this
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