----- [email protected] wrote: > 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.
Arguably, it's vague as to which dependency we should be retrieving. However, it looks like the previous code sorted versions in descending order instead of ascending, so I'll just add a reversed() to the iterators. > 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). I believe it is part of the fix for 17919, but the comment is still correct. However, I do need to call tracker.catalog_done() before returning. > 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. Some of this is historical. But one thing to keep in mind is Python's bizarre behaviour for globals. For Python, you only declare a variable as global if you intend to modify it. To simply use a global variable, you don't have to declare it as a global (screwy, I know). > 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. > Because of the cleanup code. Since the cleanup routine needs access to them in the event of a failure, it was much simpler to handle it this way. ... > 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. See above. > 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. The problem is that I don't have a good way of chunking the package versions. You're right that this is less efficient, but ultimately, this is a tradeoff to minimise space usage during transfers which has proved more important. 100 versions of osnet-incorporation with the same license file would indeed save some time if I placed all the files for the same package together, but 100 versions of openoffice might use multiple gigabytes of disk space. As such, it's simpler to avoid chunking for now. We can always work on improving efficiency later. Keep in mind that really only helps with the initial pkgrecv that's done. Once a user has done the initial seed of a repository, if they sync everytime a new build comes out, that logic wouldn't really help since you'd only be getting a single package version. -Shawn _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
