Shawn Walker wrote: > On 03/ 2/10 12:14 PM, Danek Duvall wrote: > >Shawn Walker wrote: > > > >>On 03/ 1/10 07:05 PM, Danek Duvall wrote: > >>>Shawn Walker wrote: > >>> > >>>>http://cr.opensolaris.org/~swalker/pkg-529/ > >>> > >>> - if makedirs() blows up not because the leaf directory isn't a > >>> directory, but because something up the chain isn't, what happens? > >> > >>Probably not what I want to happen. I could compare e.filename to path > >>at the beginning and simply re-raise immediately if it doesn't match? > >>(Could do that anywhere we call makedirs in the actions I'm changing > >>really...). > > > >You could do it in makedirs itself, too, no? > > Add the EBUSY, EEXIST, etc. error handling I have in the actions > themselves? Sorry for getting lost here.
Raise the operations error or whatever directly from makedirs. I'm not sure whether that would actually end up being useful, though. > >So the answer to your question, roughly, is "we want both". > > If we do the check in preinstall(), I assume the reason for having > the check again in install() is to ensure that some package in the > plan hasn't broken things since we first checked? Exactly. The situation on disk may change behind our backs at any moment, and we need to be prepared for it. > >>>file.py: > >>... > >>> - line 437: why the islink() test? > >> > >>Because os.path.isdir() will return true if it's a link pointing at a > >>directory. In that case, the error for unlink is saying that the > >>user doesn't have permission to remove the link. > > > >Okay. That shouldn't be an issue if you just do the lstat() once and look > >at the results of S_ISDIR() and S_ISLNK(). > > Note that lines 145-147 in install() have this islink() check too for > the same reason (from what I can tell). Perhaps. That's Brad's code from the original "fix" implementation. Still, there's little point in calling stat so often. > >Note that if os.path.isdir() calls stat() instead of lstat(), then you're > >using it incorrectly to make the decision on line 2512 of image.py. There > >may be other places this is happening, too. > > So are you saying that anywhere we call islink() and isdir() both we > should just do a single stat instead? > > I wonder if we should have our own isdir() in portable instead to > simplify this for cases where the only stat check we need is that... It's not a portability issue, AFAIK. I'm just suggesting that you call os.lstat() once, and do the tests on the result of that -- stat.S_ISLNK(st.st_mode) and so on. Danek _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
