[PATCH 1 of 2 STABLE] opener: raise IOError on paths ending in '/'

[Adrian Buehlmann]

On 31.10.2010 11:33, Benoit Boissinot wrote:
> On Sun, Oct 31, 2010 at 8:32 AM, Adrian Buehlmann <adrian at cadifra.com> wrote:
>> # HG changeset patch
>> # User Adrian Buehlmann <adrian at cadifra.com>
>> # Date 1288433569 -7200
>> # Branch stable
>> # Node ID cd9700552b22bf12b375fc48a582fd96da721145
>> # Parent  15ca4bfecfe343cbf53210b19c081676b2a35d3f
>> opener: raise IOError on paths ending in '/'
>>
>> It's also better not to call nlinks() for such malformed paths, so
>> let's move the path test out of the OSError exception handler.
>>
> 
> I'm not sure that's the fix I prefer, why not just:
> 
> if not basename:
>     raise

Mostly, because Martin already inserted code with 1634287b6ab1 which
depends on IOError being thrown, which makes sense, since a plain
open('foo/') will throw IOError as well and not OSError.

  if not basename:
      raise

would throw OSError -- assuming you mean keeping that inside the 'except
OSError:' catch handler (which I don't with this patch here).

> In the exception handler to re-raise it? (that way it would raise
> ENOENT when approriate, and EISDIR otherwise.

I'd say EISDIR is always correct if someone tries to open a file with
open('foo/'). Not sure what other cases you mean.

My patch only raises IOError(errno.EISDIR, ..) if basename is empty,
which only happens if the path ends with a path sep (assuming we can
ignore f being empty).

>> As another nice side effect, this patch also fixes a platform dependent
>> test output variation that happens to have been introduced with 551aa6e27929
>> (551aa6e27929 causes test-mq-qnew.t to fail on Mac OS X, as reported
>> by Steve Borho).
> 
> Do you have a link to the bug report?

http://selenic.com/pipermail/mercurial-devel/2010-October/025754.html

BTW, if this patch here turns out to be controversial, I can resend the
hardlink blindness detection patch separately.

I really don't care that much about this patch here. It just happens to
be in the way of the other patch.