Closed Bug 351893 Opened 18 years ago Closed 18 years ago

Differentiate Basic and Extended ECC for NSS version string

Categories

(NSS :: Libraries, defect, P1)

3.11.2
defect

Tracking

(Not tracked)

RESOLVED FIXED
3.11.3

People

(Reporter: christophe.ravel.bugs, Assigned: nelson)

Details

Attachments

(2 files, 2 obsolete files)

in the libraries version string NSS_VERSION
Attached patch from Wan-TehSplinter Review
3 curves ECC = basic
more curves = extended
Wan-Teh has checked this in on the NSS 3.11 branch.  Thanks Wan-Teh.

I am going to develop another patch that will obviate changing the strings.
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → 3.11.3
Summary: Differentiate Basic and Extended version for ECC → Differentiate Basic and Extended ECC for NSS version string
The purpose of this patch is to change NSS such that, in the future
only the values NSS_VMAJOR NSS_VMINOR NSS_VPATCH and NSS_BETA will
need to be changed, and the strings will automatically follow.
Same change, shown here as a unified diff.
Attachment #237544 - Attachment is obsolete: true
Attachment #237545 - Flags: review?(wtchang)
Attachment #237544 - Attachment description: Obviate future string changes → Obviate future string changes (context diff)
Attachment #237544 - Attachment is obsolete: false
Comment on attachment 237545 [details] [diff] [review]
obviate future string changes (unified diff)

This patch is a great idea.  But it requires more
changes to the Linux and Solaris package files.
See http://lxr.mozilla.org/security/search?string=NSS_VERSION.
These files grep NSS_VERSION in nss.h to obtain the
product version string.  This patch will break those
package files.

Speaking of which, we should verify that those package
files can handle two or three instances of NSS_VERSION
in nss.h.

Anothe minor problem with this patch is that we sometimes
use "Beta 2", "Beta 3", etc. in the version string.  This
patch will lose the ability to differentiate multiple
Betas by version string.
Attachment #237545 - Flags: review?(wtchang) → review+
I think this patch also fixes the pkg Makefiles.
I've done limited testing of them, using the new "test" target I added.

This patch adds a new program, nssver, that outputs a simple NSS version string.
During the build, this program is run and the output string put in a file in 
$(DIST).  The content of that file is subsequently used by the pkg Makefiles,
obviating any source file grepping.  

Christophe, please review for Solaris.  
Wan-Teh, please review the whole approach.
Attachment #237544 - Attachment is obsolete: true
Attachment #237545 - Attachment is obsolete: true
Attachment #237630 - Flags: superreview?(wtchang)
Attachment #237630 - Flags: review?(christophe.ravel.bugs)
I should fix the "initial developer" and copyright years in the license 
boilerplate.  :)

I'm not pushing to get this into 3.11.3, so there's no big rush.

Note that I fixed the "Beta N" problem described in comment 6.
Comment on attachment 237630 [details] [diff] [review]
fix pkg Makefiles, too

I haven't tested it but the code looks good w.r.t. packaging.
Attachment #237630 - Flags: review?(christophe.ravel.bugs) → review+
Comment on attachment 237630 [details] [diff] [review]
fix pkg Makefiles, too

This patch will complicate Mozilla builds and
cross compilation.

The nssver program needs to be built in the
same way as 'nsinstall' is built.  See how we
use INTERNAL_TOOLS, NATIVE_CC, NATIVE_FLAGS in
http://lxr.mozilla.org/security/source/security/coreconf/nsinstall/Makefile

To shorten build time, Mozilla builds do not
build the entire NSS cmd directory.  So this
patch will also require a change to the PSM
makefile.  Search for "cmd/lib" and "cmd/shlibsign"
in http://lxr.mozilla.org/security/source/security/manager/Makefile.in

The distinction between NSS_VERSION_STRING and
NSS_BUILD_STRING is a good idea.  I also came to
the same conclusion.  But the " (debug)" string in
http://lxr.mozilla.org/security/source/security/nss/lib/nss/nssver.c
and
http://lxr.mozilla.org/security/source/security/nss/lib/nss/nss.rc
(there are instances of these files for the other
shared libraries), which indicates a debug build,
should also be part of NSS_BUILD_STRING.
Attachment #237630 - Flags: superreview?(wtchang) → superreview-
Comment on attachment 237630 [details] [diff] [review]
fix pkg Makefiles, too

Many programmers write code like the following to test
a Boolean value:

    if (NSS_BETA == PR_TRUE) {
        ...
    }

See http://lxr.mozilla.org/security/search?string=%3D%3D+PR_TRUE

Since NSS_BETA is a Boolean, changing it to an integer
may break some code.

We've also put "Alpha" and "RC1" in the version string
before.

In any case, I'm fine with using the hardcoded string
"Beta" for all pre-releases.
(In reply to comment #11)
> Many programmers write code like the following to test
> a Boolean value:
> 
>     if (NSS_BETA == PR_TRUE) {
>         ...
>     }

I consider such code to be broken, and have said so many times.

The proper test is

     if (NSS_BETA != PR_FALSE) {

Regarding other products that substitute their own makefiles for NSS's,
this is equivalent to using private interfaces.  The other products
complicate themselves in so doing, NSS does not complicate them. 
NSS ought not to be restrained from future development because other
products use private interfaces.

We don't need this enhancement for 3.11.3.  I'm marking this bug fixed.  
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: