Closed Bug 914270 Opened 11 years ago Closed 11 years ago

move FORCE_STATIC_LIB to mozbuild

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
mozilla27

People

(Reporter: joey, Assigned: Cykesiopka)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 13 obsolete files)

Part 1: Simple/Automated moves v6 for m-i
67.15 KB, patch
Details | Diff | Splinter Review
Part 2: Manual moves v6 for m-i
8.64 KB, patch
Details | Diff | Splinter Review
Part 3: Disallow in Makefiles v4 for m-i
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.

Attachment

General

Creator:
Created:
Updated:
Size: