1019944 - Mach logging eats whitespace at start of line

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[Kartikaya Gupta (email:[e-mail])]

Things like layer dumps are indented using spaces. If I run reftest-ipc on linux via mach with layer dumping turned on, all the indentation is gone. If I run it with make -C obj-host/ the indentation is there. Something is in mach is eating the indentation and it makes the output much harder to read.

Specific STR:
EXTRA_TEST_ARGS="--setpref=layers.dump=true --setpref=layers.async-pan-zoom.enabled=true" ./mach reftest-ipc layout/reftests/reftest-sanity/

This will output layer dumps as it runs; they should have proper indentation.

Comment 1

[Markus Stange [:mstange]]

Attachment: remove the offending .strip()

This was introduced as part of the original mach landing in http://hg.mozilla.org/mozilla-central/rev/1ae2d42ad234

Comment 2

[Gregory Szorc [:gps]]

Comment on attachment 8439177 [details] [diff] [review]
remove the offending .strip()

Review of attachment 8439177 [details] [diff] [review]:
-----------------------------------------------------------------

Something in the back of my head says I added the strip here for a reason. But I can't remember what.

If I had to venture, I'd say it was trailing whitespace, possibly \r. Possibly a bug with old versions of mozprocess.

I hate cargo culting. But can you change this to .rstrip() just to be safe? I can't think of any scenarios where we'd care about trailing whitespace. Can you?

Comment 3

[Matt Woodrow (:mattwoodrow)]

Can we land this Markus?

Comment 4

[Markus Stange [:mstange]]

(In reply to Gregory Szorc [:gps] from comment #2)
> I hate cargo culting. But can you change this to .rstrip() just to be safe?

Sure.

> I can't think of any scenarios where we'd care about trailing whitespace.
> Can you?

I can't, but I'm not very familiar with the details here. All I did to test the patch was to run mach reftest on Mac - the patch didn't cause double-newlines, so I assumed it'd be fine.

(In reply to Matt Woodrow (:mattwoodrow) from comment #3)
> Can we land this Markus?

I'll land the .rstrip() version in a few minutes.

Comment 5

[Markus Stange [:mstange]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4ea8a9d24c23

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4ea8a9d24c23