On 04/11/11 06:05 PM, Shawn Walker wrote:
Greetings,
The following webrev contains fixes for the following issues:
17741 pkgrecv should allow wildcards in package versions
17846 pkgrecv package matching should be the same as pkg
17879 pkgrecv(1) should discard package cache files after each
package is republished
17919 occasional failure in TestPkgrecvMulti.test_8_archive
17920 pkgrecv can traceback without returning 99
17938 pkgrecv doesn't catch traceback when out of storage space
webrev:
http://cr.opensolaris.org/~swalker/pkg-recv/
-Shawn
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
pull.py:
227-231: If I'm reading this code right, i think it returns the version
which matches and is the smallest, not the largest. It seems strange to
me that when going down the dependencies of packages, we'd pick the
lowest version, instead of the highest which matched the dependency stated.
Should the comments on line 323-326 change? It seems like we're no
longer assuming a catalog doesn't exist. (I'm also not really sure which
of the above bugs this change is fixing).
General comment 333-334: I'm confused as to why some things are globals,
but sometimes they're passing into functions, but sometimes they're not.
For example xport_cfg is a global, but passed as an argument to several
functions (assuming that horrible name collisions aren't going on).
src_cat seems to be treated as a global, but in several functions, it's
tagged w/ global inside the function, but not in expand_fmri. If i'm
following along correctly, it seems to me that it shouldn't be tagged w/
global in fetch_catalog either as it's not being assigned to there.
I also don't get why target and archive became globals now. W/ all the
globals, this code has a different look/feel/style than most of our
other code, so I wasn't thrilled to see more items added to the list of
globals.
458: get_matches could use a docstring.
464: I think this comment makes more sense above the if, as it's talking
about what the if statement's attempting to accomplish, not what as been
accomplished by entering the true branch of the if. Positioned where it
is now, I found it actively misleading.
647: If this src_cat is the global src_cat, then shouldn't there be a
global tag for it in this function? And if it's not the same src_cat as
the global src_cat, please rename it.
general comment about transfer_pkgs: This function is long enough that I
found it hard to navigate at times.
One thing I wonder about in lines 854-861. It looks to me like you're
clearing the cache after each version (down to timestamp) of each
package is downloaded. I wonder whether it wouldn't be better to clear
the cache after you process all the versions of a single fmri. I think
especially if you're doing all versions or all timestamps of a package,
this could really cut down on the amount of data transferred over the
wire since packages, especially packages with the same version but
different timestamps, probably share a large amount of content, while
the sharing between packages seems likely to be significantly less. I
think it would just be a matter of grouping the matches by their fmri.
Thanks,
Brock
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
