441976 - Export IMPORT_LIB_SUFFIX to the NSS makefiles

Status: RESOLVED INCOMPLETE

(Core :: Security: PSM, defect)

Description

[Bo Yang]

Attachment: A Proposed patch

In security/coreconf/WIN32.mk, IMPORT_LIB_SUFFIX is assigned to .a if it is not defined. But when cross compile, the IMPORT_LIB_SUFFIX is assigned to dll.a from the topmost config/autoconf.mk. 
And this difference incurs some errors when install the NSS related import libs. The makefile at security/manager/Makefile search libnss3.dll.a to install, but the security system only provide libnss3.a. 
This little patch export the IMPORT_LIB_SUFFIX from the top-level makefile to the NSS ones to fix the problem.

Comment 1

[cls]

Per bug 134113#301, we intentionally use the standard win32 naming convention for NSS mingw builds.

Comment 2

[cls]

Attachment: special case mingw

I haven't investigated the impact of the previous patch but this is what I was using in my test build from last week.

( bug 134113 comment 301 ? Maybe that will get hyperlinked correctly. )

Comment 3

[Bo Yang]

(In reply to comment #2)
> Created an attachment (id=326978) [details]
> special case mingw
> 
> I haven't investigated the impact of the previous patch but this is what I was
> using in my test build from last week.
> 
> ( bug 134113 comment 301 ? Maybe that will get hyperlinked correctly. )

IMPORT_LIB_SUFFIX is determined by the configure, and I think export it from the top-level config/autoconf.mk to the NSS makefiles is correct. 
And I find there is no IMPORT_LIB_SUFFIX except WINDOWS build, so I think there should be no side-impact. 

Comment 4

[Bo Yang]

Comment on attachment 326856 [details] [diff] [review]
A Proposed patch

Maybe neil is not here...

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 326856 [details] [diff] [review]
A Proposed patch

Since this is not a patch to any NSS source code, no NSS review is 
required.  This is a change to PSM.  The owner of PSM should review it.
He will have to maintain it, and possibly change it again when NSS
makefiles are changed.

Comment 6

[Wan-Teh Chang]

I agree with cls's proposal.  An NSS-based application
should respect the import library suffix that NSS uses,
rather than imposing its own import library suffix on NSS.
It is the job of mozilla/security/manager/Makefile.in
to record such knowledge of NSS build system internals.

Comment 7

[Bo Yang]

(In reply to comment #6)
> I agree with cls's proposal.  An NSS-based application
> should respect the import library suffix that NSS uses,
> rather than imposing its own import library suffix on NSS.
> It is the job of mozilla/security/manager/Makefile.in
> to record such knowledge of NSS build system internals.

I think the NSS indeed wants the NSS-based application to tell it which suffix to use. Please see:
http://mxr.mozilla.org/mozilla-central/source/security/coreconf/WIN32.mk#270

Exporting the IMPORT_LIB_SUFFIX to the NSS code is better, I think. 

Comment 8

[Bo Yang]

Ok, Wan-Teh, could you please say something about my comment? If you still feel cls's patch is better, then I will try to use that patch. 

Comment 9

[Wan-Teh Chang]

I don't understand the problem fully.  Several people
build Firefox with MinGW regularly, so there must be
something different in your build environment.  In
comment 0 you mentioned "cross compile".  What is
your host platform, and what is your target platform?

I suspect that the people who build Firefox with
MinGW successfully do not cross compile, or they
cross compile on a different host platform.  So
that might be the key difference.

For cross compilation, we override a lot of make
variables here:
http://lxr.mozilla.org/security/source/security/manager/Makefile.in#230

230 ifdef CROSS_COMPILE
231 DEFAULT_GMAKE_FLAGS += \
232         NSINSTALL="$(NSINSTALL)" \
233         NATIVE_CC="$(HOST_CC)" \
234         CC="$(CC)" \
235         CCC="$(CXX)" \
236         LINK="$(LD)" \
237         AS="$(AS)" \
238         AR='$(AR) $(AR_FLAGS:$@=$$@)' \
239         RANLIB="$(RANLIB)" \
240         RC="$(RC) $(RCFLAGS)" \
241         OS_ARCH="$(OS_ARCH)" \
242         CPU_ARCH="$(TARGET_CPU)" \
243         $(NULL)
244 SKIP_CHK=1
245 endif

So if we should override IMPORT_LIB_SUFFIX, we should
override it there.

Regarding the code snippet that you pointed out in comment 7:
http://mxr.mozilla.org/mozilla-central/source/security/coreconf/WIN32.mk#270

That code was added by Nelson's patch in bug 104541.  The
"ifndef IMPORT_LIB_SUFFIX" (and the ifndef for other variables)
wasn't necessary.  I think Nelson added the ifndef mechanically,
and didn't mean that those variables can be overridden by
the applications.  My point is that NSS should use the same
suffix for import libraries whether NSS is built stand-alone
or as part of Firefox.  If necessary I'm willing to change
NSS to use the same import library suffix (.dll.a) in the
MinGW build.  But let's figure out why others can do the
MinGW build successfully without running into this bug.

Comment 10

[Bo Yang]

(In reply to comment #9)
> I don't understand the problem fully.  Several people
> build Firefox with MinGW regularly, so there must be
> something different in your build environment.  In
> comment 0 you mentioned "cross compile".  What is
> your host platform, and what is your target platform?

The host platform is Linux and the target platform is Windows XP.

> I suspect that the people who build Firefox with
> MinGW successfully do not cross compile, or they
> cross compile on a different host platform.  So
> that might be the key difference.

I guess "cross compile" is the real reason. Abviously, cls has come across the same problem too.


> That code was added by Nelson's patch in bug 104541.  The
> "ifndef IMPORT_LIB_SUFFIX" (and the ifndef for other variables)
> wasn't necessary.  I think Nelson added the ifndef mechanically,
> and didn't mean that those variables can be overridden by
> the applications.  My point is that NSS should use the same
> suffix for import libraries whether NSS is built stand-alone
> or as part of Firefox.  If necessary I'm willing to change
> NSS to use the same import library suffix (.dll.a) in the
> MinGW build.  But let's figure out why others can do the
> MinGW build successfully without running into this bug.

Mingw use dll.a by default . http://mxr.mozilla.org/mozilla-central/source/configure.in#1968 this line code set the suffix. There should be no difference between cross compile and native mingw compile. I don't understand why Mingw compiling is successful and cross compile fails. I guess both will failed here. 
Are you sure Mingw build is workable? 

Comment 12

[Kai Engert [:KaiE:]]

Comment on attachment 326856 [details] [diff] [review]
A Proposed patch

I don't have a problem with forwarding environment variables down to the NSS build, r=kaie

But I advice to listen to Nelson's and Wan-Teh's comments.

Comment 13

[Wan-Teh Chang]

I don't understand the problem fully.  If I have to choose,
I'll choose cls's patch.

Comment 14

[Bo Yang]

(In reply to comment #13)
> I don't understand the problem fully.  If I have to choose,
> I'll choose cls's patch.

So, just choose the cls's patch. I think it is OK, too.

Comment 15

[Bo Yang]

Comment on attachment 326978 [details] [diff] [review]
special case mingw

Can I ask you to review the cls's patch?

Comment 16

[Kai Engert [:KaiE:]]

reassign bug owner.
mass-update-kaie-20120918

Comment 17

[Dana Keeler (she/her) [:keeler]]

Is this still an issue?