On Tue, Feb 03, 2009 at 04:48:44PM -0800, [email protected] wrote: > Thanks for all of the reviews, but I jumped the gun. The first fix > didn't pass the test suite. I've re-whacked this so it works correctly. > The new fix is equally concise, but actually passes the test suite. > I updated the webrev in place this time, since the whole thing is so > small.
So from what I can tell, the problem was that you weren't regenerating the catalog cache with the removed catalog. That seems to be happening in the rewrite, but I don't think you're doing it in a terribly clear way. You're relying on the behavior of cache_catalogs() to ignore a missing catalog, and, like Shawn said, you're relying on origin_changed being True, which makes no sense. It's like you're shoehorning multiple meanings into the variable just to make the code more compact. I think it would be less confusing if you went back to your original change, and had a separate check for disabled down either between line 753 and 754 (putting newauth into the auths dict and the save_config() call), or after the latter, re-caching the catalog there. Danek _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
