[nss] Add new ports nspr and nss

[plq]

Describe the pull request

• What does your PR fix?

  Fixes #7171 #7172

• Which triplets are supported/not supported? Have you updated the CI baseline?

  • I found it compiles on x64-windows, x86-windows using latest msvc
  • Added "=skip" entries for other platforms to ci.baseline.txt

• Does your PR follow the maintainer guide?

  I did not make separate pull requests for NSPR and NSS as these are quite tightly coupled. Otherwise, It looks fine to my eyes

• If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

  I am still working on this PR yes

[ghost]

All CLA requirements met.

[dg0yt]

Better don't touch vcpkg_build_make unless needed, or in a separate PR: Each push will trigger a CI rebuild of all ports that depend on this function, taking many hours.

[JackBoosY]

Please get failure logs here. If you need some help, please ping me.

[plq]

Better don't touch vcpkg_build_make unless needed, or in a separate PR: Each push will trigger a CI rebuild of all ports that depend on this function, taking many hours.

done: #21296

[plq]

Hey @JackBoosY, thanks for your close interest.

Please get failure logs here. If you need some help, please ping me.

I don't have the resources to deal with all these platforms. The x64-windows dlls seem to work fine so far. I made sure the 32bit target builds but haven't tested the binaries. They should work, though. I can't comment about the others.

As for the logs, except for the spurious 502 from V8 source repo, the only issue I see is vcpkg_find_acquire forgetting about NSINSTALL. I don't see why it wouldn't work, perhaps you are hiding some of the untrusted changes from your CI?

[plq]

For completeness' sake, here's the stdout from x64-windows

-- Downloading https://releases.mozilla.org/pub/nspr/releases/v4.32/src/nspr-4.32.tar.gz -> nspr-4.32.tar.gz...
FATALunknown tool NSINSTALL -- unable to acquire.
CMake Error at scripts/cmake/vcpkg_find_acquire_program.cmake:493 (find_program):
  find_program called with incorrect number of arguments
Call Stack (most recent call first):
  scripts/cmake/vcpkg_find_acquire_program.cmake:534 (do_find)
  ports/nspr/portfile.cmake:14 (vcpkg_find_acquire_program)
  scripts/ports.cmake:142 (include)

[plq]

I'll try to make your job easier by providing some questions to discuss:

1. I removed mozbuild from vcpkg_find_acquire_program (vfap)

   If you don't want mozbuild in vcpkg_find_acquire_program (vfap), I can move it inside the ebuild. NSS needs GYP to build as well but I didn't put it inside vfap as it's a dead project and don't think it'll be useful elsewhere. mozbuild on the other hand is like chromium's depot_tools and could come in handy.

   your choice, though.

2. I do weird things with headers: NSS has two sets of headers: public and private. public headers are put in include/nss/nss and private ones are put in include/nss/private/nss. Users wouldn't notice anything with an accompanying FindNSS.cmake that deals with these quirks but I can't say I'm entirely happy with the current state of things.

   so with all this said, what's vcpkg's policy regarding private headers?

   1. ignore them
   2. put them together with public headers
   3. put them in a subfolder inside public headers
   4. keep the status quo and offer them as a separate component along with a custom FindNSS.cmake

   thoughts?

3. The build also produces some .exe files. I put them inside bin but the QA bits complained so I removed them. Currently they are being ignored. What do you want me to do with them?

4. It seems that the NSS build system also signs the produced binaries. It didn't work and I disabled it. I'm not sure it's supposed to work on random people's computers. I don't really care as we sign our installers anyway.

That's all I can think of. This one is may be second only to openssl in terms of weirdness of its build system :) Why do crypto libs are like this? They are the flakiest pieces of software we need to deal with, so need to be as easy to build as possible.

:man_shrugging:

[plq]

the ebuilds were rebased on latest master, with mozbuild removed from vfap

[JackBoosY]

Sorry for late, I will handle this tomorrow.

[plq]

Here it says the latest stable release is 3.66: https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/NSS_Releases

There seem to be newer releases (newest is currently 3.72) and actually seems legitimate but couldn't be sure.

[JackBoosY]

I removed mozbuild from vcpkg_find_acquire_program (vfap)

Agreed, if the build system is too complex, we should add a new port instead of that.

