Closed Bug 1067329 Opened 6 years ago Closed 6 years ago

Log shaking dumps at invalid place on sdcard

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.1+, firefox34 wontfix, firefox35 wontfix, firefox36 fixed, b2g-v2.1 wontfix, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S7 (24Oct)
blocking-b2g 2.1+
Tracking Status
firefox34 --- wontfix
firefox35 --- wontfix
firefox36 --- fixed
b2g-v2.1 --- wontfix
b2g-v2.2 --- verified

People

(Reporter: gerard-majax, Assigned: gerard-majax)

References

Details

(Keywords: dogfood, Whiteboard: [systemsfe])

Attachments

(1 file, 1 obsolete file)

> d---rwxr-x system   sdcard_rw          2014-09-15 12:37 2014-09-15-12-37-23
> ----rwxr-x system   sdcard_rw        0 2014-09-15 12:37 2014-09-15-12-37-23dev-log-events.log
> ----rwxr-x system   sdcard_rw   294015 2014-09-15 12:37 2014-09-15-12-37-23dev-log-main.log
> ----rwxr-x system   sdcard_rw   289702 2014-09-15 12:37 2014-09-15-12-37-23dev-log-radio.log
> ----rwxr-x system   sdcard_rw     3647 2014-09-15 12:37 2014-09-15-12-37-23dev-log-system.log
> ----rwxr-x system   sdcard_rw      284 2014-09-15 12:37 2014-09-15-12-37-23proc-cmdline.log
> ----rwxr-x system   sdcard_rw   131072 2014-09-15 12:37 2014-09-15-12-37-23proc-kmsg.log
> ----rwxr-x system   sdcard_rw      924 2014-09-15 12:37 2014-09-15-12-37-23proc-meminfo.log
> ----rwxr-x system   sdcard_rw       18 2014-09-15 12:37 2014-09-15-12-37-23proc-uptime.log
> ----rwxr-x system   sdcard_rw      137 2014-09-15 12:37 2014-09-15-12-37-23proc-version.log
> ----rwxr-x system   sdcard_rw     8834 2014-09-15 12:37 2014-09-15-12-37-23proc-vmallocinfo.log
> ----rwxr-x system   sdcard_rw     1458 2014-09-15 12:37 2014-09-15-12-37-23proc-vmstat.log

The path is wrong :(
I think this is due to a change in how OS.Path.join handles empty strings. Therefore, instead of getLogDirectory() returning OS.Path.join('logs', timestamp, ''), it should instead return OS.Path.join('logs', timestamp) and then use OS.Path.join on that directory and the log filename (which would have been the right thing to do in the first place, but I'm just going to disregard that for now).
No longer blocks: 1019816
Depends on: 1019816
(In reply to James Hobin from comment #1)
> I think this is due to a change in how OS.Path.join handles empty strings.
> Therefore, instead of getLogDirectory() returning OS.Path.join('logs',
> timestamp, ''), it should instead return OS.Path.join('logs', timestamp) and
> then use OS.Path.join on that directory and the log filename (which would
> have been the right thing to do in the first place, but I'm just going to
> disregard that for now).

This does indeed fix the issue.
Assignee: hobinjk → lissyx+mozillians
Keywords: dogfood
It's really sad, I spotted the issue when I did the review on bug 1019816 comment 49 :(
Attachment #8501635 - Flags: review?(anygregor)
Attachment #8501635 - Attachment is obsolete: true
Attachment #8501635 - Flags: review?(anygregor)
Attachment #8501647 - Flags: review?(anygregor)
Whiteboard: [systemsfe]
Attachment #8501647 - Flags: review?(anygregor) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/745dff11e9ec
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S7 (24Oct)
[Blocking Requested - why for this release]: New developer feature which is broken without the uplift but still exposed in the Developer menu (hiding it there is an alternative).
blocking-b2g: --- → 2.1?
(In reply to Archaeopteryx [:aryx] from comment #8)
> [Blocking Requested - why for this release]: New developer feature which is
> broken without the uplift but still exposed in the Developer menu (hiding it
> there is an alternative).

Is this feature new for 2.1? Is this a regression or not?

Alexandre, what's the level of risk?
Flags: needinfo?(lissyx+mozillians)
blocking-b2g: 2.1? → 2.1+
(In reply to Dietrich Ayala (:dietrich) from comment #9)
> (In reply to Archaeopteryx [:aryx] from comment #8)
> > [Blocking Requested - why for this release]: New developer feature which is
> > broken without the uplift but still exposed in the Developer menu (hiding it
> > there is an alternative).
> 
> Is this feature new for 2.1? Is this a regression or not?
> 
> Alexandre, what's the level of risk?

Low.
Flags: needinfo?(lissyx+mozillians)
Flags: needinfo?(jlorenzo)
Please request b2g34 approval on this when you get a chance.
Flags: needinfo?(lissyx+mozillians)
Comment on attachment 8501647 [details] [diff] [review]
Fix logshake directory computation r=gwagner

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 1019816
User impact if declined: developper/users are able to shake the device to dump intereting logs, but those are not saved at the proper location on the sdcard
Testing completed: landed on master, local testing on device
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: none
Flags: needinfo?(lissyx+mozillians)
Attachment #8501647 - Flags: approval-mozilla-b2g34?
Attachment #8501647 - Flags: approval-mozilla-b2g34? → approval-mozilla-b2g34+
Bug 1019816 landed when trunk was on Gecko 35, so this can't be uplifted to b2g34 unless that is first.
Flags: needinfo?(lissyx+mozillians)
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #14)
> Bug 1019816 landed when trunk was on Gecko 35, so this can't be uplifted to
> b2g34 unless that is first.

Damn you're right, bug 1019816 never made it to 2.1 :/. Gregor, should we uplift bug 1019816 ?
Flags: needinfo?(lissyx+mozillians) → needinfo?(anygregor)
Tested on 2.2: Now the logs appear correctly in /storage/sdcard/logs/YYYY-MM-DD-hh-mm-ss/*.log.

Some things that I have noticed:
* If the sdcard is busy, the notification is not correctly displayed (see bug 1093093).
* If a folder already exist, the logs are deleted. This might be problematic edge case if you move through timezones (see bug 1093108).
Status: RESOLVED → VERIFIED
Flags: needinfo?(jlorenzo)
(In reply to Alexandre LISSY :gerard-majax from comment #15)
> (In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #14)
> > Bug 1019816 landed when trunk was on Gecko 35, so this can't be uplifted to
> > b2g34 unless that is first.
> 
> Damn you're right, bug 1019816 never made it to 2.1 :/. Gregor, should we
> uplift bug 1019816 ?

No, we should disable this feature on 2.1.
Flags: needinfo?(anygregor)
Can somebody please wontfix this for v2.1 if that's the way you want to go?
Blocks: 1019816
No longer depends on: 1019816
Attachment #8501647 - Flags: approval-mozilla-b2g34+
You need to log in before you can comment on or make changes to this bug.