Closed Bug 1237872 Opened 8 years ago Closed 8 years ago

Write gyp files for NSS build

Categories

(NSS :: Build, defect)

Product:
NSS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox46 affected)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox46 --- affected

People

(Reporter: ted, Assigned: ted)

References

Details

Attachments

(2 files, 4 obsolete files)

build.sh
1.21 KB, text/plain
Details
gyp build system for NSS
176.01 KB, patch
franziskus
: review+
Details | Diff | Splinter Review 
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.
Note that I mentioned this during the NSS triage at Mozlando, too.
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.
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!
Assignee: nobody → ted
Status: NEW → ASSIGNED
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!
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).
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.
Flags: needinfo?(ted)
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.
Flags: needinfo?(ted)
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.
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.
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).
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
Depends on: 1294417
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
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
Blocks: 1295937
No longer blocks: 1129924
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.
Attached file 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/
Attachment #8782884 - Attachment mime type: application/x-sh → text/plain
(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/
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
enable_tls_1_3 shouldn't have any effect since bug 1283604 landed
Ah, my NSS checkout must be slightly out of date. Thanks!
(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
(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!
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
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.
Flags: needinfo?(ted)
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?
Flags: needinfo?(ted)
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.
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.
Blocks: 1308463
Attached patch gyp build system for NSS (obsolete) — — Splinter Review 
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.
Attachment #8798836 - Flags: review?(franziskuskiefer)
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!
Attachment #8798836 - Flags: review?(franziskuskiefer)
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.
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
(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.
Flags: needinfo?(ted)
(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.)
(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.
Flags: needinfo?(ted)
Attached patch gyp build system for NSS (obsolete) — — Splinter Review 
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.
Attachment #8798836 - Attachment is obsolete: true
Attachment #8799392 - Flags: review?(franziskuskiefer)
Attached patch bogo-gyp.patch (obsolete) — — Splinter Review 
The bogo shim got added since you started working on this. It needs to get gyp as well.
Flags: needinfo?(ted)
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
Attached patch gyp build system for NSS (obsolete) — — Splinter Review 
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.
Attachment #8799392 - Attachment is obsolete: true
Attachment #8799415 - Attachment is obsolete: true
Attachment #8799392 - Flags: review?(franziskuskiefer)
Flags: needinfo?(ted)
Attachment #8799503 - Flags: review?(franziskuskiefer)
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 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.
Attachment #8799503 - Flags: review?(franziskuskiefer)
Oops, I didn't actually commit my changes before generating that patch...
(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.
> 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.
I committed the outstanding changes in this revision.
Attachment #8799503 - Attachment is obsolete: true
Attachment #8799709 - Flags: review?(franziskuskiefer)
(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 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?
Attachment #8799709 - Flags: review?(franziskuskiefer) → review+
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.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
I assume you're going to land your taskcluster integration patch yourself?
--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
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?
Flags: needinfo?(ted)
Blocks: 1310544
(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?
Flags: needinfo?(ted)
See Also: → 1310877
Depends on: 1311619
See Also: → 1369433
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: