514665 - fix stupid module name differences on windows

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(not currently active) Ted Mielczarek]

For some reason some makefiles change their module names on Windows. I can only suppose these are ancient leftovers from something.

http://mxr.mozilla.org/mozilla-central/source/xpcom/proxy/public/Makefile.in#45
http://mxr.mozilla.org/mozilla-central/source/xpcom/threads/Makefile.in#46

Comment 1

[(not currently active) Ted Mielczarek]

Mitch: interested in taking this bug? I left some notes in the package manifest file (all the XXX comments):
http://mxr.mozilla.org/mozilla-central/source/browser/installer/package-manifest.in

Comment 2

[Mitchell Field [:Mitch]]

Attachment: Patch v1.0

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 401173 [details] [diff] [review]
Patch v1.0

This looks pretty good, just a few things:
--- a/browser/app/Makefile.in
+ifneq (,$(filter WINNT,$(OS_ARCH)))
+PROGRAM = $(MOZ_APP_NAME)$(BIN_SUFFIX)
+else

If you only need to test for a single OS, it's cleaner (and easier to read) to write it:
ifeq ($(OS_ARCH),WINNT)

That being said, let's just ditch the USE_SHORT_LIBNAME block up above, and make this
ifneq (,$(filter WINNT WINCE OS2,$(OS_ARCH)))

--- a/browser/installer/removed-files.in
+components/@DLL_PREFIX@docshell_base@DLL_SUFFIX@
+components/docshell_base.xpt
+components/@DLL_PREFIX@proxyObjInst@DLL_SUFFIX@
+components/proxyObjInst.xpt
+components/@DLL_PREFIX@xpcom_thread@DLL_SUFFIX@
+components/xpcom_thread.xpt

You don't need any of these lines in here. AFAIK we've never shipped these xpt files by themselves, since Windows has always shipped a single linked xpt file. We've also never shipped a non-static or libxul build, so these components would never have been installed. (We do need the brwsrcmp.dll line, of course.)

+ifneq (,$(filter OS2 WINCE,$(OS_ARCH)))

Let's drop USE_SHORT_LIBNAME for WinCE, too. Only OS/2 requires it, so just make this:
ifeq (OS2,$(OS_ARCH))

You'll have to kill the USE_SHORT_LIBNAME bit for WinCE in configure as well.

--- a/xpcom/build/Makefile.in
-ifneq (,$(filter-out WINNT WINCE,$(OS_ARCH)))
+ifneq (,$(filter WINCE,$(OS_ARCH)))
 SHORT_LIBNAME	= xpcomcor
 endif

If you do that, you can just drop the if here.

--- a/xulrunner/app/Makefile.in
ifeq ($(USE_SHORT_LIBNAME), 1)
 PROGRAM = xulrunner$(BIN_SUFFIX)
 else
-ifeq ($(OS_ARCH), BeOS)
+ifneq (,$(filter BeOS WINCE WINNT,$(OS_ARCH)))
 PROGRAM = xulrunner$(BIN_SUFFIX)
 else
 PROGRAM = xulrunner-bin$(BIN_SUFFIX)
 endif
 endif

Same thing I mentioned above, just ditch the USE_SHORT_LIBNAME block and have an explicit list of OSes in the filter.

The rest of this looks good, just needs these fixes.

Comment 4

[(not currently active) Ted Mielczarek]

This will probably have some comm-central fallout. Might also be some Fennec fallout since we're changing the SHORT_LIBNAME bits. Neither of them should be very serious.

Comment 5

[Mitchell Field [:Mitch]]

Attachment: Patch v1.1 [Checkin: Comment 11]

Comment 6

[(not currently active) Ted Mielczarek]

Comment on attachment 401421 [details] [diff] [review]
Patch v1.1
[Checkin: Comment 11]

Looks good, thanks!

Comment 7

[(not currently active) Ted Mielczarek]

Thunderbird and SeaMonkey will need fixup to build against m-c:
http://mxr.mozilla.org/comm-central/source/suite/app/Makefile.in#68
http://mxr.mozilla.org/comm-central/source/mail/app/Makefile.in#89

Should be fine to just change those checks to OS-specific checks like Mitch's patch does, that will work fine on any branch.

I'm not sure if this will actually break anything in Fennec, after looking at it again. They don't have a packaging manifest, and they don't build an app binary (they just use the XR stub).

Comment 8

[Phil Ringnalda (:philor)]

Attachment: simplest c-c patch

Not that I really expect us to be building on wince, but it's simpler for future searching to have things exactly copy-pasted when we can.

Comment 9

[Phil Ringnalda (:philor)]

Comment on attachment 401893 [details] [diff] [review]
simplest c-c patch

Where by "simplest" I mean, "d'oh, I do remember a little thing called packages-static, now that you mention it."

