301259 - signtool Usage message is unhelpful

Status: RESOLVED FIXED

(NSS :: Tools, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

The signtool documentation that once existed on developer.netscape.com is 
now all gone.  All we have left is the Usage message in the tool itself,
and that is very unhelpful.  The usage message cites non-existant URLs for
documentation, and it gives examples that use otherwise undocumented options
as modifiers.  Today one cannot even determine the distinct operations that 
the tool can perform from the Usage message.  This needs urgent attention.

The Usage message needs to 
- document EVERY option known to signtool
- separate the "command" options (which specify the operation to be performed)
  from the "modifier" options that merely supply file names or info about 
  variants on how to perform the operation.
- document which "options" are required for each operation, e.g. cert nickname
  is likely required for signing, but not for JAR signature verification.

The usage message should not cite urls from netscape.com any longer.

After reading the usage message, the reader should easily be able to 
- list the operations that the tool can perform,
- know which "options" are required for each of those operations.  

The existing usage message states that every command requires a positional
parameter which is a "directory-tree", but the -v and -w commands take an
an argument that is a jar file name, not a directory tree name.  So that
usage summary is wrong.  

If there are any commands that can operate from EITHER a jar file or a 
directory tree (or both) as input, this needs to be documented in the usage
somehow, along with how to specify one or the other, and which is default.  

Likewise for output.  Some commands produce a JAR file as output.  They
may also leave a (modified) directory tree as output.  The production of 
the output JAR file may be optional.  The Usage needs to make this clear. 

Finally, I will add that after reading the usage, the reader should be able
to answer the question of which command cause any JAR file contents to be 
uncompressed and extracted.

Comment 1

[Wan-Teh Chang]

The current location of the signtool documentation is
http://docs.sun.com/source/816-5531-10/app_sign.htm.

Ideally we should publish a copy of that page on our
own web site.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Wan-Teh, thanks for finding that documentation.  I'm glad Sun has kept a copy.
I agree that signtool should be documented on mozilla.org, but I would
not suggest that we simply copy that document to mozilla.org without revision.

While that old document is better than none, it's really inadequate and poorly
organized, IMO.  After reading it, I *still* cannot enumerate the different
operations that signtool is capable of performing.  And it is still not clear 
which operations expect (or allow) the input to be a directory-tree and which 
expect or allow the input to be a JAR/zip/XPI/war file.  

So, I think the research requested above needs to continue.  The cited sun
document should help with that.  

Comment 3

[Biswatosh Chakraborty]

Attachment: For review

(1)This uses certutil's usage format to some extent. 

(2)It also covers commands taken from a file. It uses http://docs.sun.com/source/816-5531-10/app_sign.htm
for that. 

(3)It attempts to cover all options.

(4)This shows command options separately and 
other options are listed separately as well.

(5)To see the usage, one can now run 
"signtool -h"
OR
"signtool -H".

Comment 4

[Christophe Ravel]

Could you add the usage message before and after your patch so we can compare the result more easily ?

Comment 5

[Biswatosh Chakraborty]

Attachment: Wan't to give this for review.

Neil,

Since Christophe asked me to attach the output of usage also, I assume
you have not started reviewing the first patch. Hence, I am taking
the liberty of slightly modifying the patch and give this for review
instead. This gives a little more usage help, in that, for each command
option, it also tells what are the other possible non-compulsory options.
The first patch did not contain that but after relooking at the patch
again I thought to add this feature as well. 
I am also going to attach now the usage output of the old version and 
this version(with this patch) of signtool.

Comment 6

[Biswatosh Chakraborty]

Attachment: The usage of signtool with the patch

Comment 7

[Biswatosh Chakraborty]

Attachment: The usage of the current signtool(that is without the patch)

Comment 8

[Biswatosh Chakraborty]

Comment #3 is applicable to this current patch (id=275094) as well.

Comment 9

[Neil Williams]

Comment on attachment 275094 [details] [diff] [review]
Wan't to give this for review.

I think this help message is too verbose. I suggest studying certutil's help messages. There are two forms. In the short form commands and their options are listed without comment:

   use signtool -H for more detail

signtool -l directory-name
signtool -L [ -d ] [ --outfile outputfile ]

enclose optional flags in square brackets; anything else is required for that command.

In the long form don't print "signtool" at the beginning of every line. Instead list the command flag and follow it with all of its options, one to a line, with a comment to the right. Please let me know if this isn't clear.

Comment 10

[Biswatosh Chakraborty]

Attachment: Patch2, This is mdelled on certutil's help

I am going now to attach the output also. I think 
reviewing that and suggesting changes based on it
will be easier than seeing this diff at first.

signtool -h will give the usage in short but 
signtool -H will give more detail. It is almost
like certutil's usage.

Comment 11

[Biswatosh Chakraborty]

Attachment: signtool's help output from patch2.

Patch2 (#284605), given in previous comment, produces this output.

Comment 12

[Julien Pierre]

Comment on attachment 284605 [details] [diff] [review]
Patch2, This is mdelled on certutil's help

The message format looks OK. However, I have never used signtool, and I am not certain that it is accurate. So I am asking Nelson for a second review.

Nit: "a XPInstall" should be "an XPInstlal". That seems to have been there in the old doc pages all along, but it's still wrong.

Comment 13

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 284605 [details] [diff] [review]
Patch2, This is mdelled on certutil's help


I mostly like this patch, but some of the usage lines are WAY WAY WAY too long.
The lines output in a usage message should not wrap the screen on an 80-column
window.  But the following examples clearly do.

   -j                          This option causes the specified directory to be 
signed and tags its entries as inline JavaScript
   -Z                          Creates a JAR file with the specified name. The 
-Z option cannot be used at the same time as the -J option
   -p                          Specifies a password for the private-key database
(insecure)
   --outfile filename          Specifies a file to receive redirected output
   --verbosity value           Sets the quantity of information signtool genera 
tes in operation
   --norecurse                 Blocks recursion into subdirectories
   --leavearc                  Retains the temporary .arc (archive) directories 
that the -J option creates

-J                   Signs a directory of HTML files containing JavaScript and 
creates as many archive files as are specified in the HTML tags
                               The options are same as without any command opti 
on, given above. The -Z option cannot be used at the same time as the -J option

-G nickname          Generates a new private-public key pair and corresponding 
object-signing certificate with the given nickname
   --keysize|-s keysize        Specifies the size of the key for generated cert 
ificate
   --token|-t token name       Specifies which available token should generate 
the key and receive the certificate
   --outfile filename          Specifies a file to receive redirected output

The above lines should appear more like this:

   -j                          This option causes the specified directory to be 
                               signed and tags its entries as inline JavaScript
   -Z                          Creates a JAR file with the specified name. The 
                               -Z option cannot be used at the same time as the 
                               -J option
   -p                          Specifies a password for the private-key 
                               database (insecure)
   --outfile filename          Specifies a file to receive redirected output
   --verbosity value           Sets the quantity of information signtool 
                               generates in operation
   --norecurse                 Blocks recursion into subdirectories
   --leavearc                  Retains the temporary .arc (archive) directories 
                               that the -J option creates

(etc)

In other words, make sure the output looks good in an 80 column window.
Please attach another patch and another sample output.

Comment 14

[Nelson Bolyard (seldom reads bugmail)]

Apparently the bugzilla web site wraps comment lines at 80 columns, but the
emails sent out by bugzilla wrap comment lines at 77 or 78 columns, so the 
email sent out for comment 13 looks TERRIBLE.  Please ignore that email
and look at the comment in the web page instead.   See 
https://bugzilla.mozilla.org/show_bug.cgi?id=301259#c13

Comment 15

[Biswatosh Chakraborty]

Attachment: Patch 3. Implements suggestions made in Comment #13

I made some lines, a few words shorter and now on 80 column
window it would not wrap the screen.
I am also going to attach a sample output in the next comment.

Since, this patch removes the help url as this was pointing
to sun's one and since netscape is not having it's own page
on signtool help, it is better we let the usage lines be a 
little long because our final aim is to help the user and 
reducing the lines in signtool command's usage message will
make it difficult for them.

Comment 16

[Biswatosh Chakraborty]

Attachment: Sample output for Patch3.

Comment 17

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 285993 [details]
Sample output for Patch3. 

This is looking much better than the previous sample output.  

I think the last section "example of use of command file" is still a little unclear because of a lack of visual separation between the sample lines and the meta comments about the sample lines.  Maybe you should indent the sample lines by a few columns to provide that separation.

Also, in sample lines, when you wrap them, you need to provide a trailing backslash to signify that the line is 
wrapped.

Comment 18

[Biswatosh Chakraborty]

Attachment: Patch 4.

1)Modified as suggested in Comment #17. 

2)Going to attach a sample output in the 
next comment.

3)Sample lines in the last section are now 
indented by a few columns and trailing backslash
given in the sample lines.

Comment 19

[Biswatosh Chakraborty]

Attachment: Sample output for patch 4

Comment 20

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 286246 [details] [diff] [review]
Patch 4.

Looks good to me.
r=nelson

Comment 21

[Biswatosh Chakraborty]

Checking to the trunk...

Checking in signtool.h;
/cvsroot/mozilla/security/nss/cmd/signtool/signtool.h,v  <--  signtool.h
new revision: 1.11; previous revision: 1.10
done

Checking in util.c;
/cvsroot/mozilla/security/nss/cmd/signtool/util.c,v  <--  util.c
new revision: 1.25; previous revision: 1.24
done

Checking in signtool.c;
/cvsroot/mozilla/security/nss/cmd/signtool/signtool.c,v  <--  signtool.c
new revision: 1.13; previous revision: 1.12
done

Comment 22

[Biswatosh Chakraborty]

Nelson,

The target milestone is 3.12. But, should it not also go to
the branch? Do I close this bug or give another patch
for superreview for the Branch?

Comment 23

[Biswatosh Chakraborty]

Closing this bug as the target was for 3.12 and patch has already 
been commited to the trunk.