Closed Bug 1014814 Opened 11 years ago Closed 11 years ago

cset f75676ac4f7d broke some types of console logging in fennec and b2g

Categories

(Core :: Audio/Video, defect)

ARM
Android
defect
Not set
blocker

Tracking

()

RESOLVED FIXED
mozilla32
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed

People

(Reporter: kats, Assigned: ajones)

References

()

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

The inbound cset f75676ac4f7d has broken the harness that I use to generate data for areweslimyet.com/mobile. Fennec seems to still run and load pages just fine, but at the end of the run the harness dumps a message to logcat [1] which goes through the console service [2]. This message (and any others printed by the harness) are not getting pushed to logcat, resulting the harness to time out and fail. I don't know what kind of magic that cset is doing with __android_log_print but I suspect it broke it for some scenarios. [1] https://github.com/staktrace/awsy-armv6/blob/master/fennec-addon-awsy/bootstrap.js#L203 [2] https://github.com/staktrace/awsy-armv6/blob/master/fennec-addon-awsy/bootstrap.js#L8
ni? to ajones just to make sure it gets a response. areweslimyet.com/mobile will not get any new data until this is fixed.
Flags: needinfo?(ajones)
It has it's own version of that code which is probably diverting the output to the console. I will look into it.
Flags: needinfo?(ajones)
Assignee: nobody → ajones
This also prevents the benchmarking stuff in bug 1014042 from working.
Attached patch Fix Android log (obsolete) (deleted) — Splinter Review
https://tbpl.mozilla.org/?tree=Try&rev=e0bba98e5d64 The attached patch should fix the issue. There's something a little strange going on with my Android build so I haven't been able test it out.
Comment on attachment 8427577 [details] [diff] [review] Fix Android log The try build fixes the problem I filed this bug about, and makes the AWSY harness work again. I'm in favor of landing it ASAP.
Attachment #8427577 - Flags: feedback+
Comment on attachment 8427577 [details] [diff] [review] Fix Android log @Anthony: could you also explain what the mechanism of the bug was and how it related to unified build?
Attachment #8427577 - Flags: review?(ehsan)
This commit also broke printf_stderr (and the attached patch fixes it). It'd be great to get this landed soon, debugging on mobile is pretty tedious without printf :)
Comment on attachment 8427577 [details] [diff] [review] Fix Android log Review of attachment 8427577 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/libstagefright/moz.build @@ -72,5 @@ > 'frameworks/av/media/libstagefright/SampleTable.cpp', > 'frameworks/av/media/libstagefright/Utils.cpp', > - 'system/core/libcutils/strdup16to8.c', > - 'system/core/liblog/logd_write.c', > - 'system/core/liblog/logprint.c', If you don't want to build these files on non-Android, please add them to UNIFIED_SOURCES not SOURCES.
Attachment #8427577 - Flags: review?(ehsan) → review-
Summary: cset f75676ac4f7d broke some types of console logging in fennec → cset f75676ac4f7d broke some types of console logging in fennec and b2g
This is also breaking logcat on B2G.
blocking-b2g: --- → 2.0?
Component: General → Video/Audio
Keywords: regression
Product: Firefox for Android → Core
Attached patch Fix Android log (obsolete) (deleted) — Splinter Review
Attachment #8428415 - Flags: review?(cpearce)
Attachment #8427577 - Attachment is obsolete: true
Comment on attachment 8428415 [details] [diff] [review] Fix Android log Review of attachment 8428415 [details] [diff] [review]: ----------------------------------------------------------------- Somebody who has a clue about build systems should review this, especially since Mike was providing input in bug 1015487.
Attachment #8428415 - Flags: review?(cpearce) → review?(mh+mozilla)
Ignore Windows 64 fix included in patch.
This is breaking development for dozens of gaia developers. If we can't get a patch landed in the next few hours, can we get a backout of f75676ac4f7d?
Comment on attachment 8428415 [details] [diff] [review] Fix Android log Review of attachment 8428415 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/libstagefright/moz.build @@ +35,5 @@ > '-Wno-comment', > '-Wno-incompatible-pointer-types-discards-qualifiers' > ] > > +if not CONFIG['OS_TARGET'] == 'Android': if CONFIG['OS_TARGET'] != 'Android' @@ +43,5 @@ > + UNIFIED_SOURCES += [ > + 'system/core/libcutils/strdup16to8.c', > + 'system/core/liblog/logd_write.c', > + 'system/core/liblog/logprint.c', > + ] You can also move FAKE_LOG_DEVICE here.
Attachment #8428415 - Flags: review?(mh+mozilla) → review+
(In reply to Mike Hommey [:glandium] from comment #16) > @@ +43,5 @@ > > + UNIFIED_SOURCES += [ > > + 'system/core/libcutils/strdup16to8.c', > > + 'system/core/liblog/logd_write.c', > > + 'system/core/liblog/logprint.c', > > + ] > > You can also move FAKE_LOG_DEVICE here. Making fake_log_device.c able to compile unified is a lower priority than landing this fix.
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #17) > Making fake_log_device.c able to compile unified is a lower priority than > landing this fix. I meant the define.
(In reply to Mike Hommey [:glandium] from comment #18) > I meant the define. Oh.. yeah. I was wondering why you were shouting... :-P
Can we land this?
Severity: normal → blocker
Attached patch Fix Android log (deleted) — Splinter Review
Applied review changes; carrying r+
Attachment #8428415 - Attachment is obsolete: true
Does this fix logging for b2g as well? It looks to be a no-op for b2g if I'm reading correctly.
(In reply to Benoit Girard (:BenWa) from comment #24) > Does this fix logging for b2g as well? It looks to be a no-op for b2g if I'm > reading correctly. Applying the patch fixed logcat for me on B2G.
(In reply to Benoit Girard (:BenWa) from comment #24) > Does this fix logging for b2g as well? It looks to be a no-op for b2g if I'm > reading correctly. OS_TARGET in Android for b2g builds.
(In reply to Mike Hommey [:glandium] from comment #26) > (In reply to Benoit Girard (:BenWa) from comment #24) > > Does this fix logging for b2g as well? It looks to be a no-op for b2g if I'm > > reading correctly. > > OS_TARGET in Android for b2g builds. And I tested the patch on my flame and observed that logging had returned to normal.
blocking-b2g: 2.0? → 2.0+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
(In reply to Dave Hylands [:dhylands] from comment #27) > And I tested the patch on my flame and observed that logging had returned to > normal. This is because `CONFIG['OS_TARGET'] == 'Android'` is true for b2g builds.
Glad to have this fix and get back GeckoConsole output in logcat. Thanks Anthony!
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: