Closed Bug 1252804 Opened 6 years ago Closed 5 years ago

Move TARGET_LOCAL_INCLUDES to moz.build

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox49 fixed)

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: Ms2ger, Assigned: mshal)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

No description provided.
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
Flags: needinfo?(ted)
Assignee: nobody → mshal
(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.)
Flags: needinfo?(ted)
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/...']
Attachment #8748248 - Flags: review?(ted) → review+
(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.
https://hg.mozilla.org/mozilla-central/rev/8fcd13137658
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.