[crashpad] Support more targets and cross-compile for Android #23827

jason-ortiz · 2022-03-28T22:38:41Z

Updates Crashpad port to build for Windows, OSX, Linux, and allows those hosts to cross-compile for Android. Also introduces generate-dump feature to optionally build and install generate_dump tool for live-process-dump-creation.

What does your PR fix?

Fixes [crashpad] Support Linux/Android targets and arm64 architecture #22891
Fixes [crashpad] Build failure #25197
Which triplets are supported/not supported? Have you updated the CI baseline?

Supports:
- x64-windows-static
- x86-windows-static
- arm64-windows-static
- x64-linux
- x64-osx
- x64-android
- x86-android
- arm64-android
- arm-android
CI baseline is updated.
Does your PR follow the maintainer guide?

Yes, I believe so.
If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

Yes

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 5f62686b96cecc91a5d02ea188857a24efd3ef4f -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/c-/crashpad.json b/versions/c-/crashpad.json
index 0f97ead..969fe1c 100644
--- a/versions/c-/crashpad.json
+++ b/versions/c-/crashpad.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "3fd92538923181d2b7932e3958d98cf99be75fa9",
+      "git-tree": "626b86347c4129cc313f7ea12f0db584992d186d",
       "version-date": "2022-03-28",
       "port-version": 0
     },

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

ports/crashpad/vcpkg.json

Valid values for the license field can be found in the documentation

jason-ortiz · 2022-03-28T23:00:33Z

Question: should I include some sort of readme about how to actually integrate Crashpad? A usage file? What would be recommended? When figuring out how to integrate Android crashpad, I relied on https://docs.bugsplat.com/introduction/getting-started/integrations/mobile/android quite a bit.

jason-ortiz · 2022-03-28T23:01:01Z

@vejmartin / @JackBoosY, please review

janisozaur · 2022-03-29T04:52:03Z

Is it possible to add arm64-windows as well?

ports/crashpad/crashpadConfig.cmake.in

ports/crashpad/update-toolchain.patch

jason-ortiz · 2022-03-29T19:12:02Z

Is it possible to add arm64-windows as well?

@janisozaur , yes, and I just got a local build of it to work

jason-ortiz · 2022-03-30T15:41:06Z

@LilyWangLL / @JackBoosY , is there a way to get clang installed on the CI x64-linux machine? I would've wanted to use vcpkg_find_acquire_program(CLANG), but currently that just tells you to run sudo apt-get install clang. I think I could mimic downloading a release from the LLVM GitHub project, but not sure if we're okay with that approach or not.

JackBoosY · 2022-03-31T02:25:47Z

@BillyONeal Request clang in Linux image.

BillyONeal · 2022-03-31T03:18:03Z

@LilyWangLL / @JackBoosY , is there a way to get clang installed on the CI x64-linux machine? I would've wanted to use vcpkg_find_acquire_program(CLANG), but currently that just tells you to run sudo apt-get install clang. I think I could mimic downloading a release from the LLVM GitHub project, but not sure if we're okay with that approach or not.

Ports don't get to be that opinionated about which compiler is used; they need to use what the triplet selected.

@BillyONeal Request clang in Linux image.

I'm nervous about doing that because we explicitly want to prevent this kind of thing.

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff

