904831 - Move MSVC_ENABLE_PGO into moz.build

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

MSVC_ENABLE_PGO is currently the 8th most frequently used variable yet to be ported to moz.build files.

Comment 1

[(not currently active) Ted Mielczarek]

I'm sure it's trivial to convert, but note that I added those via a script:
http://hg.mozilla.org/users/tmielczarek_mozilla.com/pgo-datamangling/file/b995971b79d5/pgooptin.py

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

And since it's checked in config.mk, you'll need to make sure config.mk isn't included before rules.mk in the makefile, right?

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part a: logic

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part b: automated moves

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part c: manual moves

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part d: disallow in makefiles

Comment 7

[Joey Armstrong [:joey]]

Comment on attachment 792100 [details] [diff] [review]
Part b: automated moves

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

Patch will not import cleanly

% hg qimport ~/MSVC_ENABLE_PGO-auto 
adding MSVC_ENABLE_PGO-auto to series file

% hg qpush -a
applying MSVC_ENABLE_PGO-auto
patching file caps/src/Makefile.in
Hunk #1 FAILED at 4
1 out of 1 hunks FAILED -- saving rejects to file caps/src/Makefile.in.rej
patching file caps/src/moz.build
Hunk #1 FAILED at 14
1 out of 1 hunks FAILED -- saving rejects to file caps/src/moz.build.rej
patching file chrome/src/Makefile.in
Hunk #1 FAILED at 5
1 out of 1 hunks FAILED -- saving rejects to file chrome/src/Makefile.in.rej
patching file chrome/src/moz.build
Hunk #1 FAILED at 13
1 out of 1 hunks FAILED -- saving rejects to file chrome/src/moz.build.rej

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 792100 [details] [diff] [review]
Part b: automated moves

Well, yes, there's 30 patches below it. You can get them from try: https://tbpl.mozilla.org/?tree=Try&rev=95e9f75cd8e6

Comment 9

[Joey Armstrong [:joey]]

(In reply to :Ms2ger from comment #8)
> Comment on attachment 792100 [details] [diff] [review]
> Part b: automated moves
> 
> Well, yes, there's 30 patches below it. You can get them from try:
> https://tbpl.mozilla.org/?tree=Try&rev=95e9f75cd8e6

If patches do not apply cleanly that will inhibit being able to run command line scripts to screen for problems.  Also the request was a single patch not 30, applying them all will make it more difficult to review just the requested patch content.

Comment 10

[Michael Shal [:mshal]]

Comment on attachment 792101 [details] [diff] [review]
Part c: manual moves

>diff --git a/netwerk/dns/Makefile.in b/netwerk/dns/Makefile.in
>--- a/netwerk/dns/Makefile.in
>+++ b/netwerk/dns/Makefile.in
>@@ -5,16 +5,17 @@
> 
> DEPTH     = @DEPTH@
> topsrcdir = @top_srcdir@
> srcdir    = @srcdir@
> VPATH     = @srcdir@
> 
> include $(DEPTH)/config/autoconf.mk
> 
>+# need to include etld_data.inc

Would be nice if mozbuild-migrate would get the comments above the variable rather than below it, but I don't see an easy way to do that with the pymake structures :/. Maybe I'm missing something...

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 792100 [details] [diff] [review]
Part b: automated moves

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

Didn't see any obvious issues with this automated migration.

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/059b301f9da2
https://hg.mozilla.org/mozilla-central/rev/34447d205d73
https://hg.mozilla.org/mozilla-central/rev/09ddb8d890c9
https://hg.mozilla.org/mozilla-central/rev/4a66e5d6386b