[RFC] TreeTransform fooled by os.rename on case insensitive filesystems

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Vincent Ladeuil wrote:
>>>>>> "aaron" == Aaron Bentley <aaron at aaronbentley.com> writes:

> Roughly speaking, in that test, we rely on _FileMover via
> TreeTransform to detect a name clash when the file system is case
> insensitive.

Actually, that test is about making sure that if renaming a file fails
during a TreeTransform, we roll back properly.  It doesn't really have
anything to do with case sensitivity.  I should not have used
CaseInsensitiveFilesystemFeature for this.

> The problem is that we trust a case insensitive fs to raise
> FileExists and that doesn't work on HFS+.
> 
> I think that this problem should be detected *before* we use the
> _FileMover, may be in tt.new_file() ?

Too early.  TreeTransforms are supposed to permit a malformed transform
to be configured.  Then a resolver is used to fix filesystem conflicts,
so that the transform can be applied.

> You mean that, in the following:
> 
>                 tt.new_file('foo', tt.root, 'bar')
>                 tt.new_file('Foo', tt.root, 'spam')
>                 # Lie to tt that we've already resolved all conflicts.
>                 tt.apply(no_conflicts=True)
> 
> we will never lie like that ?

Right.  Lying like that in production code would be a terrible idea.
There are places where we use no_conflicts=True in production code, but
it not a lie in those places.

>     aaron> This test is testing extreme conditions.  I don't
>     aaron> consider it a problem if TreeTransform fails under
>     aaron> those conditions.
> 
> So you agree with John about making that test a XFAIL on OSX ?

No, I think we should make it pass on OSX.

> The problem I was referring to is that
> CaseInsensitiveFilesystemFeature does not deserve its name then
> and neither is the '_case_sensitive_target' attribute of
> TreeTransformBase. That just add to confusion.

Could you please explain why not?  AFAIK, they are both accurate.

>     aaron> We already look before we leap; it's called conflict
>     aaron> detection.  We don't need to replicate it.  Conflict
>     aaron> detection and resolution is always done immediately
>     aaron> before renaming files.
> 
> Can you kindly show me where it's done in that test or is that
> conflict detection tricked by the no_conflicts=True ?

It's tricked by the no_conflicts=True.

> If that's the case, should we really keep that test ?

Yes.  It's making sure that rollbacks work.

>     aaron> Just change it into a file type that we can guarantee
>     aaron> won't allow rename-on-top.  i.e. a directory.
> 
> EPARSE_ERROR

> 
> You mean forcing TT to create a directory for every file ?

Well, what I really mean is changing the test so that it tries to rename
a directory on top of another directory.  I believe this causes failures
on every platform.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFIvsPK0F+nu1YWqI0RAsR/AJ4+ZqODS3bnXLgxmFhgjmiuPvyQhwCfX3+d
bkLAgRYhDnKqF74lGfhNpz4=
=dot1
-----END PGP SIGNATURE-----