1407368 - Rename Breakpad's src/common/memory.h

Status: RESOLVED FIXED

(Toolkit :: Crash Reporting, enhancement)

Description

[(not currently active) Ted Mielczarek]

We have a dumb makefile rule in a Makefile for Breakpad to fiddle with INCLUDES because the build system always has -I$(srcdir), and that directory has a header named memory.h, which shadows a system header:
https://hg.mozilla.org/mozilla-central/file/77a4c52e9987d2359969d7c478183b438b464744/toolkit/crashreporter/google-breakpad/src/common/Makefile.in#l9

We could figure out a smarter way to fix this, but I propose we just upstream a patch to rename that header and move on in life.

Comment 1

[(not currently active) Ted Mielczarek]

Upstream patch:
https://chromium-review.googlesource.com/c/breakpad/breakpad/+/723261

Once that lands I'll cherry-pick it for m-c.

Comment 2

[(not currently active) Ted Mielczarek]

This landed upstream, I should have a patch against m-c today.

Comment 3

[Chris Manchester (limited bugmail, email directly)]

We can get rid of http://searchfox.org/mozilla-central/rev/dca019c94bf3a840ed7ff50261483410cfece24f/toolkit/crashreporter/google-breakpad/src/common/moz.build#67 once this lands as well.

Comment 4

[(not currently active) Ted Mielczarek]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f7b43c1d2702a8542d372fd1bd5a3d981e47097e

Comment 5

[(not currently active) Ted Mielczarek]

Attachment: bug 1407368 - rename Breakpad's src/common/memory.h.

memory.h conflicts with a system header, so we have workarounds to
change include paths to work around this.

This is mostly a cherry-pick of this upstream commit:
https://chromium.googlesource.com/breakpad/breakpad/+/8bb3d55af734fd469213e0b88f5b5ad7ffec0abf

..but the patch was applied separately to toolkit/crashreporter/google-breakpad
and toolkit/crashreporter/breakpad-client since we've forked the latter,
and there's also one other fixup of a source file included.

Review commit: https://reviewboard.mozilla.org/r/190368/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190368/

Comment 6

[(not currently active) Ted Mielczarek]

Attachment: bug 1407368 - remove workarounds in google-breakpad/src/common.

Review commit: https://reviewboard.mozilla.org/r/190370/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/190370/

Comment 7

[(not currently active) Ted Mielczarek]

(In reply to Chris Manchester (:chmanchester) from comment #3)
> We can get rid of
> http://searchfox.org/mozilla-central/rev/
> dca019c94bf3a840ed7ff50261483410cfece24f/toolkit/crashreporter/google-
> breakpad/src/common/moz.build#67 once this lands as well.

Thanks! I caught that by accident as I was making sure I had fixed everything to work with this patch. I split the patches here into "just the rename" and "remove the workarounds".

Comment 8

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8919529 [details]
bug 1407368 - rename Breakpad's src/common/memory.h.

https://reviewboard.mozilla.org/r/190368/#review195758

LGTM. I've been thinking of pulling from breakpad for a while but I've got so many other things to do I keep postponing it.

Comment 9

[(not currently active) Ted Mielczarek]

(In reply to Gabriele Svelto [:gsvelto] from comment #8)
> LGTM. I've been thinking of pulling from breakpad for a while but I've got
> so many other things to do I keep postponing it.

Yeah, no worries. I don't think there are a lot of upstream fixes that'd actually be useful to us anyway.

Comment 10

[Michael Shal [:mshal]]

Comment on attachment 8919530 [details]
bug 1407368 - remove workarounds in google-breakpad/src/common.

https://reviewboard.mozilla.org/r/190370/#review196008

Comment 11

[(not currently active) Ted Mielczarek]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e53f2b4f821d8792793bfe0527287244de50edcd
bug 1407368 - rename Breakpad's src/common/memory.h. r=gsvelto

https://hg.mozilla.org/integration/mozilla-inbound/rev/608fe196e102b22bd7fc3c7f0db4342aa2a99f4f
bug 1407368 - remove workarounds in google-breakpad/src/common. r=mshal

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/e53f2b4f821d
https://hg.mozilla.org/mozilla-central/rev/608fe196e102