diff --git a/ports/crashpad/vcpkg.json b/ports/crashpad/vcpkg.json
index 5518974..8eb44dd 100644
--- a/ports/crashpad/vcpkg.json
+++ b/ports/crashpad/vcpkg.json
@@ -8,11 +8,11 @@
   "homepage": "https://chromium.googlesource.com/crashpad/crashpad/+/master/README.md",
   "supports": "(x64 & (osx | windows | linux | android)) | (arm & android) | (arm64 & (windows | linux | android))",
   "dependencies": [
-    "zlib",
     {
       "name": "vcpkg-gn",
       "host": true
-    }
+    },
+    "zlib"
   ],
   "features": {
     "generate-dump": {

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for crashpad but no changes to version or port version.
-- Version: 2022-03-28
-- Old SHA: 3fd92538923181d2b7932e3958d98cf99be75fa9
-- New SHA: 8276c3090c038cd372dc7725bef2de381c038599
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

ports/crashpad/vcpkg.json

Valid values for the license field can be found in the documentation

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff

diff --git a/ports/crashpad/vcpkg.json b/ports/crashpad/vcpkg.json
index eede4a5..c222681 100644
--- a/ports/crashpad/vcpkg.json
+++ b/ports/crashpad/vcpkg.json
@@ -7,7 +7,7 @@
   ],
   "homepage": "https://chromium.googlesource.com/crashpad/crashpad/+/master/README.md",
   "license": "Apache-2.0",
-  "supports": "(osx | windows | android | linux)",
+  "supports": "osx | windows | android | linux",
   "dependencies": [
     {
       "name": "vcpkg-tool-gn",

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout e9d2b4c40e3a3ac4c6264dd685d59df8d082ace1 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 875e830..02e1ec7 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -1669,7 +1669,7 @@
       "port-version": 1
     },
     "crashpad": {
-      "baseline": "2022-04-16",
+      "baseline": "2022-04-27",
       "port-version": 0
     },
     "crashrpt": {
diff --git a/versions/c-/crashpad.json b/versions/c-/crashpad.json
index 013a9da..34891c9 100644
--- a/versions/c-/crashpad.json
+++ b/versions/c-/crashpad.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "e87c265c481cae3521a1f01881a0ce72b032acc8",
+      "version-date": "2022-04-27",
+      "port-version": 0
+    },
     {
       "git-tree": "448abcac90e97d8b5ee03843775dd7fcba971979",
       "version-date": "2022-04-16",

jason-ortiz · 2022-04-27T22:42:02Z

@bold84, any chance you can help with validation of arm64-OSX locally? My machine's too old to validate that. Also, were you able to successfully build arm-windows with your change? When I try to build that, I get issues related to a missing toolchain.

bold84 · 2022-04-28T08:57:08Z

What I'm particular do you mean by "validation"?

I'll try a Windows arm64 build and let you know. Give me an hour or two.

jason-ortiz · 2022-04-28T13:44:04Z

What I'm particular do you mean by "validation"?

@bold84, I effectively mean that you are able to build successfully. I was able to build arm64-windows fine. I wasn't able to build arm-windows and arm64-osx. Since your recent change to the vcpkg.json broadened the scope of supported targets for OSX & Windows, I need some help validating if those builds succeed.

With your original change, I assume you were able to build all the new permutations of triplets that weren't supported before (i.e., arm, arm64, x86 for windows and arm64-osx).

bold84 · 2022-04-28T18:13:13Z

What I'm particular do you mean by "validation"?

@bold84, I effectively mean that you are able to build successfully. I was able to build arm64-windows fine. I wasn't able to build arm-windows and arm64-osx. Since your recent change to the vcpkg.json broadened the scope of supported targets for OSX & Windows, I need some help validating if those builds succeed.

With your original change, I assume you were able to build all the new permutations of triplets that weren't supported before (i.e., arm, arm64, x86 for windows and arm64-osx).

arm64-osx ✅

arm64-windows ✅

x86-windows ✅

arm-windows ❌
Doesn't seem to be supported by crashpad. I even set the target_cpu flag to "arm" in the portfile.

I believe it's possible to make this work. You'd just have to read the BUILD.gn files and figure out what needs to be adjusted.

jason-ortiz · 2022-06-21T21:07:57Z

@ras0219-msft / @BillyONeal , any updates regarding the ability to move forward with this PR?

BillyONeal · 2022-06-24T22:20:48Z

I don't believe anything has changed or is likely to change soon, no.

njakob · 2022-06-25T13:20:34Z

After reading through the current discussions, I'm still trying to understand what is the current blocker. Is it related to the changes adding support for more targets or purely having the port using an external toolchain?

The crashpad port seems to not be working as it's missing some generated headers (see #25197). Therefore, unless there is a workaround, the community will need to use 2022.04.12.

Knowing that the port was already using an external toolchain before (if I understand @jason-ortiz comments), does it mean that any attempt of fixing the port would be rejected unless a solution is found through one of the ideas that have been suggested in #23827 (comment)?

BillyONeal · 2022-06-27T19:04:00Z

After reading through the current discussions, I'm still trying to understand what is the current blocker. Is it related to the changes adding support for more targets or purely having the port using an external toolchain?

The problem is that we make the compiler to use a user setting. vcpkg says "you WILL use the compiler the user says" and this port is trying to say "NO, I will ignore you and use Clang instead" which isn't OK in general.

Knowing that the port was already using an external toolchain before (if I understand @jason-ortiz comments)

I don't see the current port using an external toolchain. In fact there's a lot of stuff in there trying to ensure user settings are respected. That may be imperfect but we certainly were not downloading clang before.

jason-ortiz · 2022-06-27T21:44:18Z

I don't see the current port using an external toolchain. In fact there's a lot of stuff in there trying to ensure user settings are respected. That may be imperfect but we certainly were not downloading clang before.

In this previous comment, I mentioned that compiler choice is hardcoded:

Crashpad & mini_chormium already have hardcoded much of their settings/config via .gn & .gni files; it is already expected by those repos that you're using Clang to build for Linux, Mac, and Android. You can see some of that here:
https://github.com/chromium/mini_chromium/blob/main/build/config/BUILD.gn#L400
https://github.com/chromium/mini_chromium/blob/main/build/compiler.gni
Even today before my changes, this port includes the windows.cmake toolchain, yet still has compilers and linkers hardcoded for Windows too here: https://github.com/chromium/mini_chromium/blob/main/build/config/BUILD.gn#L626

Also, my changes to the port (i.e., vcpkg_find_acquire_program(CLANG)) don't download clang - it just checks for its existence and throws an error if it doesn't.

In summary, I've not changed how this port handles user settings; it already ignores them/has requirements for certain compilers.

ras0219-msft · 2022-07-25T21:14:50Z

ports/crashpad/portfile.cmake

+    message(STATUS "Setting GN options for Android")
+
+    if (VCPKG_CRT_LINKAGE STREQUAL "static")
+        string(APPEND VCPKG_COMBINED_SHARED_LINKER_FLAGS_DEBUG " -static-libstdc++ ")


This is suspicious -- what if the user wants a different standard library setting? This seems like it should already be part of the combined flags.

If it isn't, we likely need to change vcpkg_cmake_get_vars

ras0219-msft · 2022-07-26T20:53:19Z

ports/crashpad/update-toolchain.patch

+diff --git a/build/config/BUILD.gn b/build/config/BUILD.gn
+index e32e40d..654bffc 100644
+--- a/build/config/BUILD.gn
+++ b/build/config/BUILD.gn


Before merging this, I want to double check that this is the minimal approach. For example, if we can simply ensure "framework_dirs" is empty when not targeting macos/ios, that would enormously reduce the length of the patch and be less concerning about unintended side effects.

ras0219-msft · 2022-07-26T21:03:25Z

ports/crashpad/vcpkg.json

  "description": [
    "Crashpad is a crash-reporting system.",
    "Crashpad is a library for capturing, storing and transmitting postmortem crash reports from a client to an upstream collection server. Crashpad aims to make it possible for clients to capture process state at the time of crash with the best possible fidelity and coverage, with the minimum of fuss."
  ],
  "homepage": "https://chromium.googlesource.com/crashpad/crashpad/+/master/README.md",
  "license": "Apache-2.0",
-  "supports": "osx | windows",
+  "supports": "static & (((x64 | x86 | arm64) & windows) | (x64 & (osx | linux)) | android)",


Generally ports should not be marked as only supporting static libraries -- vcpkg policy allows ports to ignore dynamic lib requests by instead calling the port helper vcpkg_check_linkage(ONLY_STATIC_LIBRARY).

We strongly prefer known-bad lists over known-good lists since they enable users to work through configurations we may not know about. If possible, it would be great to change this conditional to block known-bad configurations instead of trying to enumerate every known-good configuration. As an example: do we know that iOS doesn't work? Do we know that MacOS arm64 doesn't work?

Suggested change

"supports": "static & (((x64 | x86 | arm64) & windows) | (x64 & (osx | linux)) | android)",

"supports": "((x64 | x86 | arm64) & windows) | (x64 & (osx | linux)) | android",

njakob · 2022-08-14T18:49:47Z

This PR seems to be the best chance to resolve #25197 and at this point, I'm wondering if I'm the only Crashpad's port user running into building errors with the current port on Windows 😅 I did double check this is still the case with the latest vcpkg release.

Therefore, I'm wondering if you could have a look at the latest comment @jason-ortiz and how does that connect with the work that has been done in #25864?

JackBoosY · 2022-08-19T07:54:38Z

Ping @jason-ortiz for reply.

jim-wordelman-msft · 2022-08-24T23:38:06Z

Hi everyone

Jason is no longer working in this code unfortunately. At this point, anyone who is willing to take over the change and get it over the finish line is super welcome to, but we don't have the resources on our side to finish this off

JackBoosY · 2022-08-26T08:05:17Z

Convert to draft since there is no reply.

JackBoosY · 2022-09-02T09:45:51Z

Any progrees here?

JackBoosY · 2022-09-09T09:18:39Z

Drop vcpkg-team-review until this PR has any progress.

JackBoosY · 2022-09-16T08:27:47Z

Closing this PR since it seems that no progress is being made. Please ping me to reopen if work is still being done.

github-actions bot requested changes Mar 28, 2022

View reviewed changes

jason-ortiz changed the title ~~Update Crashpad port to support more targets and to cross-compile for Android~~ [crashpad] Support more targets and cross-compile for Android Mar 28, 2022

jason-ortiz mentioned this pull request Mar 28, 2022

[gn, crashpad] Add GN build support and crashpad port #10505

Merged

Cheney-W assigned LilyWangLL Mar 29, 2022

LilyWangLL added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist category:community-triplet A PR or issue related to community triplets not officially validated by the vcpkg team. labels Mar 29, 2022

tusharb86 reviewed Mar 29, 2022

View reviewed changes

ports/crashpad/crashpadConfig.cmake.in Outdated Show resolved Hide resolved

tusharb86 reviewed Mar 29, 2022

View reviewed changes

ports/crashpad/update-toolchain.patch Show resolved Hide resolved

janisozaur mentioned this pull request Mar 30, 2022

MSVC ARM64 OpenRCT2/OpenRCT2#16887

Merged

JackBoosY added the depends:vm-update PR contains changes to the VM provisioning scripts label Mar 31, 2022

JackBoosY removed the depends:vm-update PR contains changes to the VM provisioning scripts label Mar 31, 2022

LilyWangLL assigned JackBoosY Apr 1, 2022

github-actions bot requested changes Apr 22, 2022

View reviewed changes

jason-ortiz force-pushed the dev/jason-ortiz/crashpad branch from fd5b1c8 to 6b554aa Compare April 27, 2022 21:00

github-actions bot requested changes Apr 27, 2022

View reviewed changes

jason-ortiz force-pushed the dev/jason-ortiz/crashpad branch from 6b554aa to d319643 Compare April 27, 2022 22:21

github-actions bot approved these changes Apr 27, 2022

View reviewed changes

jason-ortiz marked this pull request as ready for review April 27, 2022 22:36

jason-ortiz added 2 commits June 14, 2022 18:39

Use vcpkg-cmake-get-vars instead of toolchains directly

da0f6ff

Update crashpad git-tree hash

d69d8bd

jason-ortiz force-pushed the dev/jason-ortiz/crashpad branch from c5942ba to d69d8bd Compare June 14, 2022 22:39

github-actions bot approved these changes Jun 14, 2022

View reviewed changes

LilyWangLL mentioned this pull request Jun 15, 2022

[crashpad] Install chromeos_buildflags.h #25225

Closed

LilyWangLL added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Jun 15, 2022

LilyWangLL requested a review from ras0219-msft June 15, 2022 02:21

ras0219-msft dismissed JackBoosY’s stale review via d69d8bd June 23, 2022 19:58

dan-shaw removed the info:reviewed Pull Request changes follow basic guidelines label Jun 23, 2022

Neumann-A mentioned this pull request Jul 20, 2022

[crashpad] use vcpkg_cmake_get_vars #25864

Merged

ras0219-msft reviewed Jul 26, 2022

View reviewed changes

JackBoosY added the requires:author-response label Jul 28, 2022

BillyONeal mentioned this pull request Aug 11, 2022

[mdl] Add port MDL-SDK 2021.1.2 #24633

Merged

JackBoosY marked this pull request as draft August 26, 2022 08:05

LilyWangLL mentioned this pull request Aug 30, 2022

[crashpad] Install chromeos_buildflags.h #26593

Merged

JackBoosY removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Sep 9, 2022

JackBoosY closed this Sep 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[crashpad] Support more targets and cross-compile for Android #23827

[crashpad] Support more targets and cross-compile for Android #23827

jason-ortiz commented Mar 28, 2022 •

edited by LilyWangLL

Loading

github-actions bot left a comment

jason-ortiz commented Mar 28, 2022

jason-ortiz commented Mar 28, 2022

janisozaur commented Mar 29, 2022

jason-ortiz commented Mar 29, 2022

jason-ortiz commented Mar 30, 2022

JackBoosY commented Mar 31, 2022

BillyONeal commented Mar 31, 2022

github-actions bot left a comment

github-actions bot left a comment

jason-ortiz commented Apr 27, 2022 •

edited

Loading

bold84 commented Apr 28, 2022

jason-ortiz commented Apr 28, 2022

bold84 commented Apr 28, 2022

jason-ortiz commented Jun 21, 2022

BillyONeal commented Jun 24, 2022

njakob commented Jun 25, 2022

BillyONeal commented Jun 27, 2022

jason-ortiz commented Jun 27, 2022

ras0219-msft Jul 25, 2022

ras0219-msft Jul 26, 2022

ras0219-msft Jul 26, 2022

njakob commented Aug 14, 2022

JackBoosY commented Aug 19, 2022

jim-wordelman-msft commented Aug 24, 2022

JackBoosY commented Aug 26, 2022

JackBoosY commented Sep 2, 2022

JackBoosY commented Sep 9, 2022

JackBoosY commented Sep 16, 2022

	"supports": "static & (((x64 \| x86 \| arm64) & windows) \| (x64 & (osx \| linux)) \| android)",
	"supports": "((x64 \| x86 \| arm64) & windows) \| (x64 & (osx \| linux)) \| android",

[crashpad] Support more targets and cross-compile for Android #23827

[crashpad] Support more targets and cross-compile for Android #23827

Conversation

jason-ortiz commented Mar 28, 2022 • edited by LilyWangLL Loading

What does your PR fix?

Which triplets are supported/not supported? Have you updated the CI baseline?

Does your PR follow the maintainer guide?

If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

github-actions bot left a comment

Choose a reason for hiding this comment

jason-ortiz commented Mar 28, 2022

jason-ortiz commented Mar 28, 2022

janisozaur commented Mar 29, 2022

jason-ortiz commented Mar 29, 2022

jason-ortiz commented Mar 30, 2022

JackBoosY commented Mar 31, 2022

BillyONeal commented Mar 31, 2022

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

jason-ortiz commented Apr 27, 2022 • edited Loading

bold84 commented Apr 28, 2022

jason-ortiz commented Apr 28, 2022

bold84 commented Apr 28, 2022

jason-ortiz commented Jun 21, 2022

BillyONeal commented Jun 24, 2022

njakob commented Jun 25, 2022

BillyONeal commented Jun 27, 2022

jason-ortiz commented Jun 27, 2022

ras0219-msft Jul 25, 2022

Choose a reason for hiding this comment

ras0219-msft Jul 26, 2022

Choose a reason for hiding this comment

ras0219-msft Jul 26, 2022

Choose a reason for hiding this comment

njakob commented Aug 14, 2022

JackBoosY commented Aug 19, 2022

jim-wordelman-msft commented Aug 24, 2022

JackBoosY commented Aug 26, 2022

JackBoosY commented Sep 2, 2022

JackBoosY commented Sep 9, 2022

JackBoosY commented Sep 16, 2022

jason-ortiz commented Mar 28, 2022 •

edited by LilyWangLL

Loading

If you have added/updated a port: Have you run `./vcpkg x-add-version --all` and committed the result?

jason-ortiz commented Apr 27, 2022 •

edited

Loading