I do weird things with headers: NSS has two sets of headers: public and private. public headers are put in include/nss/nss and private ones are put in include/nss/private/nss. Users wouldn't notice anything with an accompanying FindNSS.cmake that deals with these quirks but I can't say I'm entirely happy with the current state of things. \ so with all this said, what's vcpkg's policy regarding private headers?

If there is a call relationship between public header files and private header files, we still need to install them in the subfolders of include, because users actually need to use them. If not, we should follow the upstream rules to choose whether to install them.

The build also produces some .exe files. I put them inside bin but the QA bits complained so I removed them. Currently they are being ignored. What do you want me to do with them?

For those executables, they serve two purposes:

1. Generate to execute test cases, for them we should not install/generate or delete directly after installation
2. To generate tools, we should record their names and install them in tools/${PORT} using vcpkg_copy_tools. See documentation.

It seems that the NSS build system also signs the produced binaries. It didn't work and I disabled it. I'm not sure it's supposed to work on random people's computers. I don't really care as we sign our installers anyway.

Yeah, some binaries, such as boost's build tool b2, also have this problem: the official binary is signed, but the code generated locally is not signed, which may cause the anti-virus report to be abnormal. The signature requires an official certificate, and there is nothing we can do about it. But users can trust the generated binaries: because we use official codes to generate them, they are safe from the perspective of sources.

[JackBoosY]

For new ports:

1. If some architectures are not officially supported, we should add the supports field in vcpkg.json to filter these architectures. Please use a blacklist instead of a whitelist to avoid unofficial triplets unable to build. See documentation.
2. If support cannot be added in some official triplets for the time being, we should set these triplets to fail in scripts/ci.baseline.txt. You can check this file to get some examples.

[plq]

