[nss] Add new ports nspr and nss #21281

plq · 2021-11-09T17:46:07Z

Describe the pull request

What does your PR fix?

Fixes [New port request] [Netscape Portable Runtime] NSPR #7171 [New port request] [Network Security Services] NSS #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 · 2021-11-09T17:46:19Z

All CLA requirements met.

dg0yt · 2021-11-09T18:50:00Z

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.

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 4df8c6d7f0468264576f1d17401277b3d401b27f -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 7faf07c..1dfa76a 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -4704,10 +4704,6 @@
       "baseline": "3.66",
       "port-version": 0
     },
-    "nss": {
-      "baseline": "3.66",
-      "port-version": 0
-    },
     "nsync": {
       "baseline": "1.24.0",
       "port-version": 0

JackBoosY · 2021-11-10T02:46:24Z

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

plq · 2021-11-10T05:57:23Z

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 · 2021-11-10T06:30:17Z

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 · 2021-11-10T06:33:53Z

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)

JackBoosY · 2021-11-10T07:29:17Z

scripts/cmake/vcpkg_find_acquire_program.cmake

+    set(PROGNAME nsinstall)
+    set(MOZBUILD_VERSION 3.3)
+    set(PATHS ${DOWNLOADS}/tools/nsinstall/bin)
+    set(URL "https://ftp.mozilla.org/pub/mozilla/libraries/win32/MozillaBuildSetup-${MOZBUILD_VERSION}.exe")


We can only use the compressed package, not installation file.

They don't publish anything else

I don't run it, I rename it to 7z.exe and unpack it.

There are other packages that do the same in that file.

I will take a look.

Do we really want to add another program to vcpkg_find_acquire_program.cmake?
It is expensive, every change triggers rebuilds of all ports which depend on this function.
And I don't think nsinstall will become widely used.

plq · 2021-11-11T19:33:57Z

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

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.~~
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?
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?
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.

🤷‍♂️

plq · 2021-11-23T16:39:57Z

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

JackBoosY · 2021-11-24T09:15:05Z

Sorry for late, I will handle this tomorrow.

plq · 2021-11-24T15:05:21Z

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 · 2021-11-25T09:13:04Z

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:

Generate to execute test cases, for them we should not install/generate or delete directly after installation
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 · 2021-11-25T09:20:27Z

For new ports:

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.
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.

github-actions

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

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for nss but no changes to version or port version.
-- Version: 3.66
-- Old SHA: 74331f33e8cabdd7e32bab08568dfed5f2036dd2
-- New SHA: 4fc55269b5e0eef3da5603e20666d596a32a3bf1
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

plq · 2022-01-04T06:07:42Z

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)

github-actions

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

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for nss but no changes to version or port version.
-- Version: 3.75
-- Old SHA: abae7ed682f2ff2d1d3287d51659241ecb6064d8
-- New SHA: 2c918224c896738bb5862c57cd96332a5b4315e2
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
Error: Local changes detected for nspr but no changes to version or port version.
-- Version: 4.33
-- Old SHA: 99b796c358ae8d5272b275b67830095989e4661b
-- New SHA: bbe343bb451274bc1045f9cca555e3ae26f09000
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 75ae1ef84437bc457fb4db54360f88af57bb158a -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 27a7549..4c38380 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -7244,10 +7244,18 @@
       "baseline": "2021-10-23",
       "port-version": 0
     },
+    "vcpkg-tool-gyp-nss": {
+      "baseline": "2022-03-04",
+      "port-version": 0
+    },
     "vcpkg-tool-meson": {
       "baseline": "0.60.2",
       "port-version": 2
     },
+    "vcpkg-tool-mozbuild": {
+      "baseline": "3.3",
+      "port-version": 0
+    },
     "vcpkg-tool-nodejs": {
       "baseline": "14.17.4",
       "port-version": 0

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

ports/vcpkg-tool-gyp-nss/vcpkg.json
ports/vcpkg-tool-mozbuild/vcpkg.json

Valid values for the license field can be found in the documentation

github-actions

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

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for nss but no changes to version or port version.
-- Version: 3.75
-- Old SHA: abae7ed682f2ff2d1d3287d51659241ecb6064d8
-- New SHA: 2c918224c896738bb5862c57cd96332a5b4315e2
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
Error: Local changes detected for nspr but no changes to version or port version.
-- Version: 4.33
-- Old SHA: 99b796c358ae8d5272b275b67830095989e4661b
-- New SHA: bbe343bb451274bc1045f9cca555e3ae26f09000
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout 75ae1ef84437bc457fb4db54360f88af57bb158a -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/baseline.json b/versions/baseline.json
index 27a7549..4c38380 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -7244,10 +7244,18 @@
       "baseline": "2021-10-23",
       "port-version": 0
     },
