Closed
Bug 301259
Opened 19 years ago
Closed 17 years ago
signtool Usage message is unhelpful
Categories
(NSS :: Tools, defect, P2)
Tracking
(Not tracked)
RESOLVED
FIXED
3.12
People
(Reporter: nelson, Assigned: biswatosh2001)
Details
Attachments
(2 files, 8 obsolete files)
26.68 KB,
patch
|
nelson
:
review+
|
Details | Diff | Splinter Review |
7.32 KB,
text/plain
|
Details |
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•19 years ago
|
||
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.
Reporter | ||
Comment 2•19 years ago
|
||
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.
Reporter | ||
Updated•18 years ago
|
QA Contact: jason.m.reid → tools
Reporter | ||
Updated•18 years ago
|
Priority: -- → P2
Target Milestone: --- → 3.12
Updated•17 years ago
|
Assignee: neil.williams → biswatosh.chakraborty
Assignee | ||
Comment 3•17 years ago
|
||
(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".
Attachment #274900 -
Flags: review?(neil.williams)
Comment 4•17 years ago
|
||
Could you add the usage message before and after your patch so we can compare the result more easily ?
Assignee | ||
Comment 5•17 years ago
|
||
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.
Attachment #274900 -
Attachment is obsolete: true
Attachment #275094 -
Flags: review?(neil.williams)
Attachment #274900 -
Flags: review?(neil.williams)
Assignee | ||
Comment 6•17 years ago
|
||
Assignee | ||
Comment 7•17 years ago
|
||
Assignee | ||
Updated•17 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 8•17 years ago
|
||
Comment #3 is applicable to this current patch (id=275094) as well.
Comment 9•17 years ago
|
||
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.
Attachment #275094 -
Flags: review?(neil.williams) → review-
Assignee | ||
Comment 10•17 years ago
|
||
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.
Attachment #275094 -
Attachment is obsolete: true
Attachment #275095 -
Attachment is obsolete: true
Attachment #275097 -
Attachment is obsolete: true
Attachment #284605 -
Flags: review?(julien.pierre.boogz)
Assignee | ||
Comment 11•17 years ago
|
||
Patch2 (#284605), given in previous comment, produces this output.
Comment 12•17 years ago
|
||
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.
Attachment #284605 -
Flags: superreview?(nelson)
Attachment #284605 -
Flags: review?(julien.pierre.boogz)
Attachment #284605 -
Flags: review+
Reporter | ||
Comment 13•17 years ago
|
||
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.
Attachment #284605 -
Flags: superreview?(nelson) → superreview-
Reporter | ||
Comment 14•17 years ago
|
||
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
Assignee | ||
Comment 15•17 years ago
|
||
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.
Attachment #284605 -
Attachment is obsolete: true
Attachment #284607 -
Attachment is obsolete: true
Attachment #285992 -
Flags: review?(nelson)
Assignee | ||
Comment 16•17 years ago
|
||
Assignee | ||
Updated•17 years ago
|
Attachment #285993 -
Attachment mime type: application/octet-stream → text/plain
Reporter | ||
Comment 17•17 years ago
|
||
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.
Attachment #285993 -
Flags: review-
Assignee | ||
Comment 18•17 years ago
|
||
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.
Attachment #285992 -
Attachment is obsolete: true
Attachment #285993 -
Attachment is obsolete: true
Attachment #286246 -
Flags: review?(nelson)
Attachment #285992 -
Flags: review?(nelson)
Assignee | ||
Comment 19•17 years ago
|
||
Reporter | ||
Comment 20•17 years ago
|
||
Comment on attachment 286246 [details] [diff] [review] Patch 4. Looks good to me. r=nelson
Attachment #286246 -
Flags: review?(nelson) → review+
Assignee | ||
Comment 21•17 years ago
|
||
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
Assignee | ||
Comment 22•17 years ago
|
||
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?
Assignee | ||
Comment 23•17 years ago
|
||
Closing this bug as the target was for 3.12 and patch has already been commited to the trunk.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•