Skip to content

Enable MinGW builds #5

Merged
Swatinem merged 11 commits intogetsentry:getsentryfrom
Amphaal:getsentry_mingw
Mar 21, 2020
Merged

Enable MinGW builds #5
Swatinem merged 11 commits intogetsentry:getsentryfrom
Amphaal:getsentry_mingw

Conversation

@Amphaal
Copy link
Copy Markdown

@Amphaal Amphaal commented Mar 20, 2020

Requires uasm (https://github.com/Terraspace/UASM) to compile Windows ASM files, which is available through MSYS2 depos.

Copy link
Copy Markdown

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

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

lgtm!
The only question is about upstream compatibility.

@@ -0,0 +1,249 @@
// Copyright 2014 The Crashpad Authors. All rights reserved.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Are these files upstream? I wonder why they are not part of our fork?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ah, this is a patch to the dbghelp.h header file of MinGW, which misses somes references required by crashpad. I just copy-pasted the whole file from /compat/non_win/dbghelp.h and removed the duplicated references :P

@jan-auer
Copy link
Copy Markdown
Member

@Swatinem please squash-merge this when you're fine with it.

Just as a word of caution, since this adds significationly more modifications, this will make it harder to track Crashpad. Without looking into the details - should these changes potentially be fixed in upstream crashpad (excluding the CMake changes, of course).

@Amphaal
Copy link
Copy Markdown
Author

Amphaal commented Mar 21, 2020

@Swatinem please squash-merge this when you're fine with it.

Just as a word of caution, since this adds significationly more modifications, this will make it harder to track Crashpad. Without looking into the details - should these changes potentially be fixed in upstream crashpad (excluding the CMake changes, of course).

You are right, but they might want their building process updated accordingly for testing purposes. We should definitely poke the Crashpad team at some point so they can check our work on this.

@Swatinem Swatinem merged commit 5e7b7b8 into getsentry:getsentry Mar 21, 2020
@Swatinem
Copy link
Copy Markdown

We should definitely poke the Crashpad team at some point so they can check our work on this.

Please do so. I merged this, to unblock further work on sentry-native, but would be nice to actually move this upstream as well. Which reminds me that we should maybe also mare an upstream merge at some point.

@Amphaal
Copy link
Copy Markdown
Author

Amphaal commented Mar 22, 2020

https://groups.google.com/a/chromium.org/forum/#!topic/crashpad-dev/Y2tt6dDNIBI here we go.

Which reminds me that we should maybe also mare an upstream merge at some point.

That's a great idea !

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.

3 participants