+    "vcpkg-tool-gyp-nss": {
+      "baseline": "2022-03-04",
+      "port-version": 0
+    },
     "vcpkg-tool-meson": {
       "baseline": "0.60.2",
       "port-version": 2
     },
+    "vcpkg-tool-mozbuild": {
+      "baseline": "3.3",
+      "port-version": 0
+    },
     "vcpkg-tool-nodejs": {
       "baseline": "14.17.4",
       "port-version": 0

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

ports/vcpkg-tool-gyp-nss/vcpkg.json
ports/vcpkg-tool-mozbuild/vcpkg.json

Valid values for the license field can be found in the documentation

JackBoosY · 2022-03-04T06:55:30Z

ports/vcpkg-tool-gyp-nss/portfile.cmake

+if(VCPKG_CROSSCOMPILING)
+    message(FATAL_ERROR "This is a host only port!")
+endif()


I assume this is not needed, right?

JackBoosY · 2022-03-04T07:08:08Z

Looks passed.
@Neumann-A @dg0yt Please review again.

github-actions

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

ports/vcpkg-tool-gyp-nss/vcpkg.json
ports/vcpkg-tool-mozbuild/vcpkg.json

Valid values for the license field can be found in the documentation

JackBoosY · 2022-03-04T07:09:21Z

Will add license after review.

plq · 2022-03-04T10:31:03Z

NSS 3.76 was released: https://groups.google.com/a/mozilla.org/g/dev-tech-crypto/c/W13LB93wep4/m/v3yN3ISSBAAJ

Should I wait until this gets merged and bump it in a separate PR or just do it?

JackBoosY · 2022-03-07T02:01:02Z

@plq Let's wait for merge this first.

ports/nspr/portfile.cmake

ports/nss/portfile.cmake

ports/vcpkg-tool-mozbuild/vcpkg.json

ports/nss/portfile.cmake

BillyONeal · 2022-03-21T18:38:12Z

I'm pretty sure this is pretty much good, thank you so much for your contribution despite it taking a long time to get 'through the tubes'! I made only nitpick comments and 1 question.

(I will make the requested nitpick changes if you want me to)

Guard for VCPKG_BUILD_TYPE Improve error reporting for unsupported VCPKG_TARGET_ARCHITECTURE Declare support for x64 rather than !x86 (given that the URI in question only has x64 binaries) Fix version numbers to use 'version'.

BillyONeal · 2022-03-21T21:23:24Z

I just made the nitpick changes. Thanks for your contribution!

JackBoosY · 2022-03-22T00:42:06Z

Looks all good, thanks for @BillyONeal review.

BillyONeal · 2022-03-22T00:49:19Z

Thanks for your contribution!

plq · 2022-03-22T07:19:09Z

If it weren't for the fabulous test bench you guys built, this would have taken even longer. Thanks a lot for the help, kudos :)

plq force-pushed the public branch from b0fd345 to 35cb0c9 Compare November 9, 2021 17:52

plq changed the title ~~Public~~ Add nspr and nss Nov 9, 2021

plq force-pushed the public branch from 35cb0c9 to 41590fe Compare November 9, 2021 18:09

NancyLi1013 assigned JackBoosY Nov 10, 2021

github-actions bot requested changes Nov 10, 2021

View reviewed changes

JackBoosY added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Nov 10, 2021

plq force-pushed the public branch from 41590fe to 433adb3 Compare November 10, 2021 05:49

JackBoosY suggested changes Nov 10, 2021

View reviewed changes

plq force-pushed the public branch 3 times, most recently from 349a1ec to 8e967de Compare November 23, 2021 16:37

github-actions bot approved these changes Nov 24, 2021

View reviewed changes

plq force-pushed the public branch from 8e967de to b4f9fe2 Compare November 24, 2021 10:06

plq changed the title ~~Add nspr and nss~~ [nss] Add new port nspr and nss Nov 24, 2021

plq changed the title ~~[nss] Add new port nspr and nss~~ [nss] Add new ports nspr and nss Nov 24, 2021

plq force-pushed the public branch from b4f9fe2 to e6c5c45 Compare January 4, 2022 06:05

github-actions bot requested changes Jan 4, 2022

View reviewed changes

JackBoosY removed the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Mar 4, 2022

Add dependencies vcpkg-tool-mozbuild and vcpkg-tool-gyp-nss

225cf4d

github-actions bot requested changes Mar 4, 2022

View reviewed changes

clean doc

57e63bd

github-actions bot requested changes Mar 4, 2022

View reviewed changes

JackBoosY reviewed Mar 4, 2022

View reviewed changes

version

c1f0610

github-actions bot approved these changes Mar 4, 2022

View reviewed changes

JackBoosY added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Mar 7, 2022

Merge branch 'master' into public

7191549

JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Mar 18, 2022

plq requested a review from JackBoosY March 21, 2022 06:26

BillyONeal reviewed Mar 21, 2022

View reviewed changes

BillyONeal added 2 commits March 21, 2022 14:09

Merge remote-tracking branch 'origin/master' into public

e5435a6

BillyONeal approved these changes Mar 21, 2022

View reviewed changes

JackBoosY approved these changes Mar 22, 2022

View reviewed changes

BillyONeal merged commit b3c7e74 into microsoft:master Mar 22, 2022

plq deleted the public branch March 22, 2022 12:09

plq mentioned this pull request Jan 13, 2023

[nss] Bump version to 3.87, switch to official gyp-next #28933

Merged

tsondergaard mentioned this pull request Feb 22, 2024

qt5-webengine links many system libraries that it should get from vcpkg #36854

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[nss] Add new ports nspr and nss #21281

[nss] Add new ports nspr and nss #21281

plq commented Nov 9, 2021 •

edited

Loading

ghost commented Nov 9, 2021 •

edited by ghost

Loading

dg0yt commented Nov 9, 2021

github-actions bot left a comment

JackBoosY commented Nov 10, 2021

plq commented Nov 10, 2021

plq commented Nov 10, 2021 •

edited

Loading

plq commented Nov 10, 2021

JackBoosY Nov 10, 2021

plq Nov 10, 2021

JackBoosY Nov 10, 2021

dg0yt Nov 10, 2021

plq commented Nov 11, 2021 •

edited

Loading

plq commented Nov 23, 2021

JackBoosY commented Nov 24, 2021

plq commented Nov 24, 2021

JackBoosY commented Nov 25, 2021

JackBoosY commented Nov 25, 2021

github-actions bot left a comment

plq commented Jan 4, 2022 •

edited

Loading

github-actions bot left a comment

github-actions bot left a comment

JackBoosY Mar 4, 2022

JackBoosY commented Mar 4, 2022

github-actions bot left a comment

JackBoosY commented Mar 4, 2022

plq commented Mar 4, 2022 •

edited

Loading

JackBoosY commented Mar 7, 2022

BillyONeal commented Mar 21, 2022 •

edited

Loading

BillyONeal commented Mar 21, 2022

JackBoosY commented Mar 22, 2022

BillyONeal commented Mar 22, 2022

plq commented Mar 22, 2022

[nss] Add new ports nspr and nss #21281

[nss] Add new ports nspr and nss #21281

Conversation

plq commented Nov 9, 2021 • edited Loading

What does your PR fix?

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

Does your PR follow the maintainer guide?

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

ghost commented Nov 9, 2021 • edited by ghost Loading

dg0yt commented Nov 9, 2021

github-actions bot left a comment

Choose a reason for hiding this comment

JackBoosY commented Nov 10, 2021

plq commented Nov 10, 2021

plq commented Nov 10, 2021 • edited Loading

plq commented Nov 10, 2021

JackBoosY Nov 10, 2021

Choose a reason for hiding this comment

plq Nov 10, 2021

Choose a reason for hiding this comment

JackBoosY Nov 10, 2021

Choose a reason for hiding this comment

dg0yt Nov 10, 2021

Choose a reason for hiding this comment

plq commented Nov 11, 2021 • edited Loading

plq commented Nov 23, 2021

JackBoosY commented Nov 24, 2021

plq commented Nov 24, 2021

JackBoosY commented Nov 25, 2021

JackBoosY commented Nov 25, 2021

github-actions bot left a comment

Choose a reason for hiding this comment

plq commented Jan 4, 2022 • edited Loading

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

JackBoosY Mar 4, 2022

Choose a reason for hiding this comment

JackBoosY commented Mar 4, 2022

github-actions bot left a comment

Choose a reason for hiding this comment

JackBoosY commented Mar 4, 2022

plq commented Mar 4, 2022 • edited Loading

JackBoosY commented Mar 7, 2022

BillyONeal commented Mar 21, 2022 • edited Loading

BillyONeal commented Mar 21, 2022

JackBoosY commented Mar 22, 2022

BillyONeal commented Mar 22, 2022

plq commented Mar 22, 2022

plq commented Nov 9, 2021 •

edited

Loading

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

ghost commented Nov 9, 2021 •

edited by ghost

Loading

plq commented Nov 10, 2021 •

edited

Loading

plq commented Nov 11, 2021 •

edited

Loading

plq commented Jan 4, 2022 •

edited

Loading

plq commented Mar 4, 2022 •

edited

Loading

BillyONeal commented Mar 21, 2022 •

edited

Loading