Danek Duvall wrote:
<elided comments accepted>
- line 914: Why are you treating arch specially from other variants?
I'm anticipating the fix for 6672; arch is normally filtered to prevent
bloat.
- line 488: This is a bit troublesome here -- we're marking the manifest
as having been loaded, but it's not actually usable yet, since its
contents haven't been filtered yet. Can that be an issue?
Note that if the excludes are set, we call set_content - which updates
the in-memory copy w/ the filtered set.
- line 494: you can use get_manifest_path() here, too. Scanning forward,
it seems you call this a lot -- why not cache the manifest path, and
possibly its dirname as well?
I've changed this to eliminate the use of the image reference. See webrev.
- line 498: would it ever be the case that manifest.dircache exists, but
the by-type manifests don't?
Only if someone deleted them; the dircache is the last one written.
- line 559: You should probably use mkstemp() here, just in case there
are battling processes.
Deferred; added XXX to revisit when we do locking correctly.
- line 583: Wow. That (the argument to join() doesn't do what I'd expect
it to do. I don't know if it's obvious to anyone else, but the %
expando operator binds more tightly than the for.
I've reformatted this for easier comprehension.
- line 615: Would it be any faster to put this code into the loop above?
Hmmm. this would require testing for an empty dir on every insert on
each bucket, rather than once per bucket, but it would save a traversal.
It's cleaner as it sits; I'll review this when I make this smarter.
- line 635: Since we're going to the trouble to compute the contents of
manifest.dircache, shouldn't we go ahead and store it (if we can), too?
Same goes for gen_actions_by_type().
Since we go ahead and create the manifest.dircache file if we can in
__init__, if the file doesn't exist here we can't create it; same w/
gen_actions_by_type().
- line 663: Here, you should probably be making the test based on the
actual type you're looking for, rather than dircache.
I can't do this, because not all packages have all types of actions.
All packages have required directories however (var var/sadm ...), so
manifest.dircache is always present. If manifest.dircache is there, and
manifest.typeiwant is missing, there are no typeiwant actions in the
manifest (or we have enemy action :-/).
- Bart
--
Bart Smaalders Solaris Kernel Performance
[email protected] http://blogs.sun.com/barts
"You will contribute more with mercurial than with thunderbird."
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