Comment 10

[Phil Ringnalda (:philor)]

Attachment: more c-c patch [Checkin: See comment 13]

Some extras for Tb, to get us a clean package-compare, and just fallout from this for SM since for the other stuff it needs both packages files updated and needs decisions about some of the currently unpackaged stuff.

Comment 11

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/614e3c4ba3c1

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 402383 [details] [diff] [review]
more c-c patch
[Checkin: See comment 13]

>+#ifdef MOZILLA_1_9_1_BRANCH
> bin\res\charsetData.properties
> bin\res\charsetalias.properties
>+bin\res\wincharset.properties
>+#endif

These need adding in an ifndef to the removed-files.in

>diff --git a/suite/installer/windows/packages b/suite/installer/windows/packages

suite has a unix file as well. Is that affected? (if so I think we can do that in a separate patch and fix at least some of these bustages now).

r+a=Standard8

Comment 13

[Phil Ringnalda (:philor)]

Both of suite's manifests need more, yeah, but I intentionally only fixed suite's issues that were fallout from this bug, so only Windows. Added the removed-files, and jsctypes.xpt for Tb-trunk, and pushed http://hg.mozilla.org/comm-central/rev/018708da1fbd

Comment 14

[Serge Gautherie (:sgautherie)]

Attachment: (Cv1-SM) Support m-1.9.2 too, Improve sort order [Checkin: See comment 21+23]

Comment 15

[Serge Gautherie (:sgautherie)]

(In reply to comment #13)
> Added the [...] jsctypes.xpt for Tb-trunk, and pushed
> http://hg.mozilla.org/comm-central/rev/018708da1fbd

Fwiw, jsctypes.xpt doesn't exist (anymore) in m-1.9.1/m-1.9.2/m-c: bug 518721 removed it (since then).
You may want to undo that change to TB.

Comment 16

[Serge Gautherie (:sgautherie)]

I wonder why the removed .xpt are not added to removed-files.in.
If that's not needed (anymore) then could the older .xpt be unlisted from that file?

(NB: Same for bug 521428.)

Comment 17

[Serge Gautherie (:sgautherie)]

(In reply to comment #16)
> I wonder why the removed .xpt are not added to removed-files.in.

In m-1.9.2/m-1.9.1, I mean.

Comment 18

[(not currently active) Ted Mielczarek]

(In reply to comment #16)
> I wonder why the removed .xpt are not added to removed-files.in.
> If that's not needed (anymore) then could the older .xpt be unlisted from that
> file?
> 
> (NB: Same for bug 521428.)

Because we have shipped a single browser.xpt on Windows and Linux for as long as I can remember. OS X didn't get that treatment until bug 510309 landed recently.

Comment 19

[(not currently active) Ted Mielczarek]

Also, could you take any additional work to another bug? This bug is fixed.

Comment 20

[Serge Gautherie (:sgautherie)]

(In reply to comment #18)
> we have shipped a single browser.xpt on Windows and Linux for as long
> as I can remember.

Thanks, that's just the hint/confirmation I wanted :-)

Comment 21

[Serge Gautherie (:sgautherie)]

Comment on attachment 405502 [details] [diff] [review]
(Cv1-SM) Support m-1.9.2 too, Improve sort order
[Checkin: See comment 21+23]


http://hg.mozilla.org/comm-central/rev/70007a762cab

Comment 22

[Robert Kaiser]

(In reply to comment #21)
> http://hg.mozilla.org/comm-central/rev/70007a762cab

...and one more un-approved checkin. I hope those won't happen more often. Even if I do the review, that doesn't mean it's auto-approved.

Comment 23

[Serge Gautherie (:sgautherie)]

Comment on attachment 405502 [details] [diff] [review]
(Cv1-SM) Support m-1.9.2 too, Improve sort order
[Checkin: See comment 21+23]


Checked in without moving urlformatter.xpt, which doesn't need to.

Comment 24

[Serge Gautherie (:sgautherie)]

Comment on attachment 405502 [details] [diff] [review]
(Cv1-SM) Support m-1.9.2 too, Improve sort order
[Checkin: See comment 21+23]


(See bug 521428.)
Requesting approval (by comment) after the fact:
minus it if you want me to back this out.

Comment 25

[Robert Kaiser]

(In reply to comment #24)
> Requesting approval (by comment) after the fact:
> minus it if you want me to back this out.

It's alright, a=me, just remember it next time ;-)

Comment 26

[Serge Gautherie (:sgautherie)]

(In reply to comment #15)
> Fwiw, jsctypes.xpt doesn't exist (anymore) in m-1.9.1/m-1.9.2/m-c: bug 518721
> removed it (since then).
> You may want to undo that change to TB.

I'll do that in bug 823820.