[email protected] wrote:
On Tue, Sep 01, 2009 at 05:36:22PM -0500, Shawn Walker wrote:
[email protected] wrote:
Perhaps. The fact that the testsuite leaves tempfile turds everwhere
continues to be a source of aggravation.  Would you comment the code to
state that you expect callers to rename this object if they want to keep
its contents?  I would really like it if you also added a __del__ method
that removes the tempfile if it exits, unrenamed, when the object is
destroyed.
I would note that since renaming the file is a necessary part of the operation, and this tempfile is created in the caller's specified directory (in this case, catalog directory), not a temp directory, I don't believe that's really necessary, but I'll still add it if you believe so.

If this must happen as part of the operation, I'm a bit less concerned.
What do we do if the caller's side of the operation fails?  My
expectation would be then that the caller cleans this up, but I don't
remember seeing that.

It does; see save() in CatalogPartBase. If there's an error when chmod()ing or rename()ing the file into place, it removes the temporary file.

I'm not asking you to do all of the work and I'm offering to help.  I
don't think it makes sense to putback code that's going to be used at
some future indeterminate time.  In particular, it would be bad if we
discovered that the eventual verification method didn't return correct
results when applied to historical manifest versions that contain these
signatures.  In order to ensure that this code works when it's putback,
and continues to work, we should verify the manifests against their
attached signatures.

Although Krister responded, to further clarify:

* There is a test case for manifest validation in t_manifest.py

* The manifest signature code included in this changeset is *solely* to get sha-1 digest (a degenerate signature as Bart put it) support so that in Phase III, the server can include that information in the catalog, and the client can in turn use that in transport to verify that a manifest was retrieved correctly.

Cheers,
--
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to