-
-
Notifications
You must be signed in to change notification settings - Fork 57
chore: Make repo compatible to be built on Windows #2352
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
|
||
<Target Name="PrintVersions" BeforeTargets="BeforeResolveReferences" Condition="'$(MSBuildProjectName)' == 'Sentry.Unity'"> | ||
<Message Text="Building the Unity SDK with:%0a UnityVersion: $(UnityVersion)%0a TargetFramework: $(TargetFramework)" Importance="High" /> | ||
<Message Text="Building the Unity SDK with:%0a UnityVersion: $(UnityVersion)%0a TargetFramework: $(TargetFramework)" Importance="High" /> |
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.
This just indents it properly on the console.
m_EditorVersion: 2021.3.45f2 | ||
m_EditorVersionWithRevision: 2021.3.45f2 (88f88f591b2e) |
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.
This is the latest generally available version with the f2
security patch.
<Message Importance="High" Text="Building Sentry Android SDK." /> | ||
|
||
<Exec WorkingDirectory="$(SentryAndroidRoot)" EnvironmentVariables="JAVA_HOME=$(JAVA_HOME_17_X64)" Command="./gradlew -PsentryAndroidSdkName=sentry.native.android.unity :sentry-android-core:assembleRelease :sentry-android-ndk:assembleRelease :sentry:jar --no-daemon --stacktrace --warning-mode none" /> | ||
<Exec WorkingDirectory="$(SentryAndroidRoot)" Command="./gradlew -PsentryAndroidSdkName=sentry.native.android.unity :sentry-android-core:assembleRelease :sentry-android-ndk:assembleRelease :sentry:jar --no-daemon --stacktrace --warning-mode none" /> |
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 had to set JAVA_HOME
to JAVA_HOME_17_X64
due to 17
not being the default on GH runners at the time. But the fact that we're hardcoded overwriting this during the build is 1. no longer required and 2. messing with local dev environments.
<_UnityPathProp>%(_UnityPathsFoundReversed.Identity)</_UnityPathProp> | ||
</PropertyGroup> | ||
|
||
<Message |
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.
While setting up the dev environment on Windows, this message would have saved me all the headaches in the world.
CONTRIBUTING.md
<Target Name="BuildAndroidSDK" | ||
DependsOnTargets="DownloadCLI" | ||
Condition="'$(MSBuildProjectName)' == 'Sentry.Unity' And !Exists('$(SentryAndroidArtifactsDestination)')" | ||
Condition="'$(MSBuildProjectName)' == 'Sentry.Unity' And (!Exists('$(SentryAndroidArtifactsDestination)') Or (Exists('$(SentryAndroidArtifactsDestination)') And $([System.IO.Directory]::GetFiles('$(SentryAndroidArtifactsDestination)').Length) != 4))" |
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.
Soft-checking for the content: Otherwise, if the target half succeeds and create the directory, subsequent runs will skip this target, assuming it is done and cached.
<Exec WorkingDirectory="$(SentryNativeRoot)" Command="strip -s build/libsentry.so -w -K sentry_[^_]* -o $(SentryLinuxArtifactsDestination)libsentry.so" /> | ||
<Exec WorkingDirectory="$(SentryNativeRoot)" Command="cp build/libsentry.so $(SentryLinuxArtifactsDestination)libsentry.dbg.so" /> | ||
<Copy SourceFiles="$(SentryNativeRoot)build/libsentry.so" DestinationFiles="$(SentryLinuxArtifactsDestination)libsentry.dbg.so" /> |
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.
This is a linux build so not expected to run on windows. The previous or following command would fail anyway.
Building the repo on Windows straight up does not work. I.e.
exec cp
does not exist. The contributing docs are out of date as well. So this fixes it.#skip-changelog