Closed
Bug 1336548
Opened 8 years ago
Closed 8 years ago
Fork Breakpad's src/client into mozilla-central
Categories
(Toolkit :: Crash Reporting, defect)
Toolkit
Crash Reporting
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: ted, Assigned: ted)
References
Details
Attachments
(2 files)
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 | ||
Updated•8 years ago
|
Assignee: nobody → ted
Assignee | ||
Comment 1•8 years ago
|
||
Assignee | ||
Comment 2•8 years ago
|
||
Assignee | ||
Comment 3•8 years ago
|
||
(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.
Assignee | ||
Comment 4•8 years ago
|
||
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 7•8 years ago
|
||
(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 8•8 years ago
|
||
mozreview-review |
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 9•8 years ago
|
||
mozreview-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+
Assignee | ||
Comment 10•8 years ago
|
||
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.
Assignee | ||
Comment 11•8 years ago
|
||
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.
Assignee | ||
Comment 12•8 years ago
|
||
There are some failing browser-chrome tests, but they all seem to be bug 1336654.
Comment 13•8 years ago
|
||
(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.
Comment 14•8 years ago
|
||
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
Comment 15•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/fca4d2503eb8
https://hg.mozilla.org/mozilla-central/rev/6f669880477a
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•