Closed Bug 855675 Opened 7 years ago Closed 7 years ago

Disable libraries folding on mingw

Categories

(Firefox Build System :: General, defect)

x86
Windows 7
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla22

People

(Reporter: jacek, Assigned: jacek)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch fix (obsolete) — Splinter Review
After bug 648407 landed, mingw compilation is broken. There are a few problems that would have to be addressed to fix this. Two main problems are that the code gets library names wrong in many places and it expects linker to be able to fixup broken __declspec(dllexport/dllimport) on its own. While I could get the first part to mostly work, second one is more tricky as it requires binutils changes. That's why I decided to disable libraries folding for now, to have more time to prepare toolchain to cope with this mess.

The attached patch:

- Unsets MOZ_FOLD_LIBS for mingw in configure.in
- Chabges NSS_DEP_LIBS to be what is currently expected by the rest of build system (it shouldn't change anything on non-mingw targets)
- Enabled .def and .rc files in sqlite3 module. I don't know why it was ifdefed in the first place. It's not really needed for this bug, but that's something I found while trying to fix library folding.

Once this bug lands, I will file a followup to reenable folding on mingw.
Attachment #730635 - Flags: review?(mh+mozilla)
I forgot to mention try push:

https://tbpl.mozilla.org/?tree=Try&rev=d0f0ea87bac6

It's green so far, Windows builds still running.
Comment on attachment 730635 [details] [diff] [review]
fix

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

::: configure.in
@@ +3999,5 @@
>          \$(LIBXUL_DIST)/lib/\$(LIB_PREFIX)crmf.\$(LIB_SUFFIX) \
> +        \$(LIBXUL_DIST)/lib/\$(LIB_PREFIX)smime$NSS_VERSION${DLL_LINK_SUFFIX} \
> +        \$(LIBXUL_DIST)/lib/\$(LIB_PREFIX)ssl$NSS_VERSION${DLL_LINK_SUFFIX} \
> +        \$(LIBXUL_DIST)/lib/\$(LIB_PREFIX)nss$NSS_VERSION${DLL_LINK_SUFFIX} \
> +        \$(LIBXUL_DIST)/lib/\$(LIB_PREFIX)nssutil$NSS_VERSION${DLL_LINK_SUFFIX}"

What is this trying to fix?
Attachment #730635 - Flags: review?(mh+mozilla)
Flags: needinfo?(jacek)
(In reply to Mike Hommey [:glandium] from comment #2)
> Comment on attachment 730635 [details] [diff] [review]
> fix
> 
> Review of attachment 730635 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: configure.in
> @@ +3999,5 @@
> >          \$(LIBXUL_DIST)/lib/\$(LIB_PREFIX)crmf.\$(LIB_SUFFIX) \
> > +        \$(LIBXUL_DIST)/lib/\$(LIB_PREFIX)smime$NSS_VERSION${DLL_LINK_SUFFIX} \
> > +        \$(LIBXUL_DIST)/lib/\$(LIB_PREFIX)ssl$NSS_VERSION${DLL_LINK_SUFFIX} \
> > +        \$(LIBXUL_DIST)/lib/\$(LIB_PREFIX)nss$NSS_VERSION${DLL_LINK_SUFFIX} \
> > +        \$(LIBXUL_DIST)/lib/\$(LIB_PREFIX)nssutil$NSS_VERSION${DLL_LINK_SUFFIX}"
> 
> What is this trying to fix?

Those libraries' import libraries are using libX.a variant for their names currently. This changes NSS_DEP_LIBS to match that so that security/manager/ssl/src/ can find its dependences.

Note that DLL_PREFIX is empty on Windows builds and is supposed to be used for DLL names, not libs/importlibs. DLL_PREFIX is the same as LIB_PREFIX on each platform except mingw, so this change should be safe.
Flags: needinfo?(jacek)
You know what? just remove NSS_DEP_LIBS, it's useless.
Attached patch fixSplinter Review
OK, here is a patch removing NSS_DEP_LIBS. Tested on clobbered mingw build.
Attachment #730635 - Attachment is obsolete: true
Attachment #731117 - Flags: review?(mh+mozilla)
Attachment #731117 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/f8a5a2a8f02d
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Blocks: 856404
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.