1382511 - Consider adding --output-sync=line to our make invocation

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Chris Manchester (limited bugmail, email directly)]

I was using a script to scrape build logs and compare command lines and found a lot of uninteresting differences between builds due to interleaved output between make processes. Perhaps a little surprisingly, adding `MAKEFLAGS += --output-sync=line` fixes these differences. It may be worth adding this in general.

Comment 1

[Gregory Szorc [:gps]]

See previous discussion around --output-sync in bug 1285254.

Comment 2

[Chris Manchester (limited bugmail, email directly)]

(In reply to Gregory Szorc [:gps] from comment #1)
> See previous discussion around --output-sync in bug 1285254.

The issue there seems to be interleaving between multiple make invocations, which I don't think this would solve. This appears to address interleaving between worker processes spawned by make.

Comment 3

[Mike Hommey [:glandium]]

Actually, --output-sync=line might fix both issues.

Comment 4

[Chris Manchester (limited bugmail, email directly)]

Ok, well, let's see.

Comment 5

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug 1382511 - Add --output-sync=line to MOZ_MAKE_FLAGS where available.

Review commit: https://reviewboard.mozilla.org/r/159432/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/159432/

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8888480 [details]
Bug 1382511 - Add --output-sync=line to MOZ_MAKE_FLAGS where available.

https://reviewboard.mozilla.org/r/159432/#review165220

Consider make 4.2 is out, at some point we'll probably want to add make version sniffing to configure and set a config variable for whether --output-sync is supported.

I'm not telling you to scope bloat this patch. But it would be a nice addition!

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8888480 [details]
Bug 1382511 - Add --output-sync=line to MOZ_MAKE_FLAGS where available.

https://reviewboard.mozilla.org/r/159432/#review166082

::: client.mk:78
(Diff revision 1)
>  ifdef CONFIG_GUESS_SCRIPT
>    CONFIG_GUESS := $(shell $(CONFIG_GUESS_SCRIPT))
>  endif
>  
> +ifeq (4.0,$(firstword $(sort 4.0 $(MAKE_VERSION))))
> +MAKEFLAGS += --output-sync=line

Should probably be MOZ_MAKE_FLAGS

Comment 8

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8888480 [details]
Bug 1382511 - Add --output-sync=line to MOZ_MAKE_FLAGS where available.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/159432/diff/1-2/

Comment 9

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8888480 [details]
Bug 1382511 - Add --output-sync=line to MOZ_MAKE_FLAGS where available.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/159432/diff/2-3/

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8888480 [details]
Bug 1382511 - Add --output-sync=line to MOZ_MAKE_FLAGS where available.

https://reviewboard.mozilla.org/r/159432/#review168652

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ae6577d57698
Add --output-sync=line to MOZ_MAKE_FLAGS where available. r=glandium

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

as a note the compiler warnings are increased with this change:
== Change summary for alert #8470 (as of August 01 2017 18:22 UTC) ==

Regressions:

  1%  compiler warnings summary linux64 asan asan debug     11,657.33 -> 11,809.00
  1%  compiler warnings summary linux64 asan asan opt       9,491.58 -> 9,561.08

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8470

Comment 13

[Chris Manchester (limited bugmail, email directly)]

(In reply to Joel Maher ( :jmaher) (UTC-9) from comment #12)
> as a note the compiler warnings are increased with this change:
> == Change summary for alert #8470 (as of August 01 2017 18:22 UTC) ==
> 
> Regressions:
> 
>   1%  compiler warnings summary linux64 asan asan debug     11,657.33 ->
> 11,809.00
>   1%  compiler warnings summary linux64 asan asan opt       9,491.58 ->
> 9,561.08
> 
> For up to date results, see:
> https://treeherder.mozilla.org/perf.html#/alerts?id=8470

The warnings database checks each line of build output against a regex, so I guess we were missing some before due to interleaving.

Comment 14

[Gregory Szorc [:gps]]

Yes, historically the compiler warnings parser has been sensitive to interleaving.

I can't remember if it attempts to parse multiple lines, but some compiler warning output spans multiple lines and interleaved output that isn't synchronized at the process level (in terms of a make recipe's view of a process), then things get weird.

Comment 15

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

https://hg.mozilla.org/mozilla-central/rev/ae6577d57698

Comment 16

[Aki Sasaki (not active)]

https://hg.mozilla.org/projects/date/rev/ae6577d5769856cc5c899d42cf3b52d4dd1fef55
Bug 1382511 - Add --output-sync=line to MOZ_MAKE_FLAGS where available. r=glandium