1466054 - Enable STL wrapping in gn and gyp processed code

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Bug 1466054 - Enable STL wrapping in gn and gyp processed code.

We wrap STL headers so as to ensure our inline infallible operator new
definitions are used, but for some reason, STL wrappers were disabled
in whatever code we used gyp for first, and that ended up spreading as
more things used gyp, and now with gn, whose handling cargo culted
STL disabling from the gyp code.

We remove all this disabling, because it turns out it causes no build
problem anymore.

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

Comment 2

[Mike Hommey [:glandium]]

Attachment: Bug 1466054 - Run `mach build-backend -b GnMozbuildWriter`.

This refreshes the gn-generated moz.builds with the change from previous
commit. Somehow, this does unrelated changes, there must be something
funky in the gn-moz.build-generator.

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

Comment 3

[Nathan Froyd [:froydnj]]

Comment on attachment 8982450 [details]
Bug 1466054 - Enable STL wrapping in gn and gyp processed code.

https://reviewboard.mozilla.org/r/248418/#review254760

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8982451 [details]
Bug 1466054 - Run `mach build-backend -b GnMozbuildWriter`.

https://reviewboard.mozilla.org/r/248420/#review254758

I guess the `-msse2` changes are harmless, but they seem a bit weird.  Feel free to open a followup bug on figuring out why we do that.

Please open a followup bug for a more-deterministic gn `moz.build` writer.

