Closed Bug 914270 Opened 6 years ago Closed 6 years ago

move FORCE_STATIC_LIB to mozbuild

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla27

People

(Reporter: joey, Assigned: Cykesiopka)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 13 obsolete files)

67.15 KB, patch
Details | Diff | Splinter Review
8.64 KB, patch
Details | Diff | Splinter Review
1.12 KB, patch
Details | Diff | Splinter Review
% find . -name 'Makefile.in' | xargs grep FORCE_STATIC
./gfx/ots/src/Makefile.in:FORCE_STATIC_LIB = 1
./gfx/harfbuzz/src/Makefile.in:FORCE_STATIC_LIB = 1
./gfx/graphite2/src/Makefile.in:FORCE_STATIC_LIB = 1
./js/src/Makefile.in:FORCE_STATIC_LIB = 1
./js/src/editline/Makefile.in:FORCE_STATIC_LIB = 1
./widget/gonk/libdisplay/Makefile.in:FORCE_STATIC_LIB= 1
./widget/xremoteclient/Makefile.in:FORCE_STATIC_LIB = 1
./widget/qt/faststartupqt/Makefile.in:FORCE_STATIC_LIB = 1
./modules/libbz2/src/Makefile.in:FORCE_STATIC_LIB= 1
./modules/libmar/sign/Makefile.in:FORCE_STATIC_LIB = 1
./modules/libmar/verify/Makefile.in:FORCE_STATIC_LIB = 1
./modules/libmar/src/Makefile.in:FORCE_STATIC_LIB = 1
./mozglue/android/Makefile.in:FORCE_STATIC_LIB = 1
./mozglue/linker/Makefile.in:FORCE_STATIC_LIB= 1
./mozglue/build/Makefile.in:FORCE_STATIC_LIB = 1
./db/sqlite3/src/Makefile.in:FORCE_STATIC_LIB = 1
./mfbt/Makefile.in:FORCE_STATIC_LIB = 1
./xpcom/glue/standalone/Makefile.in:FORCE_STATIC_LIB = 1
./xpcom/glue/standalone/staticruntime/Makefile.in:FORCE_STATIC_LIB = 1
./xpcom/glue/nomozalloc/Makefile.in:FORCE_STATIC_LIB = 1
./xpcom/glue/Makefile.in:FORCE_STATIC_LIB = 1
./xpcom/glue/staticruntime/Makefile.in:FORCE_STATIC_LIB = 1
./xpcom/reflect/xptinfo/src/Makefile.in:FORCE_STATIC_LIB = 1
./xpcom/reflect/xptcall/src/Makefile.in:FORCE_STATIC_LIB = 1
./xpcom/reflect/xptcall/src/md/unix/Makefile.in:FORCE_STATIC_LIB = 1
./xpcom/reflect/xptcall/src/md/win32/Makefile.in:FORCE_STATIC_LIB = 1
./xpcom/reflect/xptcall/src/md/os2/Makefile.in:FORCE_STATIC_LIB = 1
./xpcom/string/src/Makefile.in:FORCE_STATIC_LIB = 1
./profile/dirserviceprovider/standalone/Makefile.in:FORCE_STATIC_LIB = 1
./profile/dirserviceprovider/src/Makefile.in:FORCE_STATIC_LIB = 1
./memory/mozalloc/Makefile.in:FORCE_STATIC_LIB= 1
./memory/mozjemalloc/Makefile.in:FORCE_STATIC_LIB= 1
./memory/build/Makefile.in:FORCE_STATIC_LIB = 1
./memory/jemalloc/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/crashreporter/breakpad-windows-libxul/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/crashreporter/google-breakpad/src/processor/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/crashreporter/google-breakpad/src/client/linux/minidump_writer/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/crashreporter/google-breakpad/src/client/linux/crash_generation/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/crashreporter/google-breakpad/src/client/linux/handler/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/crashreporter/google-breakpad/src/client/solaris/handler/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/crashreporter/google-breakpad/src/client/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/crashreporter/google-breakpad/src/client/mac/crash_generation/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/crashreporter/google-breakpad/src/client/mac/handler/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/crashreporter/google-breakpad/src/common/dwarf/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/crashreporter/google-breakpad/src/common/linux/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/crashreporter/google-breakpad/src/common/solaris/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/crashreporter/google-breakpad/src/common/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/crashreporter/google-breakpad/src/common/mac/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/crashreporter/breakpad-windows-standalone/Makefile.in:FORCE_STATIC_LIB = 1
./toolkit/components/protobuf/Makefile.in:FORCE_STATIC_LIB = 1
./rdf/util/src/internal/Makefile.in:FORCE_STATIC_LIB = 1
./rdf/util/src/Makefile.in:FORCE_STATIC_LIB = 1
./intl/unicharutil/util/internal/Makefile.in:FORCE_STATIC_LIB = 1
./intl/unicharutil/util/Makefile.in:FORCE_STATIC_LIB = 1
./parser/expat/lib/Makefile.in:FORCE_STATIC_LIB = 1
./other-licenses/android/Makefile.in:FORCE_STATIC_LIB = 1
./media/mtransport/standalone/Makefile.in:FORCE_STATIC_LIB= 1
./media/libjpeg/Makefile.in:FORCE_STATIC_LIB = 1
./media/libogg/src/Makefile.in:FORCE_STATIC_LIB= 1
./media/libtremor/lib/Makefile.in:FORCE_STATIC_LIB= 1
./media/libopus/Makefile.in:FORCE_STATIC_LIB= 1
./media/libvorbis/lib/Makefile.in:FORCE_STATIC_LIB= 1
./media/libspeex_resampler/src/Makefile.in:FORCE_STATIC_LIB = 1
./media/libnestegg/src/Makefile.in:FORCE_STATIC_LIB= 1
./media/libpng/Makefile.in:FORCE_STATIC_LIB= 1
./media/libpng/Makefile.in:FORCE_STATIC_LIB = 1
./media/libcubeb/src/Makefile.in:FORCE_STATIC_LIB= 1
./media/libtheora/lib/Makefile.in:FORCE_STATIC_LIB = 1
./media/libvpx/Makefile.in:FORCE_STATIC_LIB= 1
./build/unix/stdc++compat/Makefile.in:FORCE_STATIC_LIB= 1
./build/stlport/Makefile.in:FORCE_STATIC_LIB = 1
./content/media/omx/mediaresourcemanager/Makefile.in:FORCE_STATIC_LIB = 1
./browser/components/dirprovider/Makefile.in:FORCE_STATIC_LIB = 1
./browser/components/about/Makefile.in:FORCE_STATIC_LIB = 1
./browser/components/shell/src/Makefile.in:FORCE_STATIC_LIB = 1
./browser/components/feeds/src/Makefile.in:FORCE_STATIC_LIB = 1
./browser/components/migration/src/Makefile.in:FORCE_STATIC_LIB = 1
Blocks: nomakefiles
passthrough variable already added, Makefile.in still needs to be converted.
See Also: → 912908
Attachment #809682 - Flags: review?(joey)
Attached patch Part 2: Manual moves v1 (obsolete) — Splinter Review
Attachment #809683 - Flags: review?(joey)
Attached patch Part 3: Disallow in Makefiles v1 (obsolete) — Splinter Review
Attachment #809685 - Flags: review?(joey)
Comment on attachment 809682 [details] [diff] [review]
Part 1: Simple/Automated moves v1

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

