Closed Bug 887483 Opened 6 years ago Closed 6 years ago

LIBXUL_LIBRARY implies FORCE_STATIC_LIB so only one or the other needs to be specified

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla25

People

(Reporter: tbsaunde, Assigned: tbsaunde)

Details

Attachments

(4 files, 1 obsolete file)

No description provided.
see config.mk:244 or so for LIBXUL_LIBRARY implying FORCE_STATIC_LIB.
Attachment #767991 - Flags: review?(gps)
I'm not sure this is absolutely all of the assignments that need to go for part 1 to land, but its enough to make a x86_64 linux build finish, and  it can go on its own +  will mean less to rebase.
Attachment #767992 - Flags: review?(mshal)
Comment on attachment 767992 [details] [diff] [review]
rm a bunch of useless assignments to FORCE_STATIC_LIB

It looks like you removed FORCE_STATIC_LIB from content/media/omx/mediaresourcemanager/Makefile.in and widget/xremoteclient/Makefile.in, which don't set LIBXUL_LIBRARY=1. Do we not need FORCE_STATIC_LIB in those directories for another reason or should those go back in?

Everything else looks good - nice to see a "do we still want this?" comment from 1999 go away :)
lets put them back for now, but we can probably make them be LIBXUL_LIBRARY at some point.
Attachment #768045 - Flags: review?(mshal)
Comment on attachment 768045 [details] [diff] [review]
bug 887483 - rm a bunch of useless assignments to FORCE_STATIC_LIB

Looks good to me!
Attachment #768045 - Flags: review?(mshal) → review+
Attachment #767992 - Flags: review?(mshal)
Comment on attachment 767991 [details] [diff] [review]
disallow FORCE_STATIC_LIB when LIBXUL_LIBRARY is set because its redundant

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

Good riddance.

::: config/config.mk
@@ +244,5 @@
>  ifdef MODULE_NAME
>  $(error MODULE_NAME is $(MODULE_NAME) but MODULE_NAME and LIBXUL_LIBRARY are not compatible)
>  endif
> +ifdef FORCE_STATIC_LIB
> +	$(error Makefile sets FORCE_STATIC_LIB which was already implied by LIBXUL_LIBRARY)

Please follow existing style convention and don't indent.
Attachment #767991 - Flags: review?(gps) → review+
remove the first batch of useless assignments https://hg.mozilla.org/integration/mozilla-inbound/rev/b7d6458d2a3c
It wasn't actually at fault, though now it's already bit-rotted.
Assignee: trev.saunders → mh+mozilla
Attachment #774524 - Attachment is obsolete: true
Attachment #774524 - Flags: review?(mshal)
Assignee: mh+mozilla → trev.saunders
Comment on attachment 774528 [details] [diff] [review]
Remove some more useless FORCE_STATIC_LIB and leftover comments from previous removal

fwiw lgtm
Attachment #774528 - Flags: feedback+
Comment on attachment 774528 [details] [diff] [review]
Remove some more useless FORCE_STATIC_LIB and leftover comments from previous removal

Looks good!
Attachment #774528 - Flags: review?(mshal) → review+
https://hg.mozilla.org/mozilla-central/rev/cd2355d71b74
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.