1311619 - Add BSD support for gyp build in NSS

Status: RESOLVED FIXED

(NSS :: Build, defect)

Description

[Jan Beich]

coreconf/* is a graveyard of platform support. Patches for Tier3 are bitrotting on BMO for decades. With GYP let's assume GNU compatibility any *modern* Unix-like system cannot survive without.

With bug 1295937 impending to break ESR52 for Tier3 platforms we need a fix ASAP.

Comment 1

[Jan Beich]

Attachment: v0

Landry, can you trying building with attachment 8782884 [details] ?

Comment 2

[Landry Breuil (:gaston)]

What's needed to enable 'using gyp to build nss' on m-c ? is it already the default ?

Comment 3

[Jan Beich]

NSS + GYP build hasn't landed to mozilla-central yet. With how things are going, I think, it'd land just as 52.0 moves to Aurora, making backports harder and perpetuating Tier3 breakage for the next ESR cycle.

Comment 4

[Landry Breuil (:gaston)]

So even if i apply your patch, those codepaths wont be used ? What's the point then, if we have no way to *test* it ? It has to be applied on top of another patch ? Built from nss repo ? Apply patch on another repo ?

Comment 5

[Jan Beich]

Comment on attachment 8802836 [details] [diff] [review]
v0

Breaks Linux according to Try, the rest is blocked by bug 1309507:
https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=cff547d543da

Comment 6

[(not currently active) Ted Mielczarek]

(In reply to Jan Beich from comment #3)
> NSS + GYP build hasn't landed to mozilla-central yet. With how things are
> going, I think, it'd land just as 52.0 moves to Aurora, making backports
> harder and perpetuating Tier3 breakage for the next ESR cycle.

Thanks for being proactive here. I'm happy to help get things working in advance of landing my patches.

Looking at your patch, I wonder if it'd make sense to add a "unix" variable to replace all those `'OS!="win" and OS!="mac" and OS!="android"'` with `'unix==1'`?

(In reply to Landry Breuil (:gaston) from comment #4)
> So even if i apply your patch, those codepaths wont be used ? What's the
> point then, if we have no way to *test* it ? It has to be applied on top of
> another patch ? Built from nss repo ? Apply patch on another repo ?

The patches in bug 1295937 use the NSS gyp files to build NSS as part of Firefox.

Comment 7

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8802836 [details] [diff] [review]
v0

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

::: coreconf/config.gypi
@@ +260,5 @@
>              ],
>            }],
> +          [ 'OS=="dragonfly"', {
> +            'defines': [
> +              'DRAGONFLY',

This is not used anywhere in the code base so doesn't have any effect.

@@ +284,5 @@
> +              'HAVE_STRERROR',
> +              'HAVE_BSD_FLOCK',
> +              'XP_UNIX',
> +            ],
> +            'cflags': [

The following should be combined with the linux section. This is duplication things.

@@ +450,5 @@
>        'variables': {
>          'process_map_file': ['/bin/sh', '-c', '/bin/grep -v ";-" >(mapfile) | sed -e "s,;+,," -e "s; DATA ;;" -e "s,;;,," -e "s,;.*,;," > >@(_outputs)'],
>        },
>      }],
> +    [ 'OS!="win" and OS!="linux" and OS!="android"', {

see ted's comment 6

Comment 8

[Landry Breuil (:gaston)]

Comment on attachment 8802836 [details] [diff] [review]
v0

Resetting feedback review, ENOTIME and no proper instructions for how to test this, given that it depends on other unlanded stuff ?

Comment 9

[Landry Breuil (:gaston)]

So it seems bug 1237872 landed, effectively breaking bsd platforms not using --with-system-nss, which is the case on the buildbot. AC_MSG_ERROR([building in-tree NSS is not supported on this platform. Use --with-system-nss]) is old-configure.in is triggered. So, i'm locally adding the BSDs to the conditional...

Jan, i've tried applying your patch on top of m-i tip but it seems it's meant to apply on top of nss tip ? it doesnt apply either inside security/nss on m-i.....

Comment 10

[Jan Beich]

Attachment: v1

https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=ad5a5233d133

--m32 fails to build here: http://sprunge.us/TgRQ

Landry, try to test standalone build first! Keep in mind GYP maybe used to build system NSS in future.

  $ pkg_add bash gyp # the rest are same
  $ ./build.sh -v
  $ ./build.sh --opt
  $ ./build.sh --asan # if supported

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #6)
> Looking at your patch, I wonder if it'd make sense to add a "unix"
> variable to replace all those `'OS!="win" and OS!="mac" and
> OS!="android"'` with `'unix==1'`?

When did Android and OS X stop being Unix? As for "posix==1" even Windows has some compatibility. And "bsd==1" would leave out Solaris descendants.

In a contest to be a special snowflake Tier3 only loses because of manpower.

(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #7)
> ::: coreconf/config.gypi
> @@ +260,5 @@
> >              ],
> >            }],
> > +          [ 'OS=="dragonfly"', {
> > +            'defines': [
> > +              'DRAGONFLY',
> 
> This is not used anywhere in the code base so doesn't have any effect.

Firefox supports DragonFly. Any *new* code should not regress that. Maybe get rid of platform whitelist instead.

> 
> @@ +284,5 @@
> > +              'HAVE_STRERROR',
> > +              'HAVE_BSD_FLOCK',
> > +              'XP_UNIX',
> > +            ],
> > +            'cflags': [
> 
> The following should be combined with the linux section. This is duplication
> things.

If so then OS=="mac" applies as well. ;)

Comment 11

[Jan Beich]

With bug 1309507 still not fixed Try results are mostly useless. Does mozilla-central build NSS using GYP on more platforms than NSS standalone?

Comment 12

[Landry Breuil (:gaston)]

build.sh expects a 'python' binary, i thought everywhere in mozilla source trees there was an agreement to use PYTHON from env at least... 

that said, with a symlink from python2.7 to python, build fails because 'hash gmake' in coreconf/nspr.sh returns 0 (don't ask me why, maybe the default search path) - maybe we should use 'type' instead ?


openbsd-amd64:~/src/nss/ $type gmake ; echo $?
gmake is a tracked alias for /usr/local/bin/gmake
0
openbsd-amd64:~/src/nss/ $type foo ; echo $?  
foo not found
1

i tried to replace hash by type in there, but that didnt work. hardcoding alias make=gmake does the same..

*** Parse error in /home/othersrc/nspr/Debug: Need an operator in 'OBJECT_MODE' (./config/autoconf.mk:129)
*** Parse error: Need an operator in 'OBJECT_MODE' (./config/autoconf.mk:130)
*** Parse error: Need an operator in 'endif' (./config/autoconf.mk:131)

So i ended up hardcoding gmake in coreconf/nspr.sh everywhere, which does build nspr from ../nspr.

Then, when it reaches the gyp build in nss, it fails:

gmake_: on quitte le r_pertoire __/home/othersrc/nspr/Debug__
+ gyp -f ninja -Dnss_dist_dir=/home/othersrc/dist -Dnss_dist_obj_dir=/home/othersrc/dist/Debug -Dnspr_lib_dir=/home/othersrc/dist/Debug/lib -Dnspr_include_dir=/home/othersrc/dist/Debug/include/nspr --depth=/home/othersrc/nss --generator-output=. /home/othersrc/nss/nss.gyp
gyp: target_conditions _type=="shared_library" must be length 2 or 3, not 6

Comment 13

[Jan Beich]

Attachment: v1.1

(In reply to Landry Breuil (:gaston) from comment #12)
> that said, with a symlink from python2.7 to python, build fails because 'hash
> gmake' in coreconf/nspr.sh returns 0 (don't ask me why, maybe the default
> search path) - maybe we should use 'type' instead ?

`type` builtin behavior is inconsitent where to print errors:
- FreeBSD sh and bash -> stderr
- dash, ksh and zsh -> stdout

`command -v gmake` maybe better but it'd list aliases and functions as well.

> i tried to replace hash by type in there, but that didnt work. hardcoding alias
> make=gmake does the same..

bash doesn't allow aliases in non-interactive mode unlike FreeBSD sh, dash, ksh93 or zsh. I didn't notice because "bash" is a symlink to "zsh" here. *trolling*

Let's try a function instead.

> gmake_: on quitte le r_pertoire __/home/othersrc/nspr/Debug__
> + gyp -f ninja -Dnss_dist_dir=/home/othersrc/dist
> -Dnss_dist_obj_dir=/home/othersrc/dist/Debug
> -Dnspr_lib_dir=/home/othersrc/dist/Debug/lib
> -Dnspr_include_dir=/home/othersrc/dist/Debug/include/nspr
> --depth=/home/othersrc/nss --generator-output=. /home/othersrc/nss/nss.gyp
> gyp: target_conditions _type=="shared_library" must be length 2 or 3, not 6

Installed gyp is probably too old. Can you try to drop openbsd-specific dll_suffix? Otherwise, update gyp.

Comment 14

[Landry Breuil (:gaston)]

we have gyp from svn rev1812 in the ports-tree. your v1.1 patch works better for gmake/make detection here.

using dll_suffix='.so' produces the same error message, so i wonder if there is some caching done...
tried removing the whole OS=openbsd conditional in config.gypi, same thing.

The more i have to look at it, gyp really looks like a terrible build system...

Comment 15

[Jan Beich]

(In reply to Landry Breuil (:gaston) from comment #14)
> we have gyp from svn rev1812 in the ports-tree.

2013-12-11 snapshot is... ancient. If you can't upgrade try mozbuild GYP parser via bug 1320690.
I'm using 2016-05-04 snapshot, 02b145a1a4f4 to be specific.

Comment 16

[(not currently active) Ted Mielczarek]

(In reply to Jan Beich from comment #10)
> (In reply to Ted Mielczarek [:ted.mielczarek] from comment #6)
> > Looking at your patch, I wonder if it'd make sense to add a "unix"
> > variable to replace all those `'OS!="win" and OS!="mac" and
> > OS!="android"'` with `'unix==1'`?
> 
> When did Android and OS X stop being Unix? As for "posix==1" even Windows
> has some compatibility. And "bsd==1" would leave out Solaris descendants.
> 
> In a contest to be a special snowflake Tier3 only loses because of manpower.

Yeah, in hindsight that's not quite right, is it? Looking at your patch on bug 1320690, it looks like you want *BSD to be treated like Linux/Android, which makes sense--the distinction here is "using GNU binutils". Your conditional wasn't right in the original patch here, I see that you've changed it to `'OS!="mac" and OS!="win"'`, which makes more sense. I don't know that it's worthwhile to try to do better there, but it could stand a comment like "Platforms using GNU binutils".

Comment 17

[Jan Beich]

Attachment: v1.2

https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=7dfd93349695

- Adjusted missed conditionals e.g., -lpthread via -Wl,-z,defs; --pprof build
- Replaced "Generec Unix" comment with "GNU-like"

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #16)
> comment like "Platforms using GNU binutils".

FreeBSD aarch64 doesn't use GNU binutils but replacements from ElfToolChain and LLVM projects. What you probably mean is platforms with GNU compatibility, or GNU-like for short.

Comment 18

[Jan Beich]

Attachment: v1.2

Moved GNU-like comment closer to process_map_file. I still think "Platforms using ..." is redundant as it can be inferred from the context.

Comment 19

[Landry Breuil (:gaston)]

(In reply to Jan Beich from comment #15)
> (In reply to Landry Breuil (:gaston) from comment #14)
> > we have gyp from svn rev1812 in the ports-tree.
> 
> 2013-12-11 snapshot is... ancient. If you can't upgrade try mozbuild GYP
> parser via bug 1320690.

Sorry, can you be more specific what you mean by that ? I'm lost.

Comment 20

[Jan Beich]

Build from mozilla-central which doesn't require gyp installed. While not as useful as standalone build it may still catch wrong assumptions about OpenBSD.

Comment 21

[Jan Beich]

Attachment: v1.3

https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=a6d436a073728260a77717f1aa1fa2f25fa072ae

(In reply to Jan Beich from comment #10)
> (In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #7)
> > ::: coreconf/config.gypi
> > @@ +260,5 @@
> > >              ],
> > >            }],
> > > +          [ 'OS=="dragonfly"', {
> > > +            'defines': [
> > > +              'DRAGONFLY',
> > 
> > This is not used anywhere in the code base so doesn't have any effect.
>
> Firefox supports DragonFly. Any *new* code should not regress
> that. Maybe get rid of platform whitelist instead.

Fixed after looking what DPorts does: copies coreconf/FreeBSD.mk into coreconf/DragonFly.mk.

Build logs:
NSS standalone - http://sprunge.us/TbNN
via mozilla-central - http://slexy.org/view/s2122GHt9I

Comment 22

[Landry Breuil (:gaston)]

Comment on attachment 8815507 [details] [diff] [review]
v1.3

How can i give you feedback on a patch i still dont know how to test ? Tried importing https://reviewboard-hg.mozilla.org/gecko/rev/90872118af931c9d53eff9535bbc4e5b7062a0ac (from 1320690) on top of m-c and then apply this patch under security/nss, but it fails to apply on Hunk #8 of config.gypi.

I'm all willing to test things, but i need clearer instructions - i'm still lost in this maze of repos and build system horrors.

Comment 23

[Landry Breuil (:gaston)]

Finally got around updating our gyp version to a git snapshot, so the makefile generation inside nss standalone 'works now', but the build itself fails:

../../lib/util/verref.h:22: error: #pragma GCC diagnostic not allowed inside functions                                                     
../../lib/util/verref.h:23: error: #pragma GCC diagnostic not allowed inside functions                                                     
../../lib/util/verref.h:42: error: #pragma GCC diagnostic not allowed inside functions    

as we're still on gcc 4.2. A regular nss build works, because this was worked around in https://hg.mozilla.org/projects/nss/file/tip/coreconf/Werror.mk#l96

Comment 24

[Landry Breuil (:gaston)]

Added NSS_NO_GCC48 as a bandaid for now

           [ 'OS=="openbsd"', {
             'defines': [
               'OPENBSD',
+              'NSS_NO_GCC48',
             ],
           }],

But for some reason, ninja now fails:
$./build.sh -v
ninja: Entering directory `/home/othersrc/nss/out/Debug'
ninja: error: '/home/othersrc/dist/Debug/lib/libfreebl3.so.1.0', needed by '/home/othersrc/dist/Debug/lib/libfreebl3.chk', missing and no known rule to make it

Sorry, this is all insane voodoo to me. I dont know why it wants to build into ../dist .... same thing with build.sh -g.

Comment 25

[Jan Beich]

Attachment: v1.4

https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=93f2542dbf47068599333aaaa192c9d81bf7c8a5
https://treeherder.mozilla.org/#/jobs?repo=try&revision=75bbc4f339592b7623d67f18885c16acb081ef98

(In reply to Landry Breuil (:gaston) from comment #22)
> then apply this patch under security/nss, but it fails to apply on Hunk #8 of config.gypi.

NSS on mozilla-central is a bit out of date. If you cannot resolve conflicts grab from Try commits.

(In reply to Landry Breuil (:gaston) from comment #24)
> Added NSS_NO_GCC48 as a bandaid for now
>
>            [ 'OS=="openbsd"', {
>              'defines': [
>                'OPENBSD',
> +              'NSS_NO_GCC48',

The logic to add -DNSS_NO_GCC48 appears to be in coreconf/werror.py. Let's enable it for every Unix. Android is excluded to keep statu quo.

> $./build.sh -v
> ninja: Entering directory `/home/othersrc/nss/out/Debug'
> ninja: error: '/home/othersrc/dist/Debug/lib/libfreebl3.so.1.0', needed by
> '/home/othersrc/dist/Debug/lib/libfreebl3.chk', missing and no known rule to
> make it

dll_suffix (from NSS) != SHARED_LIB_SUFFIX (from GYP) doesn't work. Let's use .so then as fixing is outside of scope. .so.MAJOR.MINOR appears to be specific to ports rather than OpenBSD itself.

Comment 26

[Landry Breuil (:gaston)]

(In reply to Jan Beich from comment #25)
> dll_suffix (from NSS) != SHARED_LIB_SUFFIX (from GYP) doesn't work. Let's
> use .so then as fixing is outside of scope. .so.MAJOR.MINOR appears to be
> specific to ports rather than OpenBSD itself.

Nope, this is specific to OpenBSD globally. The base libraries also all use .so.MAJOR.MINOR scheme, without the .so.MINOR and .so symlink forest seen on other OSes.

Then i guess gyp should be fixed to use the proper SHARED_LIB_SUFFIX.

> The logic to add -DNSS_NO_GCC48 appears to be in coreconf/werror.py. Let's
> enable it for every Unix. Android is excluded to keep statu quo.

For some reason that didnt work with att #8815686, -DNSS_NO_GCC48 wasnt added.. added it manually in the defines section, and the build failed much later on:

nm: unknown option -- f
usage: nm [-AaCDegnoPprsuw] [-t d|o|x] [file ...]

Comment 27

[Jan Beich]

(In reply to Landry Breuil (:gaston) from comment #26)
> (In reply to Jan Beich from comment #25)
> > dll_suffix (from NSS) != SHARED_LIB_SUFFIX (from GYP) doesn't work. Let's
> > use .so then as fixing is outside of scope. .so.MAJOR.MINOR appears to be
> > specific to ports rather than OpenBSD itself.
> 
> Nope, this is specific to OpenBSD globally. The base libraries also all use
> .so.MAJOR.MINOR scheme, without the .so.MINOR and .so symlink forest seen on
> other OSes.

Every other downstream is fine with just .so and NO symlinks. NSPR/NSS use symbol versioning instead. Why OpenBSD needs synthetic .so.MAJOR.MINOR ?

Comment 28

[Landry Breuil (:gaston)]

That nm call comes from '[261/703] if [ ! -e /home/othersrc/dist/Debug/lib/libnssutil3.so -o ! -e /home/othersrc/dist/Debug/lib/libnssutil3.so.TOC ]; then cc -shared -Wl,--version-script,obj/lib/util/nssutil3.gen/out.nssutil.def -Wl,--gc-sections -Wl,-z,defs -Wl,--warn-unresolved-symbols -m64 -o /home/othersrc/dist/Debug/lib/libnssutil3.so -Wl,-soname=libnssutil3.so @/home/othersrc/dist/Debug/lib/libnssutil3.so.rsp && { readelf -d /home/othersrc/dist/Debug/lib/libnssutil3.so | grep SONAME ; nm -gD -f p /home/othersrc/dist/Debug/lib/libnssutil3.so | cut -f1-2 -d' '; } > /home/othersrc/dist/Debuglib/libnssutil3.so.TOC; else cc -shared -Wl,--version-script,obj/lib/util/nssutil3.gen/out.nssutil.def -Wl,--gc-sections -Wl,-z,defs -Wl,--warn-unresolved-symbols -m64 -o /home/othersrc/dist/Debug/lib/libnssutil3.so -Wl,-soname=libnssutil3.so @/home/othersrc/dist/Debug/lib/libnssutil3.so.rsp && { readelf -d /home/othersrc/dist/Debug/lib/libnssutil3.so | grep SONAME ; nm -gD -f p /home/othersrc/dist/Debug/lib/libnssutil3.so | cut -f1-2 -d' '; } > /home/othersrc/dist/Debug/lib/libnssutil3.so.tmp && if ! cmp -s /home/othersrc/dist/Debug/lib/libnssutil3.so.tmp /home/othersrc/dist/Debug/lib/libnssutil3.so.TOC; then mv /home/othersrc/dist/Debug/lib/libnssutil3.so.tmp /home/othersrc/dist/Debug/lib/libnssutil3.so.TOC ; fi; fi '

on OpenBSD, to have 'posix' output, one should use nm -P.

Comment 29

[Landry Breuil (:gaston)]

(In reply to Jan Beich from comment #27)
> (In reply to Landry Breuil (:gaston) from comment #26)
> > (In reply to Jan Beich from comment #25)
> > > dll_suffix (from NSS) != SHARED_LIB_SUFFIX (from GYP) doesn't work. Let's
> > > use .so then as fixing is outside of scope. .so.MAJOR.MINOR appears to be
> > > specific to ports rather than OpenBSD itself.
> > 
> > Nope, this is specific to OpenBSD globally. The base libraries also all use
> > .so.MAJOR.MINOR scheme, without the .so.MINOR and .so symlink forest seen on
> > other OSes.
> 
> Every other downstream is fine with just .so and NO symlinks. NSPR/NSS use
> symbol versioning instead. Why OpenBSD needs synthetic .so.MAJOR.MINOR ?

That's the OS-wide policy for library versioning.

Comment 30

[Jan Beich]

(In reply to Landry Breuil (:gaston) from comment #26)
> nm: unknown option -- f
> usage: nm [-AaCDegnoPprsuw] [-t d|o|x] [file ...]
(In reply to Landry Breuil (:gaston) from comment #28)
> on OpenBSD, to have 'posix' output, one should use nm -P.

This is emitted by GYP itself but only for ninja. Can you patch it out in the package?

(In reply to Landry Breuil (:gaston) from comment #29)
> That's the OS-wide policy for library versioning.

Does it include bundled libraries e.g., libxul.so?

Comment 31

[Landry Breuil (:gaston)]

(In reply to Jan Beich from comment #30)
> (In reply to Landry Breuil (:gaston) from comment #26)
> > nm: unknown option -- f
> > usage: nm [-AaCDegnoPprsuw] [-t d|o|x] [file ...]
> (In reply to Landry Breuil (:gaston) from comment #28)
> > on OpenBSD, to have 'posix' output, one should use nm -P.
> 
> This is emitted by GYP itself but only for ninja. Can you patch it out in
> the package?

Right, fixed locally (but ugh.)

Build fails on this now:

In file included from ../../lib/freebl/ecl/curve25519_64.c:9:
../../lib/freebl/ecl/uint128.h:10: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'uint128_t'

Same crap, requires gcc 4.6 ? freebl.gyp enforces HAVE_INT128_SUPPORT with a "# The Makefile does version-tests on GCC, but we're not doing that here." comment ....
> 
> (In reply to Landry Breuil (:gaston) from comment #29)
> > That's the OS-wide policy for library versioning.
> 
> Does it include bundled libraries e.g., libxul.so?

86501064 Nov 26 20:44 /usr/local/lib/firefox-51.0/libxul.so.68.0
78338584 Nov 21 04:48 /usr/local/lib/firefox-esr-45.5.0/libxul.so.3.0

there should be no exception to this scheme...

Comment 32

[Jan Beich]

(In reply to Landry Breuil (:gaston) from comment #31)
> In file included from ../../lib/freebl/ecl/curve25519_64.c:9:
> ../../lib/freebl/ecl/uint128.h:10: error: expected '=', ',', ';', 'asm' or
> '__attribute__' before 'uint128_t'
> 
> Same crap, requires gcc 4.6 ? freebl.gyp enforces HAVE_INT128_SUPPORT with a
> "# The Makefile does version-tests on GCC, but we're not doing that here."

Does NSS build fine as part of mozilla-central which already requires recent enough GCC? If so then it's a regression for standalone build.

Comment 33

[Landry Breuil (:gaston)]

I dont really know, the last green build was a while ago since bundled nss was marked intentionally unsupported for tier3.

Force-disabled int128 support and poly1305-donna-x64-sse2-incremental-source.c build via:

         [ 'disable_chachapoly==0', {
           'conditions': [
-            [ 'OS!="win" and target_arch=="x64"', {
+            [ 'OS!="win" and OS!="openbsd" and target_arch=="x64"', {
               'sources': [
                 'chacha20_vec.c',
                 'poly1305-donna-x64-sse2-incremental-source.c',
               ],
             }, {
               # not x64
               'sources': [
                 'chacha20.c',
@@ -336,17 +336,17 @@
       [ 'OS!="win"', {
         'conditions': [
           [ 'target_arch=="x64"', {
             'defines': [
               'NSS_USE_64',
               'NSS_X86_OR_X64',
               'NSS_X64',
               # The Makefile does version-tests on GCC, but we're not doing that here.
-              'HAVE_INT128_SUPPORT',
+            #  'HAVE_INT128_SUPPORT',
             ],

But there's still a -Werror failure, missing guards for this file ?

FAILED: obj/lib/certhigh/certhi.ocsp.o
cc -MMD -MF obj/lib/certhigh/certhi.ocsp.o.d -DNSS_NO_INIT_SUPPORT -DUSE_UTIL_DIRECTLY -DNO_NSPR_10_SUPPORT -DSSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES -DOPENBSD -DNSS_NO_GCC48 -DHAVE_BSD_FLOCK -DHAVE_STRERROR -DXP_UNIX -D_REENTRANT -DNSS_DISABLE_LIBPKIX -DDEBUG -I/home/othersrc/dist/Debug/include/nspr -I/home/othersrc/dist/private/nss -I/home/othersrc/dist/public/nss -fPIC -pipe -ffunction-sections -fdata-sections -m64 -Werror -Wall -g -gdwarf-2   -c ../../lib/certhigh/ocsp.c -o obj/lib/certhigh/certhi.ocsp.o
cc1: warnings being treated as errors
../../lib/certhigh/ocsp.c:2199: warning: expected [error|warning|ignored] after '#pragma GCC diagnostic'
../../lib/certhigh/ocsp.c:2200: warning: unknown option after '#pragma GCC diagnostic' kind
../../lib/certhigh/ocsp.c:2269: warning: expected [error|warning|ignored] after '#pragma GCC diagnostic'
[358/895] CC obj/lib/sqlite/sqlite.sqlite3.o

So far, NSS itself didnt hard-require a recent gcc, but maybe that changed. At that point, honestly, i dont care anymore.

Comment 34

[(not currently active) Ted Mielczarek]

It's not an NSS policy change of supported compilers, AFAIK, it was just me not bothering to port that logic to the gyp build system.

Comment 35

[Jan Beich]

Attachment: v1.5

https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=fee3f379da647c0f8699d352ee0fbb311971df99

(In reply to Landry Breuil (:gaston) from comment #33)
> ../../lib/certhigh/ocsp.c:2199: warning: expected [error|warning|ignored] after
> '#pragma GCC diagnostic'
> ../../lib/certhigh/ocsp.c:2200: warning: unknown option after '#pragma GCC
> diagnostic' kind
> ../../lib/certhigh/ocsp.c:2269: warning: expected [error|warning|ignored] after
> '#pragma GCC diagnostic'
> [358/895] CC obj/lib/sqlite/sqlite.sqlite3.o

Apply bug 1321296. It'd also fail due to -std=c++0x that v1.5 here tries to fix.

Comment 36

[Landry Breuil (:gaston)]

Comment on attachment 8815761 [details] [diff] [review]
v1.5

With v1.5 and all the recent landings on nss repo, i just need local fixes for int128 as in comment #33, it's almost good:

subprocess.CalledProcessError: Command '['/home/othersrc/dist/Debug/bin/shlibsign', '-v', '-i', '/home/othersrc/dist/Debug/lib/libfreebl3.so']' returned non-zero exit status 1

If i try to run the command manually, it fails with shlibsign: can't load library 'libnspr4.so.1.0' so i guess it's a matter of LD_LIBRARY_PATH pointing at where libnspr is installed.

Comment 37

[(not currently active) Ted Mielczarek]

We had discussed this in bug 1315231--the gyp build currently assumes that the NSPR libraries and NSS libraries are all installed in the same path.

Comment 38

[Landry Breuil (:gaston)]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #37)
> We had discussed this in bug 1315231--the gyp build currently assumes that
> the NSPR libraries and NSS libraries are all installed in the same path.

Well i didnt specify anything, and the nspr libs (as the nss libs) were automagically installed in $(srcdir)/../dist/Debug/lib, which wasnt in the default LD_LIBRARY_PATH ?

Comment 39

[Franziskus Kiefer [:franziskus]]

(In reply to Landry Breuil (:gaston) from comment #38)
> Well i didnt specify anything, and the nspr libs (as the nss libs) were
> automagically installed in $(srcdir)/../dist/Debug/lib, which wasnt in the
> default LD_LIBRARY_PATH ?

That's the expected behaviour. The gyp build resembles the old Make build for a smoother transition. Thus nss and nspr gets built into ../dist/OBJ_DIR/. We only simplified naming of the OBJ_DIR.

Comment 40

[Jan Beich]

Attachment: v1.6

(In reply to Landry Breuil (:gaston) from comment #36)
> need local fixes for int128 as in comment #33

Addressed slightly different i.e., try to use uint128.c. No other changes in v1.6.

> shlibsign: can't load library 'libnspr4.so.1.0'

.so.MAJOR.MINOR is still unfixed. Did you hack the build or executables linked against system NSPR?

Comment 41

[Landry Breuil (:gaston)]

(In reply to Jan Beich from comment #40)
> Created attachment 8816110 [details] [diff] [review]
> v1.6
> 
> (In reply to Landry Breuil (:gaston) from comment #36)
> > need local fixes for int128 as in comment #33
> 
> Addressed slightly different i.e., try to use uint128.c. No other changes in
> v1.6.

Not sure this is the right way to do, since that wouldnt use the optimizations on openbsd with a recent compiler..

> > shlibsign: can't load library 'libnspr4.so.1.0'
> 
> .so.MAJOR.MINOR is still unfixed. Did you hack the build or executables
> linked against system NSPR?

Nope, as the system nspr is /usr/local/lib/libnspr4.so.23.3. More a problem of runtime env during build, or --rpath when linking...

Comment 42

[Jan Beich]

Comment on attachment 8816110 [details] [diff] [review]
v1.6

Martin, can you check NSS builds fine with GYP on NetBSD?

  $ pkg_add bash git gmake mercurial ninja py27-pip perl5

  # devel/gyp is too old
  $ git clone https://chromium.googlesource.com/external/gyp
  $ pip install --user ./gyp
  $ PATH=$PATH:$HOME/.local/bin

  $ hg clone https://hg.mozilla.org/projects/nspr
  $ hg clone https://hg.mozilla.org/projects/nss

  $ cd nss
  $ hg import 'https://bugzilla.mozilla.org/attachment.cgi?id=8816110'

  $ ./build.sh -v
  $ ./build.sh --opt

Comment 43

[Martin Husemann]

(In reply to Jan Beich from comment #42)
> Comment on attachment 8816110 [details] [diff] [review]
> v1.6
> 
> Martin, can you check NSS builds fine with GYP on NetBSD?

Done, both builds work fine, but the result is static libs only - is that on purpose?

Comment 44

[Jan Beich]

(In reply to Martin Husemann from comment #43)
> Done, both builds work fine, but the result is static libs only - is that on
> purpose?

No. Did you check ../dist/Debug/lib/ and ../dist/Release/lib/ directories ?

Perhaps, GYP itself needs to be patched on NetBSD. Try removing "netbsd" from a conditional in pylib/gyp/generator/ninja.py then reinstalling.

Comment 45

[Martin Husemann]

Ah no, only look at ./out/Release.
Everything seems to be fine:

[/tmp/dist/Release/lib] martin@night-owl > ls
libfreebl3.chk      libnss3.so.TOC      libplc4.a*          libsoftokn3.so*
libfreebl3.so*      libnssckbi.so*      libplc4.so*         libsoftokn3.so.TOC
libfreebl3.so.TOC   libnssckbi.so.TOC   libplds4.a*         libsqlite3.so*
libgtest1.so*       libnssdbm3.chk      libplds4.so*        libsqlite3.so.TOC
libgtest1.so.TOC    libnssdbm3.so*      libsectool.a        libssl3.so*
libnspr4.a*         libnssdbm3.so.TOC   libsmime3.so*       libssl3.so.TOC
libnspr4.so*        libnssutil3.so*     libsmime3.so.TOC    pkgconfig/
libnss3.so*         libnssutil3.so.TOC  libsoftokn3.chk

Comment 46

[Jan Beich]

(In reply to Landry Breuil (:gaston) from comment #41)
>> Addressed slightly different i.e., try to use uint128.c.
> Not sure this is the right way to do, since that wouldnt use the
> optimizations on openbsd with a recent compiler..

I give up. Fixing OpenBSD requires way too much effort. FreeBSD will stop supporting GCC 4.2 on anything but sparc64 and mips* by 2017-01-01. If you only care about compiler requirements specific to mozilla-central then we can go back to v1.4.

Comment 47

[Landry Breuil (:gaston)]

All i care nowadays is being able to build m-c without patches, nss and nspr in ports can still build standalone iwith the old make-style stuff....

Comment 48

[Jan Beich]

Comment on attachment 8815686 [details] [diff] [review]
v1.4

(In reply to Landry Breuil (:gaston) from comment #47)
Can you finally test build as part of m-c then?

  $ cd /path/to/mozilla-central
  $ hg import https://hg.mozilla.org/try/raw-rev/ffe3d66b8e35
  $ hg import https://hg.mozilla.org/try/raw-rev/f2ac63e8068b
  $ ./mach bootstrap
  $ ./mach build

Comment 49

[Landry Breuil (:gaston)]

(In reply to Jan Beich from comment #48)
> Comment on attachment 8815686 [details] [diff] [review]
> v1.4
> 
> (In reply to Landry Breuil (:gaston) from comment #47)
> Can you finally test build as part of m-c then?
> 
>   $ cd /path/to/mozilla-central
>   $ hg import https://hg.mozilla.org/try/raw-rev/ffe3d66b8e35
>   $ hg import https://hg.mozilla.org/try/raw-rev/f2ac63e8068b
>   $ ./mach bootstrap
>   $ ./mach build

With this two changes, i **think** most of nss built (the libs are in obj/dist/bin/*, yay!), but the build fails - and i think this is something totally different:

35:58.02 In file included from /home/obj/m-c/js/src/Unified_cpp_js_src0.cpp:20:                                                            
35:58.04 /home/othersrc/mozilla-central/js/src/builtin/Intl.cpp:2171:39: error: reference to 'timezone' is ambiguous                       
35:58.06     for (const auto& legacyTimeZone : timezone::legacyICUTimeZones) {                                                             
35:58.08                                       ^                                                                                           
35:58.09 /usr/include/sys/time.h:72:8: note: candidate found by name lookup is 'timezone'                                                  
35:58.09 struct timezone {
35:58.09        ^
35:58.12 /home/othersrc/mozilla-central/js/src/builtin/IntlTimeZoneData.h:8:11: note: candidate found by name lookup is 'js::timezone'
35:58.12 namespace timezone {

Trying the obvious easy fix:

 IsLegacyICUTimeZone(const char* timeZone)
 {
-    for (const auto& legacyTimeZone : timezone::legacyICUTimeZones) {
+    for (const auto& legacyTimeZone : js::timezone::legacyICUTimeZones) {

Comment 50

[Landry Breuil (:gaston)]

Filed #1321777 for the timezone thing, but besides that, build of m-c is successful with your nss patches :)

Comment 51

[(not currently active) Ted Mielczarek]

Comment on attachment 8815686 [details] [diff] [review]
v1.4

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

I still think it would be nicer to set a variable to use in place of all the `'OS!="mac" and OS!="win"'` conditionals. I don't know what you'd call it, but `gnu_toolchain` or `gnu_compatible_toolchain` would be my suggestion. I wouldn't block landing this on that, but if it's not hard it would make things clearer and a bit easier to maintain.

There's just one thing here that I think isn't correct.

::: coreconf/config.gypi
@@ +52,4 @@
>            'optimize_flags%': '-O2',
> +          'moz_debug_flags%': '-gdwarf-2',
> +          'dll_prefix%': 'lib',
> +          'dll_suffix%': 'so',

I don't think this is right. You have these in the `else` block of the `'OS=="win"` condition, which means these will get set for all non-Windows builds.

Comment 52

[(not currently active) Ted Mielczarek]

(In reply to Jan Beich from comment #13)
> > gyp: target_conditions _type=="shared_library" must be length 2 or 3, not 6
> 
> Installed gyp is probably too old. Can you try to drop openbsd-specific
> dll_suffix? Otherwise, update gyp.

I filed bug 1321801 on that, FYI.

Comment 53

[Jan Beich]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #51)
> I still think it would be nicer to set a variable to use in place of all the
> `'OS!="mac" and OS!="win"'` conditionals. I don't know what you'd call it, but
> `gnu_toolchain` or `gnu_compatible_toolchain` would be my suggestion.

Without proper feature detection (autoconf-style) such categorizing only invites bitrot or wrong assumptions. For one, GNU ld compatibility is a moving target in LLVM lld: --version-script isn't implemented yet but planned. Trying to abstract it with is_gnu_ld would only *increase* maintenance.

I'd understand better if my changes affected more than one *.gyp file.

> ::: coreconf/config.gypi
> @@ +52,4 @@
>>            'optimize_flags%': '-O2',
>> +          'moz_debug_flags%': '-gdwarf-2',
>> +          'dll_prefix%': 'lib',
>> +          'dll_suffix%': 'so',
>
> I don't think this is right. You have these in the `else` block of the
> `'OS=="win"` condition, which means these will get set for all non-Windows
> builds.

OS=="mac" overrides some (above) via % but the rest assumes GCC compatibility which Clang on OS X or MINGW share.

Comment 54

[(not currently active) Ted Mielczarek]

(In reply to Jan Beich from comment #53)
> Without proper feature detection (autoconf-style) such categorizing only
> invites bitrot or wrong assumptions. For one, GNU ld compatibility is a
> moving target in LLVM lld: --version-script isn't implemented yet but
> planned. Trying to abstract it with is_gnu_ld would only *increase*
> maintenance.

Okay, that's fine. I suspect someone will break something down the road, is all.

> > ::: coreconf/config.gypi
> OS=="mac" overrides some (above) via % but the rest assumes GCC
> compatibility which Clang on OS X or MINGW share.

The mac and non-windows cases are on the same level here, which makes me not very confident that this will work reliably in gyp. You should be able to write this as:
['OS=="win"', {
 ...
 }, {
   'conditions': [
      [ 'OS!="mac"', {
         // non-mac things
      }],
   ],
   // things for all unix
 }
]

Comment 55

[Jan Beich]

Attachment: v1.4.1

https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=8d6690d854cfb5798648cc8860f675c5a116607d

Dropped OS=="solaris" as it was incomplete and untested. GCC compatibility should be easy, though.

GNU/kFreeBSD maybe fine on mozilla-central after bug 758517 but standalone build fails:

  $ gyp -f ninja -Dnss_dist_dir=../dist -Dnss_dist_obj_dir=../dist/Debug -Dnspr_lib_dir=../dist/Debug/lib -Dnspr_include_dir=../dist/Debug/include/nspr --depth=nss --generator-output=. nss.gyp
  Traceback (most recent call last):
    File "/usr/local/bin/gyp", line 11, in <module>
      load_entry_point('gyp==0.1', 'console_scripts', 'gyp')()
    File "/usr/local/lib/python2.7/dist-packages/gyp/__init__.py", line 545, in script_main
      return main(sys.argv[1:])
    File "/usr/local/lib/python2.7/dist-packages/gyp/__init__.py", line 538, in main
      return gyp_main(args)
    File "/usr/local/lib/python2.7/dist-packages/gyp/__init__.py", line 514, in gyp_main
      options.duplicate_basename_check)
    File "/usr/local/lib/python2.7/dist-packages/gyp/__init__.py", line 130, in Load
      params['parallel'], params['root_targets'])
    File "/usr/local/lib/python2.7/dist-packages/gyp/input.py", line 2776, in Load
      check, generator_input_info)
    File "/usr/local/lib/python2.7/dist-packages/gyp/input.py", line 602, in LoadTargetBuildFilesParallel
      parallel_state.pool = multiprocessing.Pool(multiprocessing.cpu_count())
    File "/usr/lib/python2.7/multiprocessing/__init__.py", line 232, in Pool
      return Pool(processes, initializer, initargs, maxtasksperchild)
    File "/usr/lib/python2.7/multiprocessing/pool.py", line 138, in __init__
      self._setup_queues()
    File "/usr/lib/python2.7/multiprocessing/pool.py", line 234, in _setup_queues
      self._inqueue = SimpleQueue()
    File "/usr/lib/python2.7/multiprocessing/queues.py", line 354, in __init__
      self._rlock = Lock()
    File "/usr/lib/python2.7/multiprocessing/synchronize.py", line 147, in __init__
      SemLock.__init__(self, SEMAPHORE, 1, 1)
    File "/usr/lib/python2.7/multiprocessing/synchronize.py", line 75, in __init__
      sl = self._semlock = _multiprocessing.SemLock(kind, value, maxvalue)
  OSError: [Errno 78] Function not implemented

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #54)
> I suspect someone will break something down the road, is all.

Added cc_use_gnu_ld. GCC compat switch would be non-trivial due to Xcode glue e.g., cflags vs. OTHER_CFLAGS. What else of GNU is left? -lpthread is only used by bundled sqlite3 outside of Linux.

Comment 56

[Jan Beich]

Hmm, HAVE_BSD_FLOCK and _REENTRANT appear to be a cargo cult from NSPR.

Comment 57

[Jan Beich]

Comment on attachment 8816716 [details] [diff] [review]
v1.4.1

Adding NSS peer to confirm the direction. Hopefully, it lands before the last NSS update after which Firefox 53 switches to Aurora.

Comment 58

[(not currently active) Ted Mielczarek]

Comment on attachment 8816716 [details] [diff] [review]
v1.4.1

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

Thanks, this looks good! Sorry for the lag on this last review, the Mozilla company all-hands was last week.

Comment 59

[Franziskus Kiefer [:franziskus]]

https://hg.mozilla.org/projects/nss/rev/582aa8f488d1868f23ce1d444d1da286f860486b