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.
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.
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.
Created attachment 274900 [details] [diff] [review] 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".
Could you add the usage message before and after your patch so we can compare the result more easily ?
Created attachment 275094 [details] [diff] [review] 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 #3 is applicable to this current patch (id=275094) as well.
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.
Created attachment 284605 [details] [diff] [review] 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.
Created attachment 284607 [details] signtool's help output from patch2. Patch2 (#284605), given in previous comment, produces this output.
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.
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
Created attachment 285992 [details] [diff] [review] 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 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.
Created attachment 286246 [details] [diff] [review] 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 on attachment 286246 [details] [diff] [review] Patch 4. Looks good to me. r=nelson
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
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?
Closing this bug as the target was for 3.12 and patch has already been commited to the trunk.