1237872 - Write gyp files for NSS build

Status: RESOLVED FIXED

(NSS :: Build, defect)

Description

[(not currently active) Ted Mielczarek]

The integration between the NSS build and Mozilla build is a big headache for Gecko build peers. After some discussions with ekr we've decided that if we can write gyp files to drive the NSS build, the Mozilla build can invoke those with fewer problems.

Ideally someone with NSS experience could do the bulk of the work on the NSS side, but obviously we will help out in whatever way we can.

Whether this becomes the official NSS build system or simply maintained in parallel is not really important to us, as long as it works.

Comment 1

[Mike Hommey [:glandium]]

Note that I mentioned this during the NSS triage at Mozlando, too.

Comment 2

[Steve Workman [:sworkman] (INACTIVE)]

Thanks folks! FYI, I've added this bug to our NSS wishlist (the work item was already being tracked there). Our NSS folks are working on higher priority things right now, so it'll be a while (next quarter?) before we can get to it.

Comment 3

[(not currently active) Ted Mielczarek]

I had poked at some scripts to parse the NSS Makefiles a while back, but I sat down this week and did some more work on them, and a script to generate gyp files from the output:
https://gist.github.com/luser/99de102e16cbeec3f5a0eec4d9b2fc9c

...and I got a working gyp build on 64-bit Linux:
https://hg.mozilla.org/users/tmielczarek_mozilla.com/nss/rev/7caef7504a15

I haven't run the tests yet, and I know it's missing a few things, at the very least:
* Generating certdata.c from certdata.txt
* Generating and using .def files for libraries

My plan is to fix those two issues, get the build working for all Firefox Tier-1 platforms (Linux 32/64, OS X 32/64, Windows 32/64, Android) and try to sanity check that the build is producing the same output, then try to get the generated gyp files landed in the NSS repository so we can use that for Firefox builds.

Some fun timing info on my machine (Core i7-3770, 32GB ram, SSD, Ubuntu 14.04):
* Existing NSS build system, full build:
luser@eye7:/build/nss$ time make USE_64=1 NSPR_INCLUDE_DIR=/usr/include/nspr
<...>
real	1m29.331s
user	1m22.418s
sys	0m13.154s

* Existing NSS build system, no-op rebuild:
luser@eye7:/build/nss$ time make USE_64=1 NSPR_INCLUDE_DIR=/usr/include/nspr
<...>

real	0m5.243s
user	0m2.171s
sys	0m2.928s

* gyp build system using ninja, full build:
luser@eye7:/build/nss$ time (gyp --parallel -f ninja --depth=. nss.gyp && ninja -C out/Debug)
ninja: Entering directory `out/Debug'
[1000/1000] STAMP obj/nss_all.actions_depends.stamp

real	0m22.846s
user	1m55.620s
sys	0m13.352s

* gyp build system using ninja, no-op rebuild:
luser@eye7:/build/nss$ time ninja -C out/Debug
ninja: Entering directory `out/Debug'
ninja: no work to do.

real	0m0.025s
user	0m0.016s
sys	0m0.008s

