Bug 855675 disabled libraries folding on mingw targets as a short term fix to get mingw builds working in the tree. Long term, however, we want to enable filding on mingw. This will most likely require binutils changes as well as some fixes to m-c build system.

I worked on this and it seems very tricky to support directly in binutils. However, I found a workaround for that by using -mnop-fun-dllimport compiler flag. This is not perfect (see bug 1151447), but at least the known problem won't exist once NSS and NSPR live in the same DLL.

This is build system part. Sadly, it's not enough because it reveals some problems in NSPR.

This is similar to bug 1060401. This time it's about static library files that need lib prefix on mingw.

The problem that we need to solve is: we pass -mnop-fun-dllimport, which is valid only on mingw target, but it gets populated to HOST_CFLAGS, where it's invalid. I could work around it in top level configure (eg. by passing HOST_FLAGS=" ", which prevents defaulting HOST_CFLAGS). Gonk already has hackish solution inside NSPR configure, which does just that. IMO we should simply avoid assigning CFLAGS to HOST_CFLAGS for all platforms when cross compiling. It's not expected that setting target flags may break host parts.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e97da7c92a55

Comment on attachment 8592241 [details] [diff] [review] NSPR: Use correct library prefix on mingw This part is broken on MSVC for some reason. Here is green try push without that part: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e7dfce2d77c2

Comment on attachment 8592245 [details] [diff] [review] NSPR: configure part Review of attachment 8592245 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, that seems reasonable. I don't know why anyone would want their host CFLAGS to default to their target CFLAGS anyway.

Comment on attachment 8592232 [details] [diff] [review] build system part Review of attachment 8592232 [details] [diff] [review]: ----------------------------------------------------------------- ::: config/external/nss/Makefile.in @@ +263,5 @@ > # it, creating race conditions. See bug #836220 > DEFAULT_GMAKE_FLAGS += TARGETS='$$(LIBRARY) $$(SHARED_LIBRARY) $$(PROGRAM)' > > +ifdef MOZ_FOLD_LIBS_FLAGS > +DEFAULT_GMAKE_FLAGS += OS_CFLAGS='$(MOZ_FOLD_LIBS_FLAGS)' You should use XCFLAGS instead.

Thanks for reviews. I can't reproduce try failure locally with MSVC, which is strange. It seems like MSC_VER is not defined on try for some reason. I will keep investigating. Meantime, the other NSPR part is ready to be committed. m-c part depends on both MSVC parts, so it will have to wait.

Comment on attachment 8592241 [details] [diff] [review] NSPR: Use correct library prefix on mingw I found the reason why it didn't work on try. MSVC version detection is broken because NSPR can't handle properly the way sccache is used. I will file a separate bug for that, but this patch itself is fine. https://treeherder.mozilla.org/#/jobs?repo=try&revision=0884b5bf3893

Ryan, thanks for the commit, but shouldn't this go to NSPR repo first?

Thanks for the review. This may be landed after patch from bug 1160125. I could land it on m-i like the previous patch was landed myself, but I don't think that's the right thing to do.

I don't have the ability to land in the NSPR repo.

wtc, can you please help get this NSPR patch landed?

Comment on attachment 8592241 [details] [diff] [review] NSPR: Use correct library prefix on mingw Review of attachment 8592241 [details] [diff] [review]: ----------------------------------------------------------------- r=wtc. This seems like a continuation of the changeset https://hg.mozilla.org/projects/nspr/rev/133b835b9ad9 in NSPR 4.10.8. Note that both of these changes break the OS/2 build. I guess we have dropped OS/2 support? Patch checked in: https://hg.mozilla.org/projects/nspr/rev/cfda142e3c2b

Thank you for checkins. Could you please also commit attachment 8599412 [details] [diff] [review]? It has been already committed to m-c, but not NSPR. BTW, I don't know much about OS/2, but I think it should be fine with the patch.

Comment on attachment 8599412 [details] [diff] [review] NSPR: configure part (for commit) Patch checked in: https://hg.mozilla.org/projects/nspr/rev/337fef162531

Comment on attachment 8592241 [details] [diff] [review] NSPR: Use correct library prefix on mingw Review of attachment 8592241 [details] [diff] [review]: ----------------------------------------------------------------- ::: nsprpub/config/rules.mk @@ -80,5 @@ > # Win95 and OS/2 require library names conforming to the 8.3 rule. > # other platforms do not. > # > ifeq (,$(filter-out WIN95 WINCE WINMO OS2,$(OS_TARGET))) > -LIBRARY = $(OBJDIR)/$(LIBRARY_NAME)$(LIBRARY_VERSION)_s.$(LIB_SUFFIX) Jacek: note the "OS2" on line 83. This is why I think this patch affects OS/2.

NSPR parts were merged to m-c, so I landed the last part.