• Bumped nspr to 4.33
• Bumped nss to 3.73.1 where an important security update was released. (https://developer.mozilla.org/en-US/docs/Mozilla/Projects/NSS/NSS_Releases still says 3.66 is the stable version but it looks like we're supposed to ignore it)
• Forked and patched GYP so now it supports msvc 17 (https://github.com/plq/gyp)

[JackBoosY]

Please get failure logs here.

[plq]

• Public headers now go to include/nss (was going to include/nss/nss)
• Private headers now go to include/nss/private (was going to include/nss/private/nss)
• Tools (.exe files) are now being copied to their designated locations
• vcpkg.json file was updated for NSS
• scripts/ci.baseline.txt was modified to ignore non-tested triplets

CI fails because ninja can't be found. I have it installed here: https://github.com/microsoft/vcpkg/pull/21281/commits/56c7a885df938606ea5caf08028dbf3c0f855e06#diff-960b668db44aad86a4013785856a99f980c4ee1337669d221b12aa17ca7cc130R23

I don't understand the problem here. Any input @JackBoosY ?

Thanks!

[plq]

Please get failure logs here.

Yes, I looked at them. The error is very simple:

Building NSS requires an installation of ninja: https://ninja-build.org/

How do I make sure ninja is found? This builds without any problems on my system.

I'm using https://developer.microsoft.com/en-us/windows/downloads/virtual-machines/ I'm using cmake and ninja that are installed by visual studio installer

[plq]

https://groups.google.com/a/mozilla.org/g/dev-tech-crypto/c/aV_2AJaPimI

[JackBoosY]

I can't repro this on my local machine which using Visual Studio 2017.

[plq]

I can't repro this on my local machine which using Visual Studio 2017.

Can you be a bit more specific? Is it working or is it not working?

[JackBoosY]

@plq It built fine on my local machine. I will try to improve the code.

[JackBoosY]

We can convert mozbuild part to vcpkg_*_mozbuild, but only these two ports are using them, which is not worth it.

[plq]

Vanilla GYP doesn't work with MSVC 17 aka Visual Studio 2022. You need to use my fork (https://github.com/plq/gyp/) for it to work.

[JackBoosY]

@plq We can only accept the official tools/sources, maybe we should wait for upstream approve your changes and update it in the official master branch.

[plq]

GYP is abandoned, there is no upstream to approve my (or anybody else's) changes.

[JackBoosY]

I checked the source, and I found configure/make in it. Maybe we can swith to vcpkg_*_make.

[plq]

Readme says the make-based build system is legacy and can be removed in future, that's why I went with GYP-based one.

I don't think GYP belongs in vcpkg_fetch_acquire because:

1. It's abandoned, nobody is using it
2. Vanilla version doesn't work with nss/windows/msvc17 combination

Frankly, I don't see the difference between

https://github.com/plq/gyp/archive/b3177c3f6c2c45a8ca098ae0f0ebb4536c624762.zip

and

https://github.com/chromium/gyp/archive/d6c5dd51dc3a60bf4ff32a5256713690a1a10376.zip

So you need to either bend your official tools policy for this one or drop this pull request.

[JackBoosY]

@plq If the upstream approve your changes, I think we can use your repo 😃

[plq]

[plq]

hahaha seriously though, GYP is abandoned and there is no upstream. Chrome switched to GN, NodeJS forked it as gyp-next and maybe I should name this as gyp-nss and be done with it?

[JackBoosY]

@plq Also you should use the same license with gyp.

[plq]

@plq Also you should use the same license with gyp.

No problem. https://github.com/plq/gyp-nss

[plq]

So, are we done? Should I flag this as "ready for review"?

[plq]

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful! PRs must add only one version and must not modify any published versions

for the record, this is very annoying

[JackBoosY]

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful! PRs must add only one version and must not modify any published versions

for the record, this is very annoying

That's for the version control.

[JackBoosY]

When building nss:

[749/842] ninja -t msvc -e environment.x64 -- "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\bin\Hostx64\x64\cl.exe" /nologo /showIncludes /FC @obj\nss-tool\hw-support.hw-support.obj.rsp /c ..\..\nss-tool\hw-support.c /Foobj\nss-tool\hw-support.hw-support.obj /Fdobj\nss-tool\hw-support.c.pdb 
FAILED: obj/nss-tool/hw-support.hw-support.obj 
ninja -t msvc -e environment.x64 -- "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\bin\Hostx64\x64\cl.exe" /nologo /showIncludes /FC @obj\nss-tool\hw-support.hw-support.obj.rsp /c ..\..\nss-tool\hw-support.c /Foobj\nss-tool\hw-support.hw-support.obj /Fdobj\nss-tool\hw-support.c.pdb 
d:\buildtrees\nss\x64-windows-dbg\nss\lib\freebl\blapii.h(83): error C2143: syntax error: missing ')' before '*'
d:\buildtrees\nss\x64-windows-dbg\nss\lib\freebl\blapii.h(83): error C2143: syntax error: missing '{' before '*'
d:\buildtrees\nss\x64-windows-dbg\nss\lib\freebl\blapii.h(83): error C2059: syntax error: 'type'
d:\buildtrees\nss\x64-windows-dbg\nss\lib\freebl\blapii.h(83): error C2059: syntax error: ')'

And please apply this patch.

[plq]

When building nss:

FAILED: obj/nss-tool/hw-support.hw-support.obj 

Interesting, builds fine here

And please apply this patch.

{"$id":"1","innerException":null,"message":"TF400813: The user 'Windows Live ID\\<redacted>' is not authorized to access this resource.","typeName":"Microsoft.TeamFoundation.Framework.Server.UnauthorizedRequestException, Microsoft.TeamFoundation.Framework.Server","typeKey":"UnauthorizedRequestException","errorCode":0,"eventId":3000}

[JackBoosY]

unzip this file to get patch.

[plq]

unzip this file to get patch.

done

[plq]

When building nss:

[749/842] ninja -t msvc -e environment.x64 -- "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\bin\Hostx64\x64\cl.exe" /nologo /showIncludes /FC @obj\nss-tool\hw-support.hw-support.obj.rsp /c ..\..\nss-tool\hw-support.c /Foobj\nss-tool\hw-support.hw-support.obj /Fdobj\nss-tool\hw-support.c.pdb 
FAILED: obj/nss-tool/hw-support.hw-support.obj 
ninja -t msvc -e environment.x64 -- "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\bin\Hostx64\x64\cl.exe" /nologo /showIncludes /FC @obj\nss-tool\hw-support.hw-support.obj.rsp /c ..\..\nss-tool\hw-support.c /Foobj\nss-tool\hw-support.hw-support.obj /Fdobj\nss-tool\hw-support.c.pdb 
d:\buildtrees\nss\x64-windows-dbg\nss\lib\freebl\blapii.h(83): error C2143: syntax error: missing ')' before '*'
d:\buildtrees\nss\x64-windows-dbg\nss\lib\freebl\blapii.h(83): error C2143: syntax error: missing '{' before '*'
d:\buildtrees\nss\x64-windows-dbg\nss\lib\freebl\blapii.h(83): error C2059: syntax error: 'type'
d:\buildtrees\nss\x64-windows-dbg\nss\lib\freebl\blapii.h(83): error C2059: syntax error: ')'

I dug this a bit. Here's my theory:

• Here's blapii.h: https://github.com/nss-dev/nss/blob/ae9fc118f98573a2e04dc736e59326b8d09bc60c/lib/freebl/blapii.h#L83
• Despite appearances, MSVC is actually complaining about mp_int being undefined.
• mp_int is supposed to be defined in mpi.h
• Here's mpi.h: https://github.com/nss-dev/nss/blob/ae9fc118f98573a2e04dc736e59326b8d09bc60c/lib/freebl/mpi/mpi.h#L158
• MPI stands for two very separate things: Multi-Precision Integer [Library] and Message Passing Interface.
• If you have one of the Message Passing Interface implementations installed, the include "mpi.h" line finds its header file, because (I suspect) it's installed in includedir root, which makes it higher priority than the mpi.h from the nss repository.
• Hence, MSVC reporting undefined type error like any sane compiler would do, erm, syntax error 😄

[JackBoosY]

Sorry for late, I will handle this tomorrow. I'm really busy.

[JackBoosY]

Will handle this tomorrow.

[JackBoosY]

Reproducing now.

[JackBoosY]

Can't reproduce it locally, curious why it happened.

[JackBoosY]

[730/826] ninja -t msvc -e environment.x86 -- "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\bin\Hostx64\x86\cl.exe" /nologo /showIncludes /FC @obj\nss-tool\hw-support.hw-support.obj.rsp /c ..\..\nss-tool\hw-support.c /Foobj\nss-tool\hw-support.hw-support.obj /Fdobj\nss-tool\hw-support.c.pdb 
FAILED: obj/nss-tool/hw-support.hw-support.obj 
ninja -t msvc -e environment.x86 -- "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\bin\Hostx64\x86\cl.exe" /nologo /showIncludes /FC @obj\nss-tool\hw-support.hw-support.obj.rsp /c ..\..\nss-tool\hw-support.c /Foobj\nss-tool\hw-support.hw-support.obj /Fdobj\nss-tool\hw-support.c.pdb 
d:\buildtrees\nss\x86-windows-dbg\nss\lib\freebl\blapii.h(83): error C2143: syntax error: missing ')' before '*'
d:\buildtrees\nss\x86-windows-dbg\nss\lib\freebl\blapii.h(83): error C2143: syntax error: missing '{' before '*'
d:\buildtrees\nss\x86-windows-dbg\nss\lib\freebl\blapii.h(83): error C2059: syntax error: 'type'
d:\buildtrees\nss\x86-windows-dbg\nss\lib\freebl\blapii.h(83): error C2059: syntax error: ')'

line 83 is:

SECStatus generate_prime(mp_int *prime, int primeLen);

mp_int was not recognized by the compiler.

Maybe need to repro it with Visual Studio 2019, will retry now.

[plq]

to confirm (or deny) my theory, try to compile that unit with /showIncludes to see whether the correct mpi.h gets included

[JackBoosY]

to confirm (or deny) my theory, try to compile that unit with /showIncludes to see whether the correct mpi.h gets included

I don't think so, if the header mpi.h was not included, it will report a different compiler error.

[plq]

to confirm (or deny) my theory, try to compile that unit with /showIncludes to see whether the correct mpi.h gets included

I don't think so, if the header mpi.h was not included, it will report a different compiler error.

I'm not saying that mpi.h is not being found, I'm saying that maybe the wrong mpi.h is being found

[JackBoosY]

All the mpi.h:

• blaze:x86-windows:blaze/config/MPI.h
• blaze:x86-windows:blaze/system/MPI.h
• msmpi:x86-windows:mpi.h
• mathgl:x86-windows:mgl2/mpi.h

Which header should we use?

[plq]

None, nss comes with its own mpi.h. see my comment above: https://github.com/microsoft/vcpkg/pull/21281#issuecomment-1007448956

[plq]

msmpi:x86-windows:mpi.h

This can be the only culprit as it installs itself in the include dir root.