362857 - Simplify the NSS version string definition.

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Wan-Teh Chang]

This bug continues Nelson's work in bug 351893.  I will propose
a simple patch that gets us half of the benefit of Nelson's patch,
essentially back to how the NSS version string macro was defined
before we added ECC support in NSS 3.11.1.

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch for the NSS_3_11_BRANCH

Christophe, could you review and test this patch by building
the Solaris and Linux NSS packages and see if they get the
correct version 3.11.5?

This patch not only fixes this bug but also changes the version
from 3.11.4 (RTM) to 3.11.5 Beta.

The simplication I made to Nelson's patch is that I still use
the string literal "3.11.5" in the definition of the NSS_VERSION
macro.  Like Nelson's patch, I use a macro _NSS_ECC_STRING for
the ECC support string.  (Note that I added a leading underscore
to the macro's name to indicate that it is used by the implementation.)
Unlike Nelson's patch, I use the string literal " Beta" (or nothing for RTM).
The result is that we restore the NSS_VERSION macro definition very
close to what it was before.  To illustrate this point, here is the
relevant change block in the diffs of nss.h with NSS_3_10_2_BETA1 after
this patch is applied:

@@ -45,20 +45,30 @@

 SEC_BEGIN_PROTOS

+/* The private macro _NSS_ECC_STRING is for NSS internal use only. */
+#ifdef NSS_ENABLE_ECC
+#ifdef NSS_ECC_MORE_THAN_SUITE_B
+#define _NSS_ECC_STRING " Extended ECC"
+#else
+#define _NSS_ECC_STRING " Basic ECC"
+#endif
+#else
+#define _NSS_ECC_STRING ""
+#endif
+
 /*
  * NSS's major version, minor version, patch level, and whether
  * this is a beta release.
  *
  * The format of the version string should be
- *     "<major version>.<minor version>[.<patch level>] [<Beta>]"
+ *     "<major version>.<minor version>[.<patch level>] [<ECC>] [<Beta>]"
  */
-#define NSS_VERSION  "3.10.2 Beta"
+#define NSS_VERSION  "3.11.5" _NSS_ECC_STRING " Beta"
 #define NSS_VMAJOR   3
-#define NSS_VMINOR   10
-#define NSS_VPATCH   2
+#define NSS_VMINOR   11
+#define NSS_VPATCH   5
 #define NSS_BETA     PR_TRUE

-
 /*
  * Return a boolean that indicates whether the underlying library
  * will perform as the caller expects.

Comment 2

[Christophe Ravel]

Comment on attachment 247557 [details] [diff] [review]
Proposed patch for the NSS_3_11_BRANCH

Packaging fails with this patch.

The parsing to get the product version in the packaging is currently:

PRODUCT_VERSION = $(shell grep NSS_VERSION $(CORE_DEPTH)/../dist/public/nss/nss.
h \
    | head -1 \
    | sed -e 's/"$$//' -e 's/.*"//' -e 's/ .*//')

It would be much easier to define a variable in nss.h that give the numeric part of NSS_VERSION directly (NSS_VERSION_NUMERIC for example). And use this variable to build NSS VERSION:

#define NSS_VERSION_NUMERIC "3.11.5"
#define NSS_VERSION NSS_VERSION_NUMERIC _NSS_ECC_STRING " Beta"

In the packaging part, we cannot directly include nss.h, but the parsing would be easier and would be less likely to be broken in the future

PRODUCT_VERSION = $(shell grep NSS_VERSION_NUMERIC $(CORE_DEPTH)/../dist/public/nss/nss.
h \
     | sed -e 's/"$$//' -e 's/.*"//')

(Take everything between the 2 double quotes from the line containing NSS_VERSION_NUMERIC).

Comment 3

[Wan-Teh Chang]

Thank you, Christophe.  I verified that the patch indeed breaks
that sed script.

I searched in the NSS tip for the "head -1" command in the
definition of PRODUCT_VERSION but didn't find it:

http://lxr.mozilla.org/security/search?string=head+-1

So you need to apply that change to the NSS trunk.

If we can change the sed script to parse the new format,
this sed script works (by Joaquin Blas):

    sed -e 's/[^"]*"//' -e 's/".*//' -e 's/ .*//'

It removes everything up to and including the first double
quote and everything after and including the second double
quote, and then removes anything after the space in the
remaining text.

Comment 4

[Christophe Ravel]

Attachment: Additional patch to fix packaging on Solaris and Linux

This additional patch would fix the packaging part.

Comment 5

[Wan-Teh Chang]

Comment on attachment 247579 [details] [diff] [review]
Additional patch to fix packaging on Solaris and Linux

r=wtc.  Thank you, Christophe.

Comment 6

[Christophe Ravel]

Comment on attachment 247557 [details] [diff] [review]
Proposed patch for the NSS_3_11_BRANCH

r+ when used with additional patch for packages.

Comment 7

[Wan-Teh Chang]

Comment on attachment 247557 [details] [diff] [review]
Proposed patch for the NSS_3_11_BRANCH

Nelson, is this patch OK?  (See also the diffs in comment 1.)

Unfortunately this patch requires changing the sed scripts in
our Solaris and Linux package makefiles, but it turned out that
Christophe had to change the parsing command (to add 'head -1'
before the sed script) before, when nss.h started to have
multiple definitions of the NSS_VERSION macro.  So there is
already a precedence of a NSS_VERSION change breaking parsing
commands.

Comment 8

[Wan-Teh Chang]

Attachment: Proposed patch for the NSS_3_11_BRANCH (as checked in)

I checked in the patch on the NSS_3_11_BRANCH (NSS 3.11.5).
I made a minor change to the format of the NSS version string in
the comment.

Checking in nss.h;
/cvsroot/mozilla/security/nss/lib/nss/nss.h,v  <--  nss.h
new revision: 1.46.2.11; previous revision: 1.46.2.10
done

Comment 9

[Wan-Teh Chang]

Attachment: Proposed patch for the NSS trunk (as checked in)

On the NSS trunk (NSS 3.12), I also needed to change the new file
lib/softoken/softkver.h.  Since softkver.h is a private header,
the macro SOFTOKEN_ECC_STRING doesn't have a leading underscore
and doesn't need a comment that warns it's for NSS internal use
only.

Checking in nss/nss.h;
/cvsroot/mozilla/security/nss/lib/nss/nss.h,v  <--  nss.h
new revision: 1.50; previous revision: 1.49
done
Checking in softoken/softkver.h;
/cvsroot/mozilla/security/nss/lib/softoken/softkver.h,v  <--  softkver.h
new revision: 1.2; previous revision: 1.1
done

Comment 10

[Christophe Ravel]

Checked in attachment #247579 [details] [diff] [review] on NSS_3_11_BRANCH:

Checking in linux/Makefile;
/cvsroot/mozilla/security/nss/pkg/linux/Makefile,v  <--  Makefile
new revision: 1.15.2.2; previous revision: 1.15.2.1
done
Checking in solaris/Makefile-devl.com;
/cvsroot/mozilla/security/nss/pkg/solaris/Makefile-devl.com,v  <--  Makefile-devl.com
new revision: 1.3.28.2; previous revision: 1.3.28.1
done
Checking in solaris/Makefile-tlsu.com;
/cvsroot/mozilla/security/nss/pkg/solaris/Makefile-tlsu.com,v  <--  Makefile-tlsu.com
new revision: 1.3.28.2; previous revision: 1.3.28.1
done
Checking in solaris/Makefile.com;
/cvsroot/mozilla/security/nss/pkg/solaris/Makefile.com,v  <--  Makefile.com
new revision: 1.7.28.2; previous revision: 1.7.28.1
done

Comment 11

[Christophe Ravel]

Checked in attachment #247579 [details] [diff] [review] [edit] on NSS trunk:

Checking in linux/Makefile;
/cvsroot/mozilla/security/nss/pkg/linux/Makefile,v  <--  Makefile
new revision: 1.16; previous revision: 1.15
done
Checking in solaris/Makefile-devl.com;
/cvsroot/mozilla/security/nss/pkg/solaris/Makefile-devl.com,v  <--  Makefile-devl.com
new revision: 1.4; previous revision: 1.3
done
Checking in solaris/Makefile-tlsu.com;
/cvsroot/mozilla/security/nss/pkg/solaris/Makefile-tlsu.com,v  <--  Makefile-tlsu.com
new revision: 1.4; previous revision: 1.3
done
Checking in solaris/Makefile.com;
/cvsroot/mozilla/security/nss/pkg/solaris/Makefile.com,v  <--  Makefile.com
new revision: 1.8; previous revision: 1.7
done