Closed Bug 1270879 Opened 7 years ago Closed 7 years ago

Build Issue : Build still fails, looks like something within the crash reporter?

Categories

(Firefox OS Graveyard :: General, defect)

Product:
Firefox OS Graveyard
Component:
General
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED DUPLICATE of bug 1276927

People

(Reporter: nhirata, Assigned: gsvelto)

References

Details

(Whiteboard: fixed-in-pine)

Attachments

(5 files)

tail_of_build_log.txt
13.86 KB, text/plain
Details
[PATCH WIP] Fix the crashreporter build on b2g
3.42 KB, patch
Details | Diff | Splinter Review
gonk build failure
4.17 KB, text/plain
Details
workaround
736 bytes, text/plain
mshal
: feedback+
Details
MozReview Request: Bug 1270879 - Fix crashreporter build on Gonk r?
58 bytes, text/x-review-board-request
Details 
Looks like this might have been caused by bug 1252804 which changed the way the crash reporter includes are handled.
I just confirmed that it's indeed bug 1252804, I'll try to fix it. But guess what, there's more breakage still!
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Michael, I'm trying to figure out the breakage here but I cannot seem to put my finger on it. Since bug 1252804 landed it seems that we're not picking LOCAL_INCLUDES any more while building the files in the toolkit/crashreporter/google-breakpad/src/common/linux directory. The sources there are listed under HOST_SOURCES so I tried to alter config/rules.mk so that the rule for building HOST_CPPOBJS picks up LOCAL_INCLUDES correctly but it doesn't seem to help. Since you authored the patch there could you help me figure out what's wrong here?
Flags: needinfo?(mshal)
The includes were originally added in bug 792391, and were done in a way that they only applied to the target files, not the host files. When I moved them to moz.build in bug 1252804, it seemed like this was no longer an issue because everything built on try, but we don't build B2G on try anymore. So I believe the problem is that you now *are* picking up the LOCAL_INCLUDES to '/toolkit/crashreporter/gonk-include' for host files whereas you weren't before.

ted's suggestion in https://bugzilla.mozilla.org/show_bug.cgi?id=1252804#c3 is to build with --disable-crashreporter.

Another option if you can't do that might be to move the SOURCES and UNIFIED_SOURCES from toolkit/crashreporter/google-breakpad/src/common/linux/moz.build to this block a few directories up: https://dxr.mozilla.org/mozilla-central/rev/043082cb7bd8490c60815f67fbd1f33323ad7663/toolkit/crashreporter/moz.build#31

Then you could just leave the HOST_SOURCES and such in the linux/moz.build, and remove the LOCAL_INCLUDES from there (the same LOCAL_INCLUDES are already in toolkit/crashreporter/moz.build)

I haven't actually tried that though - feel free to ping me if you need a hand.
Flags: needinfo?(mshal)
If we "--disable-crashreporter"; won't we lose the crashreporter from reporting crashing in B2G?
I would guess so. Let me know if moving things into toolkit/crashreporter/moz.build presents a problem.
(In reply to Michael Shal [:mshal] from comment #5)
> The includes were originally added in bug 792391, and were done in a way
> that they only applied to the target files, not the host files. When I moved
> them to moz.build in bug 1252804, it seemed like this was no longer an issue
> because everything built on try, but we don't build B2G on try anymore. So I
> believe the problem is that you now *are* picking up the LOCAL_INCLUDES to
> '/toolkit/crashreporter/gonk-include' for host files whereas you weren't
> before.

You're right, somehow I got it backwards while looking at the compiler invocation parameters.

> ted's suggestion in https://bugzilla.mozilla.org/show_bug.cgi?id=1252804#c3
> is to build with --disable-crashreporter.
> 
> Another option if you can't do that might be to move the SOURCES and
> UNIFIED_SOURCES from
> toolkit/crashreporter/google-breakpad/src/common/linux/moz.build to this
> block a few directories up:
> https://dxr.mozilla.org/mozilla-central/rev/
> 043082cb7bd8490c60815f67fbd1f33323ad7663/toolkit/crashreporter/moz.build#31
> 
> Then you could just leave the HOST_SOURCES and such in the linux/moz.build,
> and remove the LOCAL_INCLUDES from there (the same LOCAL_INCLUDES are
> already in toolkit/crashreporter/moz.build)
> 
> I haven't actually tried that though - feel free to ping me if you need a
> hand.

I'll give it a spin and see if it fixes the problem, thanks!
Yep, moving the sources around seems to do the trick. I'm now checking if this breaks anything else.
This is a WIP patch that fixes the crashreporter build on b2g, but breaks Android and Linux. Nothing definitive yet but it can be used to check other build problems while I polish it.
Attached file gonk build failure — 
This is failing even on b2g for me :/
Maybe we should instead do bug 1091975 ?
Removing the LOCAL_INCLUDES that references gonk-includes fixed the build failure from attachment 8753817 [details]
Attached file workaround — 
With this change (removing all changes made previously by gabriele) I am able to build b2g (z3c kk). Does it makes sense?
Attachment #8753892 - Flags: feedback?(mshal)
What I currently see is libbreakpad_common_linux_s.a is not being build. It's required by testing/tools/fileid.
Comment on attachment 8753892 [details]
workaround

Looks fine to me if it's working properly for you. In bug 1069556 there is this comment:

* Upstream removed their Android-compat sys/ucontext.h because the Android NDK
  added it, but the bionic we're using for Gonk builds is too old, so add a
  copy of sys/ucontext.h from bionic master to keep Gonk building.

Is that no longer relevant?
Attachment #8753892 - Flags: feedback?(mshal) → feedback+
(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #16)
> What I currently see is libbreakpad_common_linux_s.a is not being build.
> It's required by testing/tools/fileid.

Can you clarify which patch this is an issue with? Or is it with the current tip and no patches applied?
(In reply to Michael Shal [:mshal] from comment #17)
> Comment on attachment 8753892 [details]
> workaround
> 
> Looks fine to me if it's working properly for you. In bug 1069556 there is
> this comment:
> 
> * Upstream removed their Android-compat sys/ucontext.h because the Android
> NDK
>   added it, but the bionic we're using for Gonk builds is too old, so add a
>   copy of sys/ucontext.h from bionic master to keep Gonk building.
> 
> Is that no longer relevant?

Hm, good catch, I will verify.
I am going to push this temp patch to pine, to unblock and I will investigate next week
(In reply to Michael Shal [:mshal] from comment #18)
> (In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #16)
> > What I currently see is libbreakpad_common_linux_s.a is not being build.
> > It's required by testing/tools/fileid.
> 
> Can you clarify which patch this is an issue with? Or is it with the current
> tip and no patches applied?

This happens with the WIP patch [1] applied.

[1] https://bugzilla.mozilla.org/attachment.cgi?id=8753314
(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #22)
> (In reply to Michael Shal [:mshal] from comment #18)
> > (In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #16)
> > > What I currently see is libbreakpad_common_linux_s.a is not being build.
> > > It's required by testing/tools/fileid.
> > 
> > Can you clarify which patch this is an issue with? Or is it with the current
> > tip and no patches applied?
> 
> This happens with the WIP patch [1] applied.
> 
> [1] https://bugzilla.mozilla.org/attachment.cgi?id=8753314

Never mind. Alex' patch seems to work.
After applying the proposed workaround I hit this error:

--
libdom_presentation.a.desc
In file included from /root/B2G/gecko/toolkit/crashreporter/google-breakpad/src/common/linux/elfutils.h:37:0,
                 from /root/B2G/gecko/toolkit/crashreporter/google-breakpad/src/common/linux/file_id.cc:44:
/root/B2G/objdir-gecko/dist/system_wrappers/link.h:3:23: fatal error: link.h: No such file or directory
 #include_next <link.h>
                       ^
compilation terminated
--

Not really sure of the "link.h" referenced here, any idea ?

Thanks,
s.
Flags: needinfo?(gsvelto)
(In reply to sahid from comment #24)
> After applying the proposed workaround I hit this error:
> 
> --
> libdom_presentation.a.desc
> In file included from
> /root/B2G/gecko/toolkit/crashreporter/google-breakpad/src/common/linux/
> elfutils.h:37:0,
>                  from
> /root/B2G/gecko/toolkit/crashreporter/google-breakpad/src/common/linux/
> file_id.cc:44:
> /root/B2G/objdir-gecko/dist/system_wrappers/link.h:3:23: fatal error:
> link.h: No such file or directory
>  #include_next <link.h>
>                        ^
> compilation terminated
> --
> 
> Not really sure of the "link.h" referenced here, any idea ?
> 
> Thanks,
> s.

The preprocessor searches the include paths in a specific order. 'include_next' includes the next found header. Apparently, there's none.

I have a fix for this problem in bug 1276927.
Clearing my NI since Thomas already responded.
Flags: needinfo?(gsvelto)
I'm closing this as duplicated as the problem has been fixed by the other bug's patch set.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.