Build sentry from source #706
Conversation
only windows tested currently
|
needed to make build self contained
|
@tustanivsky Any insights on the build issues. from what i can tell something is referencing |
|
Only thing left before final review is add the ability to build for both crashpad and breakpad independently |
# Conflicts: # CHANGELOG.md # modules/sentry-native
|
I have marked this as ready to review as all contents are ready. The is 1 thing missing related to building backtrace from CI and this looks to be a larger discussion |
|
|
|
Ah yeah when I looked at it before it looked to be a permissions issue but looks to be doesn't with that last build. I'll take a look |
| sentry-native/.codecov.yml | ||
| sentry-native/.craft.yml | ||
| sentry-native/.editorconfig | ||
| sentry-native/.git |
There was a problem hiding this comment.
Maybe a @tustanivsky question:
Should we be adding .git to the packages? This will include all commit history etc.
Similarly about .github, looks like we're adding the whole git submodule as-is.
plugin-dev/sentry-native
Outdated
There was a problem hiding this comment.
Did this require a change in sentry-native to work?
If so, lets get that added to main through a PR on sentry-native first. Then we bump main here and pull that into this branch.
This way we don't need to change sentry-native in this PR and having it pass CI checks that the change is compatible with it.
There was a problem hiding this comment.
There should be no changes to native. The current issues to CI imply im missing something with building Linux.
| public class Sentry : ModuleRules | ||
| { | ||
| [CommandLine("-usebreakpad")] | ||
| public bool bUseBreakpad = false; |
There was a problem hiding this comment.
If I say false, what am I using then?
And would that be true regardless of platform? (On Xbox today we only support breakpad so that's odd)
Seems like config will be more complicated than a bool as it'll depend on what platform you're targeting, and what's supported there.
There was a problem hiding this comment.
I should probably rename this. Its an override more then anything. Native already select the backend needed for selected platforms but this allows the build system to override to build to use breakpad on windows. its needed for shipping releases as we only ship breakpad for the marketplace version
There was a problem hiding this comment.
Yeah then it needs a better name, and docs on it (XML docs over C# members)
| { | ||
| string generatorName=""; | ||
|
|
||
| switch(compiler) |
There was a problem hiding this comment.
Do we cover all these branches in CI?
I don't think we're building on Windows yet, so we need to add that, for example.
There was a problem hiding this comment.
We do not currently build windows. i did talk to Ivan about this briefly when gathering information. Last info i got on the topic was during a weekly meeting it was mentioned he was pointed at some stuff that may help get it working.
As for the generators we currently dont build all the unreal generators even on linux. Im not even sure we try all cmake generators on the native side. But the general goal here is to map the unreal compiler to its best fit in the cmake space.
There was a problem hiding this comment.
We might be better off adding all of the CI infrastructure first, before we continue with this change.
That way we can be confident we started off with something that works, and the changes continue to do so
| } | ||
| public class CMakeTargetInst | ||
| { | ||
| // Based on UE4CMake with modifications |
There was a problem hiding this comment.
Is there a link to where this came from that you can copy and paste here?
Instead of linking to the file through a branch, use the commit sha (on GitHub, if you press y it changes the path from branch to sha automatically.
.gitmodules
Outdated
There was a problem hiding this comment.
Did it need to move to plugin-dev? It was located on modules before and it's unclear why it had to change.
There was a problem hiding this comment.
The SDK plugin now needs to know the path to native in order to build. I did originally try do this via symlink but with the SDK itself being used as a symlink it caused errors and pointed to the wrong direction. The safest and least error prone option for users was to simple move native into the SDK folder so that wherever you build SDK it can easily locate native without manual user steps
There was a problem hiding this comment.
It seems that one side effect is that we have the whole repo there (sentry-native) and a bunch of unrelated stuff shows up in the final package. I left notes on those snapshot files.
Also, why is it named plugin-dev then? is it relates to dev vs prod time?
Could sentry-native be a better name if that's what it is?
There was a problem hiding this comment.
If that's based on Unity: https://github.com/getsentry/sentry-unity/tree/cb71faad6304e437b7e6d063f198d99c4aa57069/package-dev
We named it like that because we have the package used in development and the one used for packaging and release.
Doesn't seem to be the same situation here
Co-authored-by: Bruno Garcia <bruno@brunogarcia.com>
|
As discussed i have reverted linux to make this initial version window and xbox only. |
|
The 5.0 branch is currently failing due to a bug in unreal https://issues.unrealengine.com/issue/UE-143579 |
* Restore old native submodule location * Fix submodule path in scripts * Fix submodule path * Remove redundant step for native * Remove cache check * Fix native upload * Update sdk download script * Fix ci * Update snapshot * Remove separate cmake c# script * Fix sdk download script for mac/linux * Disable source build by default
There was a problem hiding this comment.
After updating the process of building the Native SDK in Unreal (#771) we're good to proceed with the merge. Build scripts like Sentry.Build.cs and SentryNative.Build.cs could benefit from some cleanup but we can tackle that in a separate ticket. Also, we'll need to document how to build the Native SDK from within the Unreal plugin.
WIP changes to build sentry-native from source.
Currently i have only tested/developed on windows and will not yet fully compile due to not linking libraries.
The changes here are based on UE4CMake with modification and cleanup to better function for the needs of sentry.
going forward further modification will be needed to add platform support for all our needs.
In addition we will need to load custom toolchains for some platforms.
TODO: