838989 - Fix issues with patch from bug 772144 and add support for building NSS for Android on Windows

Status: RESOLVED WONTFIX

(NSS :: Test, defect)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: Fix various issues with Android build configuration and add support for building on Windows

+++ This bug was initially created as a clone of Bug #772144 +++

I originally attached this patch to bug 772144 based on my review of Kai's patch. Kai asked me to file a new bug for it, so here it is. I think this patch is important because it changes some of the compiler options on ARM back to the way that Firefox normally builds NSS.

(In reply to Robert Relyea from Bug #772144 comment #45)
> Comment on attachment 689589 [details] [diff] [review]
> Additional changes, including changes needed to support building on Windows
> 
> r- for this patch, to feedback+ for the general idea.
> 
> Issues I'd like to see addressed:
> 
> Setting USE_PTHREADS have a number of side effects, including adding the
> _PTH to the OBJECT_DIR. I would like to make sure we aren't messing up the
> Linux install builds with this change. (I also prefer the shorter name for
> the android directory if we can keep it).

I don't understand what you mean by "I would like to make sure we aren't messing up the Linux install builds with this change." Since the OBJDIR name includes "Android" there shouldn't be any conflicts, right?

Also, I think that we should keep the _PTH suffix since that's consistent with what we do for Linux.

> The shlib sign changes should also handle the case that you have shlbisign
> on your platform. Either through an environment override or by looking for
> it in your path.

I think you can do this already by setting NSS_HOST_PREFIX="/usr" or NSS_HOST_PREFIX="/usr/lib64/nss/unsupported-tools" if shlibsign is installed in one of those directories. (Though, does nsinstall get installed there?)

> The rest of the patch looks fine to me.
> 
> bob

Comment 1

[Robert Relyea]

Comment on attachment 711194 [details] [diff] [review]
Fix various issues with Android build configuration and add support for building on Windows

r+ 

I still would rather not have the _PTH in the Android name, but I can see your constancy argument (actually having it in the Linux name is a bit of a carry on. We always have _PTH these days, so it's really just unnecessary cruft). Since android is new, we should be able to drop it here. You can kill it simply by dropping the IMPL_STRATEGY = _PTH if android is set.

Also, I would *REALLY* like things to work automatically if shlibsign is already in my path. I'm OK if setting NSS_HOST_PREFIX overrides looking for shlibsign in the path, but it it's not set it should just work...

bob

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 711194 [details] [diff] [review]
Fix various issues with Android build configuration and add support for building on Windows

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

mhamrick, could you please test out this patch on Mac OS X, and fix it if necessary? I think there may be some dylib path issue with the changes that replace sign.sh/sign.cmd with makefile logic.

Comment 3

[Meadhbh Hamrick :mhamrick]

Hey bsmith... what repo is this patch off of? the directory structure referenced in the patch looks different from the one in both nss & mozilla-central.

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Meadhbh Hamrick : mhamrick from comment #3)
> Hey bsmith... what repo is this patch off of? the directory structure
> referenced in the patch looks different from the one in both nss &
> mozilla-central.

Thanks for pointing that out. Since I wrote this patch we reorganized the directory structure and moved from CVS to Mercurial. I will update the patch.

Comment 5

[Marcus Burghardt]

Hi Brian,

This is a very old bug. Is this yet necessary?
Does this documentation help you?
https://wiki.mozilla.org/NSS:Android

I know the documentation if for Linux, but could be useful depending the demand.

Thanks,

Comment 6

[Marcus Burghardt]

Looks that this bug is not relevant anymore.
I am changing to WONTFIX.

Feel free to reopen if necessary.

Thanks!