::: media/webrtc/trunk/moz.build:193
(Diff revision 1)
> -if CONFIG["CPU_ARCH"] == "x86_64" and CONFIG["OS_TARGET"] == "Linux":
> +if CONFIG["CPU_ARCH"] == "x86" and CONFIG["OS_TARGET"] == "Android":
>  
>      DIRS += [
>          "/media/webrtc/trunk/webrtc/common_audio/common_audio_sse2_gn",
>          "/media/webrtc/trunk/webrtc/modules/audio_processing/audio_processing_sse2_gn",
> -        "/media/webrtc/trunk/webrtc/modules/desktop_capture/desktop_capture_differ_sse2_gn",
>          "/media/webrtc/trunk/webrtc/modules/video_processing/video_processing_sse2_gn"
>      ]
>  
> -if CONFIG["CPU_ARCH"] == "x86" and CONFIG["OS_TARGET"] == "Android":
> +if CONFIG["CPU_ARCH"] == "x86_64" and CONFIG["OS_TARGET"] == "Linux":
>  
>      DIRS += [
>          "/media/webrtc/trunk/webrtc/common_audio/common_audio_sse2_gn",
>          "/media/webrtc/trunk/webrtc/modules/audio_processing/audio_processing_sse2_gn",
> +        "/media/webrtc/trunk/webrtc/modules/desktop_capture/desktop_capture_differ_sse2_gn",

Perhaps these changes are because we're iterating over some dictionary in a non-stable order in the gn backend?  We should probably fix that (maybe not here, though?).

::: media/webrtc/trunk/webrtc/common_audio/common_audio_sse2_gn/moz.build:90
(Diff revision 1)
> +        "-msse2",
>          "-msse2"

This change seems ungood.

::: media/webrtc/trunk/webrtc/modules/desktop_capture/desktop_capture_differ_sse2_gn/moz.build:69
(Diff revision 1)
> +        "-msse2",
>          "-msse2"

This change seems ungood.

::: media/webrtc/trunk/webrtc/modules/video_processing/video_processing_sse2_gn/moz.build:93
(Diff revision 1)
> +        "-msse2",
>          "-msse2"

This change seems ungood.

Comment 5

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 52468d5189549515192f6f9a19add1f6ff8ea4c4 -d 838c0ab9cbdb: rebasing 466528:52468d518954 "Bug 1466054 - Enable STL wrapping in gn and gyp processed code. r=froydnj"
rebasing 466529:f18c346fe63f "Bug 1466054 - Run `mach build-backend -b GnMozbuildWriter`. r=froydnj" (tip)
merging media/webrtc/trunk/moz.build
merging media/webrtc/trunk/webrtc/api/audio_mixer_api_gn/moz.build
merging media/webrtc/trunk/webrtc/api/call_api_gn/moz.build
merging media/webrtc/trunk/webrtc/api/transport_api_gn/moz.build
merging media/webrtc/trunk/webrtc/api/video_frame_api_gn/moz.build
merging media/webrtc/trunk/webrtc/audio/audio_gn/moz.build
merging media/webrtc/trunk/webrtc/audio/utility/audio_frame_operations_gn/moz.build
merging media/webrtc/trunk/webrtc/base/gtest_prod_gn/moz.build
merging media/webrtc/trunk/webrtc/base/rtc_base_approved_gn/moz.build
merging media/webrtc/trunk/webrtc/base/rtc_numerics_gn/moz.build
merging media/webrtc/trunk/webrtc/base/rtc_task_queue_gn/moz.build
merging media/webrtc/trunk/webrtc/call/call_gn/moz.build
merging media/webrtc/trunk/webrtc/call/call_interfaces_gn/moz.build
merging media/webrtc/trunk/webrtc/common_audio/common_audio_c_gn/moz.build
merging media/webrtc/trunk/webrtc/common_audio/common_audio_gn/moz.build
merging media/webrtc/trunk/webrtc/common_audio/common_audio_neon_c_gn/moz.build
merging media/webrtc/trunk/webrtc/common_audio/common_audio_neon_gn/moz.build
merging media/webrtc/trunk/webrtc/common_audio/common_audio_sse2_gn/moz.build
merging media/webrtc/trunk/webrtc/common_video/common_video_gn/moz.build
merging media/webrtc/trunk/webrtc/logging/rtc_event_log_api_gn/moz.build
merging media/webrtc/trunk/webrtc/logging/rtc_event_log_impl_gn/moz.build
merging media/webrtc/trunk/webrtc/media/mozilla_rtc_media_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/audio_coding_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/audio_decoder_factory_interface_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/audio_decoder_interface_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/audio_encoder_interface_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/audio_format_conversion_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/audio_format_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/audio_network_adaptor_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/builtin_audio_decoder_factory_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/cng_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/g711_c_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/g711_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/g722_c_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/g722_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/isac_c_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/isac_common_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/isac_fix_c_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/isac_fix_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/isac_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/isac_neon_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/neteq_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/pcm16b_c_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/pcm16b_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/rent_a_codec_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/webrtc_opus_c_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_coding/webrtc_opus_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_conference_mixer/audio_conference_mixer_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_device/audio_device_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_mixer/audio_frame_manipulator_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_mixer/audio_mixer_impl_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_processing/audio_processing_c_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_processing/audio_processing_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_processing/audio_processing_neon_c_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_processing/audio_processing_neon_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/audio_processing/audio_processing_sse2_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/bitrate_controller/bitrate_controller_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/congestion_controller/congestion_controller_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/desktop_capture/desktop_capture_differ_sse2_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/desktop_capture/desktop_capture_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/desktop_capture/primitives_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/media_file/media_file_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/pacing/pacing_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/rtp_rtcp/rtp_rtcp_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/utility/utility_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/video_capture/video_capture_internal_impl_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/video_capture/video_capture_module_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/video_coding/video_coding_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/video_coding/video_coding_utility_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/video_coding/webrtc_h264_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/video_coding/webrtc_i420_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/video_coding/webrtc_vp8_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/video_coding/webrtc_vp9_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/video_processing/video_processing_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/video_processing/video_processing_neon_gn/moz.build
merging media/webrtc/trunk/webrtc/modules/video_processing/video_processing_sse2_gn/moz.build
merging media/webrtc/trunk/webrtc/system_wrappers/field_trial_default_gn/moz.build
merging media/webrtc/trunk/webrtc/system_wrappers/metrics_default_gn/moz.build
merging media/webrtc/trunk/webrtc/system_wrappers/system_wrappers_gn/moz.build
merging media/webrtc/trunk/webrtc/video/video_gn/moz.build
merging media/webrtc/trunk/webrtc/video_engine/video_engine_gn/moz.build
merging media/webrtc/trunk/webrtc/voice_engine/audio_coder_gn/moz.build
merging media/webrtc/trunk/webrtc/voice_engine/file_player_gn/moz.build
merging media/webrtc/trunk/webrtc/voice_engine/file_recorder_gn/moz.build
merging media/webrtc/trunk/webrtc/voice_engine/level_indicator_gn/moz.build
merging media/webrtc/trunk/webrtc/voice_engine/voice_engine_gn/moz.build
merging media/webrtc/trunk/webrtc/webrtc_common_gn/moz.build
merging media/webrtc/trunk/webrtc/webrtc_gn/moz.build
warning: conflicts while merging media/webrtc/trunk/moz.build! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 6

[Mike Hommey [:glandium]]

Comment on attachment 8982450 [details]
Bug 1466054 - Enable STL wrapping in gn and gyp processed code.

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

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8982451 [details]
Bug 1466054 - Run `mach build-backend -b GnMozbuildWriter`.

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6e7c5695e341
Enable STL wrapping in gn and gyp processed code. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/972057926234
Run `mach build-backend -b GnMozbuildWriter`. r=froydnj

Comment 9

[Mike Hommey [:glandium]]

(In reply to Nathan Froyd [:froydnj] from comment #4)
> Perhaps these changes are because we're iterating over some dictionary in a
> non-stable order in the gn backend?  We should probably fix that (maybe not
> here, though?).

Interestingly, the code *is* sorting the conditions, and the changed output *is* actually sorted as it should be. This would suggest that the previous changes to those files were done with a version of the code before bug 1431879. O_o

Comment 10

[Mike Hommey [:glandium]]

OTOH, it looks like overall, conditions are *not* ordered as expected.

As for the extra -msse2... here's the funny thing. If I locally back out my change and run GnMozbuildWriter, it's still there.

Comment 11

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/6e7c5695e341
https://hg.mozilla.org/mozilla-central/rev/972057926234