browser/components/about/moz.build
browser/components/dirprovider/moz.build
browser/components/feeds/src/moz.build
browser/components/migration/src/moz.build
build/stlport/moz.build
intl/unicharutil/util/internal/moz.build
js/src/editline/moz.build
media/libcubeb/src/moz.build
[...]
==================================
  > FORCE_STATIC_LIB = True
  >

nit: several files have trailing blank lines
Attachment #809682 - Flags: review?(joey) → review+
Comment on attachment 809683 [details] [diff] [review]
Part 2: Manual moves v1

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

gfx/graphite2/src/moz.build
gfx/harfbuzz/src/moz.build
gfx/ots/src/moz.build
[...]
=================
suggestion: reversing the conditional would make the WINNT-ness of the blocks bit more explicit.

if CONFIG['OS_TARGET'] == 'WINNT':
    FORCE_STATIC_LIB = True
else:
    LIBXUL_LIBRARY = True

Also the statement placeholder 'pass' can be removed since FORCE_STATIC has been uncommented.


Are there any try results available for the patch(es) ?
Attachment #809683 - Flags: review?(joey) → feedback?(joey)
Attachment #809683 - Flags: feedback?(joey) → feedback+
Whiteboard: [leaveopen]
Comment on attachment 809685 [details] [diff] [review]
Part 3: Disallow in Makefiles v1

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

looks good, the [leaveopen] whiteboard tag on the bug can be removed when this patch is submitted after the other conversion patches have landed.
Attachment #809685 - Flags: review?(joey) → review+
+ Removed trailing blank lines

