1149279 - remove unused build config variable NSS_NO_LIBPKIX

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Dana Keeler (she/her) [:keeler]]

NSS_NO_LIBPKIX was added in bug 787155, it seems, but we've since removed libpkix as a certificate validation option, so we never call it (and all instances of NSS_NO_LIBPKIX in actual code appear to have been removed).

Comment 1

[Tyler Steiman]

Hey I would love to be assigned this bug, can you assign it to me David?

Comment 2

[Tyler Steiman]

As NSS_NO_LIBPKIX seems to still be being tested in the configure.in: 

if test -n "$NSS_NO_LIBPKIX"; then

    AC_DEFINE(NSS_NO_LIBPKIX)

fi

AC_SUBST(NSS_NO_LIBPKIX)

What would be the appropriate course of action? In b2g/confvars.sh it's also being defined. Just a little help would clear things up as my understanding of the summary is it's not being used, thanks!

Comment 3

[Dana Keeler (she/her) [:keeler]]

Hi Muhsin, I believe the use of NSS_NO_LIBPKIX in configure.in is just the way we convert variables set in confvar.sh and related files to build-time #defines. Since NSS_NO_LIBPKIX doesn't appear in any source files, it should be ok to simply remove all references to it.

Comment 4

[Tyler Steiman]

Attachment: Removes unused NSS_NO_LIBPKIX

Patch to remove unused NSS_NO_LIBPKIX. David, you are the only name in the suggested reviewers dropdown list so if you are unable to review please let me know who can. Thank you!!

Comment 5

[Dana Keeler (she/her) [:keeler]]

Comment on attachment 8614810 [details] [diff] [review]
Removes unused NSS_NO_LIBPKIX

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

Looks good to me, but technically I think this needs sign-off from someone like :gps.

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8614810 [details] [diff] [review]
Removes unused NSS_NO_LIBPKIX

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

I love patches that delete dead code.

Comment 7

[Tyler Steiman]

Keyword changed to checkin-needed. At this point do I change the status to 'RESOLVED' or is this done upon checkin? Thanks!

Comment 8

[Dana Keeler (she/her) [:keeler]]

Great!

This bug will get marked RESOLVED automatically when it gets merged to mozilla-central. In theory we should first run it through try, although it will almost certainly not break anything. Since I had another patch to try, I went ahead and included it with that:

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

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/411a9af14178

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/411a9af14178