* gyp build system using ninja, touching nss.h and rebuilding:
luser@eye7:/build/nss$ touch lib/nss/nss.h
luser@eye7:/build/nss$ time ninja -C out/Debug
ninja: Entering directory `out/Debug'
[298/298] STAMP obj/nss_all.actions_depends.stamp

real	0m6.917s
user	0m33.392s
sys	0m4.538s

I tried to time the existing NSS build system, touching nss.h and rebuilding but I don't think it actually rebuilds anything in that case, so that's certainly a data point!

Comment 4

[Martin Thomson [:mt:]]

This looks like a huge improvement!  I've wasted quite a bit of time rebuilding recently, since the best plan I have involves cleaning whole directories whenever I touch header files.

> I tried to time the existing NSS build system, touching nss.h and rebuilding but I don't think it actually rebuilds anything in that case, so that's certainly a data point!

Oh yeah, that's the build system we know and love.  (We have a copy of mkdepend in the tree, but I don't think that it's actually able to build, so we should remove it.)

Ted, if you need any help to get this fit for landing, sign me on!

Comment 5

[(not currently active) Ted Mielczarek]

Glad to hear positive feedback! I'm still working on a few things, once I get to the point where it's ready to start landing I'll be sure to rope you in.

Latest version:
https://hg.mozilla.org/users/tmielczarek_mozilla.com/nss/rev/9ef4b9a0fbb9

I fixed certdata.c generation and the usage of .def files, as well as a few other fiddly things. I'm starting to massage things to prepare to generate gyp files that will build on platforms other than Linux (as well as support non-x86_64 Linux builds).

Comment 6

[Franziskus Kiefer [:franziskus]]

I played a bit with your last version Ted. It looks like google_test.gyp is missing lstdc++.
Do you plan on adding gyp builds to nspr as well? The build currently expects nspr to be installed on the system, which is usually not the case (or only an outdated version is installed). The easiest solution might be to use the existing make files for nspr, invoke them and point the nss gyp build to the nspr build.

Comment 7

[(not currently active) Ted Mielczarek]

Oh, I submitted a patch to gyp to fix that in the ninja backend:
https://chromium.googlesource.com/external/gyp/+/93cc6e2c23e4d5ebd179f388e67aa907d0dfd43d

I'll probably have to work around that in the gyp files though since not everyone will be using the bleeding-edge version.

I'm not planning on writing gyp files for NSPR, no. I already wrote moz.build files for NSPR a few months ago and the Firefox build is using those. I've just been using my system NSPR for development.

Comment 8

[(not currently active) Ted Mielczarek]

Updated patch:
https://hg.mozilla.org/users/tmielczarek_mozilla.com/nss/rev/9fd768122aff

I've got it building for Linux x86/x86-64/arm now, so next I'm going to work on getting it to build for Mac and Windows. Then I'd like to validate that the gyp+ninja build is running the same compile/link commands that the makefile build was.

I haven't run any tests yet, mostly because I don't know how. I'd like to wire the gyp+ninja build up in the NSS taskcluster graph and get it running tests there, since that seems useful.

Comment 9

[Martin Thomson [:mt:]]

Yeah, my suggestion would be to run a few try runs with the gyp build in place of the standard make.  That should flush out problems quickly.

Comment 10

[(not currently active) Ted Mielczarek]

Updated patch:
https://hg.mozilla.org/users/tmielczarek_mozilla.com/nss/rev/807907b6731c

I have it building on Mac and Windows. Windows is a little fiddly, you need Python, Perl, gyp, and ninja in your PATH (the gyp and ninja packages from msys2 don't work unfortunately, but you can use other binaries in an msys2 shell if you want), as well as Visual C++ installed, as well as a local build of NSPR, and then you can do:

gyp -f ninja -Dnspr_include_dir="c:/build/obj-nspr/dist/include/nspr" -Dnspr_libs="c:/build/obj-nspr/dist/lib/nspr4.lib c:/build/obj-nspr/dist/lib/plds4.lib c:/build/obj-nspr/dist/lib/plc4.lib" --depth=. nss.gyp
ninja -C out/Debug_x64

(you can build a 32-bit build by pointing nspr_include_dir/nspr_libs at 32-bit libs and ninja -C out/Debug).

Comment 11

[(not currently active) Ted Mielczarek]

As a data point, the entire gyp+ninja build on windows takes ~35s on my machine (in an msys2 shell):
$ time (gyp -f ninja -Dnspr_include_dir='c:/build/obj-nspr/dist/include/nspr' -Dnspr_libs='c:/build/obj-nspr/dist/lib/nspr4.lib c:/build/obj-nspr/dist/lib/plds4.lib c:/build/obj-nspr/dist/lib/plc4.lib' --depth=. nss.gyp && /c/build/depot_tools/ninja.exe -C out/Debug_x64/)
<...>
real    0m35.755s
user    0m0.000s
sys     0m0.030s

vs. 3m48s for the existing Makefile build (in a MozillaBuild shell):
$ time mozmake USE_64=1 NSS_ENABLE_WERROR=0 NSPR_INCLUDE_DIR=c:/build/obj-nspr/dist/include/nspr NSPR_LIB_DIR=c:/build/obj-nspr/dist/lib
<...>
real    3m48.411s
user    0m0.000s
sys     0m0.000s

Comment 12

[(not currently active) Ted Mielczarek]

For reference, I was able to build from a fresh Ubuntu 16.04 install by first doing:
sudo apt-get install build-essential pkg-config zlib1g-dev libnspr4-dev gyp ninja-build mercurial

and then:
gyp -f ninja --depth=. nss.gyp && ninja -C out/Debug

Comment 13

[(not currently active) Ted Mielczarek]

On OS X I was able to build from a relatively fresh install (no developer tools installed previously) by doing:
# Install Xcode command-line tools
xcode-select --install
# Install Homebrew
/usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
brew install pkg-config nspr ninja mercurial git
git clone https://chromium.googlesource.com/external/gyp/
hg clone https://hg.mozilla.org/users/tmielczarek_mozilla.com/nss
PKG_CONFIG_PATH=/usr/local/Cellar/nspr/4.12/lib/pkgconfig/ ../gyp/gyp -f ninja --depth=. nss.gyp && ninja -C out/Debug

Comment 14

[(not currently active) Ted Mielczarek]

Latest revision:
https://hg.mozilla.org/users/tmielczarek_mozilla.com/nss/rev/41c5d6ad4343

I'm working on bug 1295937 now, I've got NSS building via the gyp files in the Mozilla build on Linux, and I had to make some changes to support that. I will need some more changes to match the build configuration we build with in Mozilla, and I'll likely need some more changes to make things work on other platforms. We also build NSS in a very nonstandard configuration on most platforms, where we link NSPR+sqlite+most of NSS into the nss3 library, so I'll have to figure out how to best support that.

Comment 15

[Franziskus Kiefer [:franziskus]]

Attachment: build.sh

So this is looking pretty good already. I did some testing (on Linux for now) to see what's missing to use gyp builds instead of make.

1) it looks like you removed the nspr_cflags [1]. I could only get it to work with that.
2) we need chk files for some libs (see attaches script)
3) I don't see an option to build for 32 bit? I'm happy with defaulting to 64 bit on 64 bit machines. But we also have to be able to build the 32 bit version.
4) as long as we have both build systems we should use the old style of folder structures for builds (see attached script for where things should go)

The attached script allows you to build NSS on linux and get the same result as |make nss_build_all|. It uses the same layout (nspr and nss next to each other) than before and you can run it in our docker CI image [2].
I didn't compare binaries yet, but all tests pass! :)

[1] https://hg.mozilla.org/users/tmielczarek_mozilla.com/nss/rev/41c5d6ad4343#l2.144
[2] https://hub.docker.com/r/ttaubert/nss-ci/tags/

Comment 16

[(not currently active) Ted Mielczarek]

(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #15)
> 1) it looks like you removed the nspr_cflags [1]. I could only get it to
> work with that.

I switched it for "nspr_include_dir" which made it easier to deal with in the mozilla build. You should be able to just pass -Dnspr_include_dir=/path/to/nspr where you would have passed -Dnspr_cflags="-I/path/to/nspr".

> 2) we need chk files for some libs (see attaches script)

This should be straightforward, I will take a crack at it when I get the Mozilla build settled.

> 3) I don't see an option to build for 32 bit? I'm happy with defaulting to
> 64 bit on 64 bit machines. But we also have to be able to build the 32 bit
> version.

You should be able to pass -Dtarget_arch=ia32. I couldn't quite get it to work properly on my machine because I don't have a full set of 32-bit dependencies here. I did test that the build worked in a 32-bit Ubuntu VM.

> 4) as long as we have both build systems we should use the old style of
> folder structures for builds (see attached script for where things should go)

This should not be hard, I'll look at this as well once I get the Mozilla build together.

Thanks for testing this, hooray for passing tests!

> 
> The attached script allows you to build NSS on linux and get the same result
> as |make nss_build_all|. It uses the same layout (nspr and nss next to each
> other) than before and you can run it in our docker CI image [2].
> I didn't compare binaries yet, but all tests pass! :)
> 
> [1]
> https://hg.mozilla.org/users/tmielczarek_mozilla.com/nss/rev/41c5d6ad4343#l2.
> 144
> [2] https://hub.docker.com/r/ttaubert/nss-ci/tags/

Comment 17

[(not currently active) Ted Mielczarek]

https://hg.mozilla.org/users/tmielczarek_mozilla.com/nss/rev/476d521af74c

I added some gyp variables for use in the mozilla build:
* disable_tests
* disable_dbm
* disable_libpkix
* ssl_enable_zlib
* enable_tls_1_3
* mozilla_client

Comment 18

[Franziskus Kiefer [:franziskus]]

enable_tls_1_3 shouldn't have any effect since bug 1283604 landed

Comment 19

[(not currently active) Ted Mielczarek]

Ah, my NSS checkout must be slightly out of date. Thanks!

Comment 20

[Franziskus Kiefer [:franziskus]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #16)
> (In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #15)
> > 1) it looks like you removed the nspr_cflags [1]. I could only get it to
> > work with that.
> 
> I switched it for "nspr_include_dir" which made it easier to deal with in
> the mozilla build. You should be able to just pass
> -Dnspr_include_dir=/path/to/nspr where you would have passed
> -Dnspr_cflags="-I/path/to/nspr".

ah, okay. But on Mac it's still nspr_cflags :)

> > 3) I don't see an option to build for 32 bit? I'm happy with defaulting to
> > 64 bit on 64 bit machines. But we also have to be able to build the 32 bit
> > version.
> 
> You should be able to pass -Dtarget_arch=ia32. I couldn't quite get it to
> work properly on my machine because I don't have a full set of 32-bit
> dependencies here. I did test that the build worked in a 32-bit Ubuntu VM.

Yep, that works.

I started [1] to write down build configurations. Feel free to add things.

[1] https://wiki.mozilla.org/NSS/Build_System

Comment 21

[(not currently active) Ted Mielczarek]

(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #20)
> (In reply to Ted Mielczarek [:ted.mielczarek] from comment #16)
> > (In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #15)
> > > 1) it looks like you removed the nspr_cflags [1]. I could only get it to
> > > work with that.
> > 
> > I switched it for "nspr_include_dir" which made it easier to deal with in
> > the mozilla build. You should be able to just pass
> > -Dnspr_include_dir=/path/to/nspr where you would have passed
> > -Dnspr_cflags="-I/path/to/nspr".
> 
> ah, okay. But on Mac it's still nspr_cflags :)

Oops, I'll fix this to make it consistent across platforms.

> > > 3) I don't see an option to build for 32 bit? I'm happy with defaulting to
> > > 64 bit on 64 bit machines. But we also have to be able to build the 32 bit
> > > version.
> > 
> > You should be able to pass -Dtarget_arch=ia32. I couldn't quite get it to
> > work properly on my machine because I don't have a full set of 32-bit
> > dependencies here. I did test that the build worked in a 32-bit Ubuntu VM.
> 
> Yep, that works.
> 
> I started [1] to write down build configurations. Feel free to add things.
> 
> [1] https://wiki.mozilla.org/NSS/Build_System

Great!

Comment 22

[(not currently active) Ted Mielczarek]

I'm still working on making things build on Windows as part of the Firefox build (I have it working on Linux/Mac). I remembered I hadn't tested Android, I had to fix a few things for that, so I pushed those changes:
https://hg.mozilla.org/users/tmielczarek_mozilla.com/nss/rev/de5a945b1737

I followed the 'advanced method' here to generate a standalone Android toolchain:
https://developer.android.com/ndk/guides/standalone_toolchain.html#itc

then invoked gyp like:
CC=/tmp/my-android-toolchain/bin/arm-linux-androideabi-gcc CXX=/tmp/my-android-toolchain/bin/arm-linux-androideabi-gcc  gyp -f ninja -DOS=android -Dnspr_include_dir=/build/obj-nspr-android/dist/include/nspr -Dnspr_lib_dir=/build/obj-nspr-android/dist/lib --depth=. nss.gyp

Comment 23

[Franziskus Kiefer [:franziskus]]

Ted, do you think we could land a first version of the gyp build files that support Linux and Mac? If comment 17 and comment 15 are addressed, we should be good to go for that. This would allow us to test it in development and on CI and thus give better feedback if we need anything else.

Comment 24

[(not currently active) Ted Mielczarek]

We could certainly get the current version of the patch reviewed and landed--it builds on Linux/Mac/Windows/Android. I've been holding off on that because I've been making changes to it to get things working as part of the Firefox build, but if I hit any more of those it would be fine to land them in a follow-up patch.

I still haven't implemented the .chk file generation, would you be OK landing without that as a first pass?

Comment 25

[Franziskus Kiefer [:franziskus]]

I'm not sure if things will work without the chk files. But that shouldn't be too hard. We can either use a script like the one I used (attachment 8782884 [details]) or integrate those 4 lines for chk file generations into gyp. How about the output folders? Last time I checked I also had to copy the build to the correct folder (see attached script again). I'd suggest to either add those two things before landing or land a bash script with it that does those additional steps and remove it as soon as it's not necessary anymore.

Comment 26

[(not currently active) Ted Mielczarek]

I think for now it would be best to just adapt your script to produce the chk files and produce the output folders so we can get this work landed. I had to rebase the patch over a pair of changes that added/removed files and it was annoying to fix (it would have been simple for the original patch authors to make the right changes). I'll file a follow-up bug on making the gyp build system do the chk file generation and place binaries in the correct location.

Comment 27

[(not currently active) Ted Mielczarek]

Attachment: gyp build system for NSS

Ok! I think this is ready enough to start reviewing. I will integrate attachment 8782884 [details] and hook up some gyp builds in taskcluster before landing this so that we can have confidence that it works where we need it to.

This is a pretty involved patch, obviously, so feel free to ask questions about anything that's not straightforward.

Comment 28

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8798836 [details] [diff] [review]
gyp build system for NSS

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

This doesn't seem to build (on linux). At least not the way it did before :)
I'm running

> PKG_CONFIG_PATH=$PWD/../nspr-build/config gyp -f ninja -Dnspr_include_dir='-I$PWD/../nspr-build/dist/include/nspr' -Dnspr_libs='-L$PWD/../nspr-build/dist/lib -lplds4 -lplc4 -lnspr4' --depth=. nss.gyp

Note that we can't use the system nspr!

Comment 29

[(not currently active) Ted Mielczarek]

You should be able to use the system NSPR, if it's new enough. The system NSPR on my Ubuntu 14.04 box seems to work fine.

Comment 30

[(not currently active) Ted Mielczarek]

For reference, if I remove my system nspr4-dev package, I can build with:
gyp -f ninja -Dnspr_include_dir=/build/obj-nspr/dist/include/nspr -Dnspr_lib_dir=/build/obj-nspr/dist/lib --depth=. nss.gyp
ninja -C out/Debug

Comment 31

[Franziskus Kiefer [:franziskus]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #29)
> You should be able to use the system NSPR, if it's new enough. The system
> NSPR on my Ubuntu 14.04 box seems to work fine.

NSPR isn't installed on all systems (e.g. CI) and we can't rely on a system NSPR as NSS development sometimes requires NSPR versions that are not even released yet.

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #30)
> For reference, if I remove my system nspr4-dev package, I can build with:
> gyp -f ninja -Dnspr_include_dir=/build/obj-nspr/dist/include/nspr
> -Dnspr_lib_dir=/build/obj-nspr/dist/lib --depth=. nss.gyp
> ninja -C out/Debug

hm, I must have done something wrong last time. This seems mostly ok. But I run into a different problem now.
It looks like lib/ckfw/builtins/builtins.gyp is writing the generated certdata file to stdout and not to certdata.c

> [827/1088] CC obj/lib/ckfw/builtins/obj/lib/ckfw/builtins/nssckbi.gen/nssckbi.certdata.o
> FAILED: obj/lib/ckfw/builtins/obj/lib/ckfw/builtins/nssckbi.gen/nssckbi.certdata.o 
> cc -MMD -MF obj/lib/ckfw/builtins/obj/lib/ckfw/builtins/nssckbi.gen/nssckbi.certdata.o.d -DNSS_NO_INIT_SUPPORT -DUSE_UTIL_DIRECTLY -DNO_NSPR_10_SUPPORT -DSSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES -DLINUX2_1 -DLINUX -Dlinux -DHAVE_STRERROR -DXP_UNIX -D_REENTRANT -DDEBUG -I../../lib/ckfw/builtins -I/home/franziskus/Code/nspr-build/dist/include/nspr -Idist/nss/private -Idist/nss/public -fPIC -pipe -ffunction-sections -fdata-sections -gdwarf-2 -m64   -c obj/lib/ckfw/builtins/nssckbi.gen/certdata.c -o obj/lib/ckfw/builtins/obj/lib/ckfw/builtins/nssckbi.gen/nssckbi.certdata.o
> cc: error: obj/lib/ckfw/builtins/nssckbi.gen/certdata.c: No such file or directory
> cc: fatal error: no input files
> compilation terminated.

Comment 32

[Franziskus Kiefer [:franziskus]]

(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #31)
> hm, I must have done something wrong last time. This seems mostly ok. But I
> run into a different problem now.
> It looks like lib/ckfw/builtins/builtins.gyp is writing the generated
> certdata file to stdout and not to certdata.c

ha, I only got that far because I used the system NSPR. So PKG_CONFIG_PATH for NSPR isn't working. (I'm pretty sure it used to work in an earlier version.)

Comment 33

[(not currently active) Ted Mielczarek]

(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #31)
> hm, I must have done something wrong last time. This seems mostly ok. But I
> run into a different problem now.
> It looks like lib/ckfw/builtins/builtins.gyp is writing the generated
> certdata file to stdout and not to certdata.c

You need the patch from bug 1294417 as well, that hasn't landed yet.

(In reply to Franziskus Kiefer [:fkiefer or :franziskus] from comment #32)
> ha, I only got that far because I used the system NSPR. So PKG_CONFIG_PATH
> for NSPR isn't working. (I'm pretty sure it used to work in an earlier
> version.)

I tried this out--PKG_CONFIG_PATH should work fine, but you can't just point it at an NSPR objdir. The nspr.pc file will contain paths to its expected install location. If you configure it with a --prefix and run `make install` you can point PKG_CONFIG_PATH *there*, otherwise you'll have to pass nspr_include_dir / nspr_lib_dir.

Comment 34

[(not currently active) Ted Mielczarek]

Attachment: gyp build system for NSS

OK, franziskus was having problems getting that patch to work in nss-try. He and I chatted on IRC and I tracked down the problem. The `coreconf/nspr_lib_dir.py` in that patch wasn't parsing the output of pkg-config properly, so I fixed that.

Comment 35

[Franziskus Kiefer [:franziskus]]

Attachment: bogo-gyp.patch

The bogo shim got added since you started working on this. It needs to get gyp as well.

Comment 36

[Franziskus Kiefer [:franziskus]]

The last blocker I see right now to get the first version landing is compiler detection.
Running the gyp build with clang seems to be using gcc arguments and files, and thus fails. See log at [1].

[1] https://public-artifacts.taskcluster.net/16U18ueAQCONk4V_R_EzcA/0/public/logs/live_backing.log

Comment 37

[(not currently active) Ted Mielczarek]

Attachment: gyp build system for NSS

I integrated your bogo-gyp.patch, and also put in a real test for cc_is_clang. It seems to work in my limited testing. It may not catch all cases, but I think it should work well enough for our needs.

Comment 38

[Martin Thomson [:mt:]]

I don't know how feasible this is, not knowing gyp, but not having warnings-as-errors will prevent me from moving to using this.

Also, is there anything we can do about making the commands less cumbersome?  Maybe some defaults based on the common configuration?

Comment 39

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8799503 [details] [diff] [review]
gyp build system for NSS

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

There's no gyp file for bogo anywhere. I think something went wrong.
Otherwise this is looking promising https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=0c6cf2d2311da00d65b4845c6e6386db426f386f&group_state=expanded

(In reply to Martin Thomson [:mt:] from comment #38)
> I don't know how feasible this is, not knowing gyp, but not having
> warnings-as-errors will prevent me from moving to using this.
> 
> Also, is there anything we can do about making the commands less cumbersome?
> Maybe some defaults based on the common configuration?

It shouldn't be a problem to add -Werror. I didn't get that far with reviewing yet. 
But I suspect there will be more things missing. This should really be the initial landing. Once that's done we can all use it and file follow ups for things we think are missing and should/must be added.

Comment 40

[(not currently active) Ted Mielczarek]

Oops, I didn't actually commit my changes before generating that patch...

Comment 41

[(not currently active) Ted Mielczarek]

(In reply to Martin Thomson [:mt:] from comment #38)
> I don't know how feasible this is, not knowing gyp, but not having
> warnings-as-errors will prevent me from moving to using this.

That should be straightforward, it's just one of the things I didn't quite get to.

> Also, is there anything we can do about making the commands less cumbersome?
> Maybe some defaults based on the common configuration?

Unfortunately I don't know that there's any way to specify defaults for gyp. I hate that you have to pass `--depth=.`, but gyp will only set a default if the topsrcdir is named `src` because that's what Chromium's topsrcdir is named. :-/ Similarly, ninja is not the default output format, although you can override that by setting `GYP_GENERATORS=ninja` in the environment. I think the best we can do here is probably check in a `gyp.sh` script that passes these necessary options to gyp.

Ideally we'd have something like the Firefox build system's `mach build` that handled all of this for you, but I'm not committed enough to NSS development to push something like that through.

Comment 42

[Martin Thomson [:mt:]]

> Ideally we'd have something like the Firefox build system's `mach build` that handled all of this for you, but I'm not committed enough to NSS development to push something like that through.

I am :)

I'm somewhat inclined to modify the top-level makefile to detect whether gyp is viable and have it run gyp and ninja if they are present.  It adds a slow step to the process, but on balance it's probably more ergonomic for existing users.

Comment 43

[(not currently active) Ted Mielczarek]

Attachment: gyp build system for NSS

I committed the outstanding changes in this revision.

Comment 44

[(not currently active) Ted Mielczarek]

(In reply to Martin Thomson [:mt:] from comment #42)
> > Ideally we'd have something like the Firefox build system's `mach build` that handled all of this for you, but I'm not committed enough to NSS development to push something like that through.
> 
> I am :)

I would be happy to advise if you need any assistance.

Comment 45

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8799709 [details] [diff] [review]
gyp build system for NSS

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

I didn't read every gyp file in detail and there are things that we have to add but I think we could land this as a first version (when bug 1294417 landed). I already filed bug 1309505 as a start.

The try run looks good as well https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=dd34d590f22b62baf1dbf083545b363ffd6e173a

Something that's annoying is that running gyp takes pretty long even if we wouldn't have to run it. Is there a way to detect if running gyp is necessary?

::: .hgignore
@@ +2,5 @@
>  *~
>  *OPT.OBJ/*
>  *DBG.OBJ/*
>  *DBG.OBJD/*
> +out/*

Some folks are using git. Could you update the .gitignore as well?

Comment 46

[(not currently active) Ted Mielczarek]

Depending on what version of gyp you're using, you might want to pass `--parallel` to it to get it to use more cores. (Newer versions of gyp do that by default, but the version available in Ubuntu repositories doesn't.)

I can't find that gyp supports generating ninja files that automatically re-run gyp when needed. That's unfortunate. We could probably fix that in gyp upstream.

Comment 47

[(not currently active) Ted Mielczarek]

https://hg.mozilla.org/projects/nss/rev/21dacf6b07fee05fd2ea3541d5e8899077c31325
bug 1237872 - Add gyp build system for NSS. r=fkiefer

Comment 48

[(not currently active) Ted Mielczarek]

I assume you're going to land your taskcluster integration patch yourself?

Comment 49

[Franziskus Kiefer [:franziskus]]

--parallel isn't a valid option on newer gyps (but the default). It's still slow..

I landed a rudimentary build script for TC and testing:
https://hg.mozilla.org/projects/nss/rev/0891cd3881db564673c1027a2b6b0e48b3cad7e3
review at https://nss-dev.phacility.com/D82

Comment 50

[Martin Thomson [:mt:]]

I'm a little confused about the product of all of this.  It creates separate out/Debug and out/Debug_x64 directories, but these seem to contain the same content.

Why not create out/Debug == 32-bit and out/Debug_x64 == 64-bit?  I realize that takes two NSPR builds, but that's possible, right?

Comment 51

[(not currently active) Ted Mielczarek]

(In reply to Martin Thomson [:mt:] from comment #50)
> I'm a little confused about the product of all of this.  It creates separate
> out/Debug and out/Debug_x64 directories, but these seem to contain the same
> content.
> 
> Why not create out/Debug == 32-bit and out/Debug_x64 == 64-bit?  I realize
> that takes two NSPR builds, but that's possible, right?

So, on Windows that's what it does, yes. I had to do things that way because the ninja backend requires it. On other platforms they're identical. We might be able to avoid creating those extraneous directories on non-Windows. Can you file another bug on that?