Fork Breakpad's src/client into mozilla-central

RESOLVED FIXED in Firefox 54

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: ted, Assigned: ted)

Tracking

Trunk
mozilla54
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

Attachments

(2 attachments)

I mentioned this in another bug, but Google is no longer maintaining the Windows or OS X client libraries in Breakpad, and the patches for the Linux client seem to be pretty limited to ChromeOS support these days. We might as well just fork Breakpad's src/client code into mozilla-central instead of making extra work for ourselves by having to land changes upstream. We can continue to use the rest of Breakpad (src/tools, src/processor) from upstream since those parts are maintained.

A patch to fork the code should involve just:
1) `hg mv toolkit/crashreporter/google-breakpad/src/client toolkit/crashreporter/somewhere`
2) Fixup paths in moz.build files as appropriate.
3) Change the update-breakpad script to rm src/client after syncing.
Assignee: nobody → ted
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #1)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=376e29bf4556

The 32-bit Windows builds broke here in the injector code, I only built Win64 locally which doesn't build that. Should be fixed in the second try push. Otherwise it looks pretty green.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #3)
> The 32-bit Windows builds broke here in the injector code, I only built
> Win64 locally which doesn't build that. Should be fixed in the second try
> push. Otherwise it looks pretty green.

Third time's the charm. (I did a Win32 build locally and got it building, so I have more confidence now!)
Comment on attachment 8834841 [details]
bug 1336548 - fork breakpad src/client into toolkit/crashreporter/breakpad-client.

https://reviewboard.mozilla.org/r/110672/#review111978

This looks good to me, I still have to give it some local testing but if it passes a try run then I'm fairly certain everything's fine.

::: toolkit/crashreporter/moz.build
(Diff revision 1)
> -        'google-breakpad/src/client/linux/',
>          'google-breakpad/src/processor',
>          'google-breakpad/src/tools/linux/dump_syms',
>      ]
>  
> -elif CONFIG['OS_ARCH'] == 'SunOS':

Good that you've removed this stuff, we don't support it and it's also unmaintained in Breakpad.
Attachment #8834841 - Flags: review?(gsvelto) → review+
Comment on attachment 8834842 [details]
bug 1336548 - remove unused files from breakpad-client fork

https://reviewboard.mozilla.org/r/110674/#review112050

Likewise.
Attachment #8834842 - Flags: review?(gsvelto) → review+
I built locally on linux64/mac/win32/win64 and all the builds worked fine, and the try push had all green builds, so I don't expect anything to actually be broken here. As you saw in the patch the bulk of the work was just fixing up #include paths.

I did test the update-breakpad script atop this patch, and it shows some changes, but I believe that's because the last time you did a sync you did a bit of manual work.
For my own sanity I triggered mochitest browser-chrome and xpcshell tests on all platforms on my last try push, since we have crashreporter tests in both of those suites. Better safe than sorry.
There are some failing browser-chrome tests, but they all seem to be bug 1336654.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #10)
> I did test the update-breakpad script atop this patch, and it shows some
> changes, but I believe that's because the last time you did a sync you did a
> bit of manual work.

Yes, I took out two patches which were messing up building the module list because of multiple mappings that weren't being merged. We can consider fixing that up properly now that we're in control.
Pushed by tmielczarek@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fca4d2503eb8
fork breakpad src/client into toolkit/crashreporter/breakpad-client. r=gsvelto
https://hg.mozilla.org/integration/autoland/rev/6f669880477a
remove unused files from breakpad-client fork r=gsvelto
https://hg.mozilla.org/mozilla-central/rev/fca4d2503eb8
https://hg.mozilla.org/mozilla-central/rev/6f669880477a
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Depends on: 1338290
You need to log in before you can comment on or make changes to this bug.