Danek Duvall wrote:
> On Wed, Nov 12, 2008 at 09:18:35PM -0800, Brock Pytlik wrote:
>
>
>>> - 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.
>>
>
> So the way I have it above, the modtime is read prior to reading the file,
> which means that it could be out of date by the time the file is read,
> leading to a second reading -- erroring on the side of safety.
>
> Caching that modtime, though, if done before we read the file and fail,
> means that the time we cache is potentially later than the copy we have in
> memory, meaning we won't try to reread the file next time through, even
> though we need to.
>
> To be ultra-paranoid, though, you could set self._catalog = {} in the
> except clause (and even _ccmt to None there, too), essentially forcing it
> to be retried next time through.
>
> Here's one error, though, w.r.t. the out-of-catalog packages -- if we error
> out in load_catalog_cache(), that information never gets loaded. Something
> to think about for later.
>
You're right of course. Thanks for using a wiffle bat rather than a
baseball bat to knock some sense into me ;) I've also tried pulling the
out-of-catalog package processing up a level so that it always happens
when you load_catalogs (I think we want it to right?). If that's too big
of a change at this point, I'm happy to move it back, just thought the
code was more right this way.
>
>>> 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.
>>
>
> Okay. Please be sure to comment what you're doing.
>
I've changed the tests again since the catalog.pkl files doesn't (seem?)
to track the installed status of packages and added comments
New webrev at:
http://cr.opensolaris.org/~bpytlik/ips-4886-v2/
> Danek
>
Thanks again,
Brock
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
