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)
Tracking
()
Tracking | Status | |
---|---|---|
b2g-v2.0 | --- | fixed |
People
(Reporter: kats, Assigned: ajones)
References
()
Details
(Keywords: regression)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
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
Reporter | ||
Comment 1•11 years ago
|
||
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)
Assignee | ||
Comment 2•11 years ago
|
||
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 | ||
Updated•11 years ago
|
Assignee: nobody → ajones
Updated•11 years ago
|
Comment 3•11 years ago
|
||
This also prevents the benchmarking stuff in bug 1014042 from working.
Assignee | ||
Comment 4•11 years ago
|
||
Assignee | ||
Comment 5•11 years ago
|
||
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.
Reporter | ||
Comment 6•11 years ago
|
||
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 7•11 years ago
|
||
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)
Comment 8•11 years ago
|
||
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 9•11 years ago
|
||
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-
Updated•11 years ago
|
Summary: cset f75676ac4f7d broke some types of console logging in fennec → cset f75676ac4f7d broke some types of console logging in fennec and b2g
Updated•11 years ago
|
Assignee | ||
Comment 12•11 years ago
|
||
Attachment #8428415 -
Flags: review?(cpearce)
Assignee | ||
Updated•11 years ago
|
Attachment #8427577 -
Attachment is obsolete: true
Comment 13•11 years ago
|
||
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)
Assignee | ||
Comment 14•11 years ago
|
||
Ignore Windows 64 fix included in patch.
Comment 15•11 years ago
|
||
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 16•11 years ago
|
||
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+
Assignee | ||
Comment 17•11 years ago
|
||
(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.
Comment 18•11 years ago
|
||
(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.
Assignee | ||
Comment 19•11 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #18)
> I meant the define.
Oh.. yeah. I was wondering why you were shouting... :-P
Assignee | ||
Comment 22•11 years ago
|
||
Applied review changes; carrying r+
Assignee | ||
Updated•11 years ago
|
Attachment #8428415 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 23•11 years ago
|
||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 24•11 years ago
|
||
Does this fix logging for b2g as well? It looks to be a no-op for b2g if I'm reading correctly.
Comment 25•11 years ago
|
||
(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.
Comment 26•11 years ago
|
||
(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.
Comment 27•11 years ago
|
||
(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.
Updated•11 years ago
|
blocking-b2g: 2.0? → 2.0+
Comment 28•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Updated•11 years ago
|
status-b2g-v2.0:
--- → fixed
Assignee | ||
Comment 29•11 years ago
|
||
(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.
Comment 30•11 years ago
|
||
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.
Description
•