543382 - Port |Bug 467862 - Build system should support building both a static and a shared library from the same Makefile| to comm-central

Status: RESOLVED FIXED

Description

[Serge Gautherie (:sgautherie)]

Fwiw, the only current explicit use is
{
/mozilla/js/src/Makefile.in
    * line 88 -- STATIC_LIBRARY_NAME = js_static
}

Comment 1

[Robert Kaiser]

Right, but if I read the original bug correctly, this should let us run xpcshell against a static binary - if so, Standard8 should be interested, I guess ;-)

Comment 2

[Justin Wood (:Callek)]

Yes, from what I've read and seen on this; it is worth porting; even if c-c has no consumers of its use yet.  We could likely want some in the future.

Comment 3

[Mark Banner (:standard8)]

We don't need a future milestone on this. It can be ported at any time. IMO a future milestone is only really necessary for things we really don't intend to do at this time because we're not ready.

Comment 4

[Serge Gautherie (:sgautherie)]

(In reply to comment #3)

> We don't need a future milestone on this. It can be ported at any time. IMO a

Agreed. It was not meant to "require" a new milestone, but just that we have no need to port this until it's eventually needed.
(Just like setting it to P5, but I'm not using 'Importance'.)

> future milestone is only really necessary for things we really don't intend to
> do at this time because we're not ready.

Afaik, I (and now Callek) am the only developer working on bug 506493, so I'm sad you can't just let me(/us) do it my/our way :-<

That said, as it's not Future anymore, I'd love if you could assign someone to this bug...

Comment 5

[Justin Wood (:Callek)]

(In reply to comment #4)
> (In reply to comment #3)
> > future milestone is only really necessary for things we really don't intend to
> > do at this time because we're not ready.
> 
> Afaik, I (and now Callek) am the only developer working on bug 506493, so I'm
> sad you can't just let me(/us) do it my/our way :-<

I was not for or against setting the "future" milestone, I usually leave those things default unless there is a *reason* to set them. in this case there was no reason so I agree with mark's statements. Please don't put words in my mouth.

> That said, as it's not Future anymore, I'd love if you could assign someone to
> this bug...

That said, I don't see a need to "force" someone to take this bug, its not a blocker. Most of the c-c build system team is volunteer. And I will likely work on this before next release of TB or SM (if no-one beats me to it). I do not wish to be assigned until I *start* work on bugs though, so the current state of assignee, flags, etc. are correct.

Comment 6

[Serge Gautherie (:sgautherie)]

(In reply to comment #5)

> Please don't put words in my mouth.

I was not trying to put words in your mouth: my comment was more a "why should anyone else but me cares?".

> That said, I don't see a need to "force" someone to take this bug, its not a
> blocker.

Well, it looked like to me that Mark seemed to care that this bug gets resolved sooner than later...

Comment 7

[Justin Wood (:Callek)]

Attachment: v1 -- Port bug and its bustage fix.

Comment 8

[Serge Gautherie (:sgautherie)]

Comment on attachment 430800 [details] [diff] [review]
v1 -- Port bug and its bustage fix.


Bug 534408 should make this port simpler. (I didn't read further...)
Any reason not to wait for it?

Comment 9

[Justin Wood (:Callek)]

(In reply to comment #8)
> (From update of attachment 430800 [details] [diff] [review])
> 
> Bug 534408 should make this port simpler. (I didn't read further...)
> Any reason not to wait for it?

If that bug is *also* porting this, no reason not to wait. If it is NOT porting this, I'd rather get this in and do more in "other ports".

Comment 10

[Serge Gautherie (:sgautherie)]

(In reply to comment #9)

> If that bug is *also* porting this, no reason not to wait. If it is NOT porting

I don't know yet, but I assume that bug won't port this one for free.

> this, I'd rather get this in and do more in "other ports".

What's the hurry? Porting in the wrong order just makes things more complex: more porting, more cleanup. (Though you're not the only one to do that :-/)
Besides, I'm concerned that Standard8 spends time on things like this (which afaik we don't need yet) instead of on my blocking ports (like debug packaged tests bustage bugs)...

Comment 11

[Robert Kaiser]

Comment on attachment 430800 [details] [diff] [review]
v1 -- Port bug and its bustage fix.

>@@ -840,9 +834,9 @@ ifdef EXPORT_LIBRARY
> ifdef IS_COMPONENT
> ifdef BUILD_STATIC_LIBS
> ifdef MOZILLA_1_9_2_BRANCH
>-	@$(PERL) -I$(MOZILLA_DIR)/config $(MOZILLA_DIR)/config/build-list.pl $(FINAL_LINK_COMPS) $(LIBRARY_NAME)
>+    @$(PERL) -I$(MOZILLA_DIR)/config $(MOZILLA_DIR)/config/build-list.pl $(FINAL_LINK_COMPS) $(STATIC_LIBRARY_NAME)
> else
>-	@$(PYTHON) $(MOZILLA_DIR)/config/buildlist.py $(FINAL_LINK_COMPS) $(LIBRARY_NAME)
>+	@$(PYTHON) $(MOZILLA_DIR)/config/buildlist.py $(FINAL_LINK_COMPS) $(STATIC_LIBRARY_NAME)
> endif
> ifdef MODULE_NAME
> ifdef MOZILLA_1_9_2_BRANCH

nit: don't change the indentation in the |ifdef MOZILLA_1_9_2_BRANCH|, kepp the tab intact.

Comment 12

[Justin Wood (:Callek)]

(In reply to comment #11)
> nit: don't change the indentation in the |ifdef MOZILLA_1_9_2_BRANCH|, kepp the
> tab intact.

Gyah that was actually my local editor screwing up, I'll fix by checkin. Just waiting for Mark now.

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 430800 [details] [diff] [review]
v1 -- Port bug and its bustage fix.

Looks reasonable, although I've not tried it out.

Comment 14

[Justin Wood (:Callek)]

(In reply to comment #12)
> (In reply to comment #11)
> > nit: don't change the indentation in the |ifdef MOZILLA_1_9_2_BRANCH|, kepp the
> > tab intact.
> 
> Gyah that was actually my local editor screwing up, I'll fix by checkin. Just
> waiting for Mark now.

...and actually this shouldn't even have been a nit, it is a breaking change as this is a rule's command! :/ (I failed tab-conversion in two spots actually)

Anyway...
Pushed as: http://hg.mozilla.org/comm-central/rev/9979fbcff2c9