1252804 - Move TARGET_LOCAL_INCLUDES to moz.build

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[Michael Shal [:mshal]]

ted, any recollection what would actually break from host builds that required bug 792391 in the first place? I tried just removing TARGET_LOCAL_INCLUDES and using LOCAL_INCLUDES in moz.build instead, and it seems to work: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a13f33970bbf

Comment 2

[Michael Shal [:mshal]]

Attachment: MozReview Request: Bug 1252804 - remove TARGET_LOCAL_INCLUDES; r?ted

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

Comment 3

[(not currently active) Ted Mielczarek]

(In reply to Michael Shal [:mshal] from comment #1)
> ted, any recollection what would actually break from host builds that
> required bug 792391 in the first place? I tried just removing
> TARGET_LOCAL_INCLUDES and using LOCAL_INCLUDES in moz.build instead, and it
> seems to work:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=a13f33970bbf

Looking at the state of toolkit/crashreporter/google-breakpad/src/common/android/include/ from when I landed bug 791775:
https://hg.mozilla.org/mozilla-central/file/eae123c9c884/toolkit/crashreporter/google-breakpad/src/common/android/include/sys

I think that directory just had more stuff in it at that time, so it's possible the stuff that was breaking the host build is just no longer there. It would have broken in one of the directories using TARGET_LOCAL_INCLUDES, so toolkit/crashreporter/google-breakpad/src/common/linux etc. If it works without those, then I'm fine with this. It's possible this breaks B2G builds somehow, but I'm not terribly concerned. (They can --disable-crashreporter if they have to.)

Comment 4

[(not currently active) Ted Mielczarek]

Comment on attachment 8748248 [details]
MozReview Request: Bug 1252804 - remove TARGET_LOCAL_INCLUDES; r?ted

https://reviewboard.mozilla.org/r/50195/#review47053

::: toolkit/crashreporter/google-breakpad/src/common/Makefile.in:7
(Diff revision 1)
> -endif
> -endif
> -
>  include $(topsrcdir)/config/rules.mk
>  
>  # memory.h in this dir breaks things if -I$(srcdir) gets added, since memory.h

Bleh, this is still a thing, eh? I wonder if the simplest workaround for this wouldn't just be to combine moz.build files and move them all up a few levels.

Have toolkit/crashreporter/breakpad.mozbuild or something, and have it do SOURCES += ['google-breakpad/src/common/...']

Comment 5

[Michael Shal [:mshal]]

(In reply to Ted Mielczarek [:ted.mielczarek] (Vacation May 5th-11th) from comment #4)
> Bleh, this is still a thing, eh? I wonder if the simplest workaround for
> this wouldn't just be to combine moz.build files and move them all up a few
> levels.
> 
> Have toolkit/crashreporter/breakpad.mozbuild or something, and have it do
> SOURCES += ['google-breakpad/src/common/...']

I filed bug 1270284 for this.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8fcd13137658

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8fcd13137658