Carrying forward r+.
Assignee: nobody → cykesiopka.bmo
Attachment #809682 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attached patch Part 2: Manual moves v2 (obsolete) — Splinter Review
+ Reverse the WINNT conditional blocks
+ Remove instances of |pass|
Attachment #809683 - Attachment is obsolete: true
Attachment #809949 - Flags: review?(joey)
(In reply to Joey Armstrong [:joey] from comment #6)
> Are there any try results available for the patch(es) ?

No, unfortunately. I don't have access to the try server.
(In reply to Cykesiopka from comment #10)
> (In reply to Joey Armstrong [:joey] from comment #6)
> > Are there any try results available for the patch(es) ?
> 
> No, unfortunately. I don't have access to the try server.

Oh if you do not have access [yet] feel free to ask for someone to submit a try job on your behalf.  In channel, from whomever is doing your code review, etc.  Usually not a problem as long as the patch applies cleanly and esp good to sanity check conversion patches.  Just note what platforms/test coverage is needed for the patches.

patch 1: [build & test] https://tbpl.mozilla.org/?tree=Try&rev=23c815b51217
patch 2: [build-only] https://tbpl.mozilla.org/?tree=Try&rev=e1a1ebe8e082
Comment on attachment 809949 [details] [diff] [review]
Part 2: Manual moves v2

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

Looks good, minor nit for extra blank lines.

db/sqlite3/src/moz.build
========================
+if CONFIG['MOZ_FOLD_LIBS']:
+    FORCE_STATIC_LIB = True
+

memory/mozalloc/moz.build
=========================
+if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'gonk':
+    FORCE_STATIC_LIB = True
+
Attachment #809949 - Flags: review?(joey) → review+
Attached patch Part 2: Manual moves v3 (obsolete) — Splinter Review
+ Remove extra blank lines

Carrying forward r+.
Attachment #809949 - Attachment is obsolete: true
(In reply to Joey Armstrong [:joey] from comment #11)
> Oh if you do not have access [yet] feel free to ask for someone to submit a
> try job on your behalf.  In channel, from whomever is doing your code
> review, etc.  Usually not a problem as long as the patch applies cleanly and
> esp good to sanity check conversion patches.  Just note what platforms/test
> coverage is needed for the patches.

OK, thanks.

> patch 1: [build & test] https://tbpl.mozilla.org/?tree=Try&rev=23c815b51217
> patch 2: [build-only] https://tbpl.mozilla.org/?tree=Try&rev=e1a1ebe8e082

All the oranges etc are intermittent I guess?
Flags: needinfo?(joey)
Unbitrotted.
Attachment #809947 - Attachment is obsolete: true
Attached patch Part 2: Manual moves v4 (obsolete) — Splinter Review
Unbitrotted.
Attachment #811210 - Attachment is obsolete: true
responded but never cleared the needinfo flag.
Flags: needinfo?(joey)
Are the previous try push oranges all intermittent?

Should I get a new try push done now that so much time has passed?

(Sorry, I couldn't find the answer anywhere...)
Flags: needinfo?(joey)
Sorry for not responding sooner.  Since you are not changing underlying logic and the faiures are orange/known/intermittent your patch will be ok to push.
Flags: needinfo?(joey)
(In reply to Joey Armstrong [:joey] from comment #19)
> Sorry for not responding sooner.  Since you are not changing underlying
> logic and the faiures are orange/known/intermittent your patch will be ok to
> push.

OK, thanks!
Unbitrotted.
Attachment #815730 - Attachment is obsolete: true
Attached patch Part 3: Disallow in Makefiles v2 (obsolete) — Splinter Review
Updated for post Bug 900526, which moved _MOZBUILD_EXTERNAL_VARIABLES to config.mk

This updated patch still works, and the changes seem really trivial, so carrying forward r+. Please correct otherwise.
Attachment #809685 - Attachment is obsolete: true
Keywords: checkin-needed
Sorry doesn't apply cleanly against inbound, please can you rebase? :-)
Keywords: checkin-needed
Whiteboard: [leaveopen]
Against 17d130494887 on m-i.
Attachment #821440 - Attachment is obsolete: true
Attached patch Part 2: Manual moves v5 for m-i (obsolete) — Splinter Review
Against 17d130494887 on m-i.
Attachment #815731 - Attachment is obsolete: true
Against 17d130494887 on m-i.
Attachment #821442 - Attachment is obsolete: true
(In reply to Ed Morley [:edmorley UTC+1] from comment #23)
> Sorry doesn't apply cleanly against inbound, please can you rebase? :-)

Done. Hope it applies cleanly this time...
Keywords: checkin-needed
Rebased on a70124509785 of m-i.
Attachment #821750 - Attachment is obsolete: true
Attachment #821751 - Attachment is obsolete: true
Attachment #821752 - Attachment is obsolete: true
(In reply to Cykesiopka from comment #27)
> Done. Hope it applies cleanly this time...

It does, thank you :-)
Followup to touch the CLOBBER file, since this was burning otherwise:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b35cd52a1f40

Joey, until bug 852814 is fixed, moving variables to moz.build requires a clobber - so can you ask people to tocuh the CLOBBER file as part of these transition bugs please? (Note: try server is always-clobber so won't pick this up)
Flags: needinfo?(joey)
ok
Flags: needinfo?(joey)
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.