Danek Duvall wrote: > On Wed, Nov 12, 2008 at 06:10:24PM -0800, Brock Pytlik wrote: > > >> http://cr.opensolaris.org/~bpytlik/ips-4886-v1/ >> > > image.py: > > - line 180: what does this guy intended to do? > Relic, explained more below > - line 1451: wow, this is a bit dense. There are some things in here > that don't make complete sense to me, but I'll try to rephrase it > anyway. > > try: > mod_time = os.stat(cache_file).st_mtime > except EnvironmentError, e: > if e.errno == errno.ENOENT: > mod_time = None > else: > raise > > if not self._catalog or self._cache_reread or \ > mod_time != self._catalog_cache_mod_time: > try: > load_pkl() > except blah: > raise Runtime > > # This should come after the attempt to load the pickle; > # otherwise the modtime isn't always going to be a good test. > self._catalog_cache_mod_time = mod_time > > I think you captured the basic intent. One thing is I'm pretty sure about though is that self._catalog_cache_mod_time should be set before the file is read, otherwise in (extremely perverse and adversarial situations) you could up with a self._ccmt of a file newer than had been read into memory. It's better to read the file twice unnecessarily than to not read it again when needed. > I'm still not sure I understand how you're using _cache_reread. It's > only set to True right after you've written the cache, but there's no > point in reading it back in at that point, since what's on disk matches > what's in memory -- unless the modtime has changed. Am I confused? > Basically, it was a relic from before I understood what was going on with the difference between catalog.pkl and self._catalog with regards to catalogless but installed packages. I believe now that I understand things better, it's not necessary at all. I've pulled it and am rerunning the testsuite now. > Danek > Danek Duvall wrote: > On Wed, Nov 12, 2008 at 06:10:24PM -0800, Brock Pytlik wrote: > > >> http://cr.opensolaris.org/~bpytlik/ips-4886-v1/ >> > > Whoops, forgot the test. > > Are the test changes related to 4886? > Yes, the idea is to change the catalog.pkl file out from under the existing api object and make sure the info is accurately updated. I realized after you asked about them, that a better test would be to create a separate api object to do the installation. I'm running with these changes now. > line 171, 172: do you do anything with these values? > > No, I can pull them. > Danek
Thanks for taking a look at these so quickly. I'll be putting out a new webrev once I make the changes recommended and make sure it passes the test suite. Brock _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
