[PATCH] Allow manipulating files with long names on Windows

[Mads Kiilerich]

Aaron Cohen wrote, On 01/20/2011 11:06 PM:
>> Such history might be more appropriate in an introduction email than in the
>> changeset description.
> Yes, but I'm actually not sure how to do the introductory email
> correctly at all. I'm new to the patchbomb extension. I guess I'm
> supposed to use --intro and then compose the message, and end up with
> a [0/1] email? I've been fruitlessly looking for a way to send just
> the patch with introduction before "---".

Yes, "hg email" has an awful lot of options, but no options for that.

>   I also haven't been able to
> get "versioned patches" going the way I'd like. I was expecting
> --flagV3 to produce a subject like [PATCH][V3], but it does not.

--flag V3 will give a subject like [PATCH V3].+ [extensions]

>>> +# vanilla os.listdir handles UNC ok, but breaks if they're longer than
>>> MAX_PATH
>> Use docstring instead of comment.
> Won't that replace the docstring for listdir with mine? If so do I
> need to copy all the original docstring too? Maybe I need to do that
> anyway.

Right, good point. But I don't think the docstring is relevant on 
runtime. It matters more that the source code is readable.

>>> +# vanilla loses a trailing backslash:
>> Why is that a problem?
> If you do os.path.join(os.path.split("\\\\?\\C:\\"), "foo") you end up
> with \\?\C:foo which is invalid. Same problem with dirname.

Ok, so the problem is that the original function doesn't work correctly 
with UNC paths.

>> But it seems like a bad idea that chdir doesn't do a chdir. That will most
>> likely have unexpected consequences. Wouldn't it be better to just fail if
>> cwd is too long?
> That would be possible I guess, there aren't that many places in hg
> where os.chdir is used, and it's almost always to repo.root. The most
> important usages seem to be in the record extension and the patch
> command.
>
> I originally just documented that the root of the repo shouldn't be
> more than 244 characters long, but figured I'd give it my best shot to
> fix it "right".

FWIW I think it would be more right to just give a nice error message.

I guess that fixing it right would involve expanding relative paths in 
calls to for example os.open with absolute paths starting with the 
latest chdir value.

>>> +    if hasattr(util, "unlinkpath"):
>>> +        util.unlinkpath = wrap(util.unlinkpath)
>>> +    if hasattr(util, "unlink"):
>>> +        util.unlink = wrap(util.unlink)
>> This invasive monkey patching looks a bit scary and fragile and might make
>> it less easy for it to become an "official" extension.
> This is actually pretty much identical to what win32mbcs patches, just
> written out directly instead of with a fancy loop. I find this easier
> to read, but could switch to a style like mbcs is using. In fact,
> anywhere win32mbcs isn't patching a function I patched, I'd bet they
> have a bug. (util.makedirs for instance).

Yes, your way of doing it isn't any worse than win32mbcs - and I doubt 
there are any more elegant solutions to this ugly problem at all. But 
having two different implementations where one of them is buggy is also 
ugly. I don't know how that could be solved.

>  From a quick readthrough, I would expect the two extensions to
> actually be able to coexist, but only if mine loads first.

So it would make sense to fail if win32mbcs has been loaded first?

>>> +def list(ui, repo):
>>> +    for root, _ignored, files in os.walk(repo.root):
>>> +        for file in files:
>>> +            if len(root + file)>= 259:
>> MAXPATH - 1?
>>
>> "contrib/check-code.py hgext/win32lfn.py" will complain here.
> Hmm, not for me. I've been running that actually. The only complaint
> it has is about one line that's too long, that I deliberately left in
> because it's a URL. I can split that line if desired. I'll replace the
> magic number though.

Sorry. My mailer lied to me. There is no problem.

>> Finally:
>>
>> Many fine extensions have their own life and isn't distributed with
>> Mercurial. That has the advantage that they can support multiple Mercurial
>> versions (if they can) and they can use their own release and bugfix
>> schedule.
> Yes, I hear you. I do consider it an unfortunate weakness of Mercurial
> that it can have repositories that it's unable to checkout on Windows.
> And it's a regression if someone is moving from SVN, because
> TortoiseSVN at least is able to check out these same projects.

I don't think anybody disagree with that - _if_ it can be done 
sufficiently efficient, reliably and elegant.

If your approach works fine then it might even be better to do it 
directly in core Mercurial than in an extension.

(I think more people are annoyed by the interoperability problems 
between for example UCS16 based and UTF8 based systems with unicode file 
names. Fixing that is however more controversial, but I think 
compatibility with fixutf8 is important for your audience.)

>> Extensions might be accepted in Mercurial if they have proven that they are
>> stable and widely used and actively maintained.
>>
>> I suggest you publish this extension somewhere (for example on bitbucket)
>> and add it to http://mercurial.selenic.com/wiki/UsingExtensions . Time will
>> tell if it would be better to distribute it with Mercurial.
> Alright

If you do that you don't have to listen to any stupid review comments ;-)

/Mads