Re: [Monotone-devel] Filesystem normalisation

[Peter Stirling]

On 29 Nov 2008, at 11:48 am, Stephen Leake wrote:

> Peter Stirling <address@hidden> writes:
>
> > I've written a small module (it's one 'public' function and some
> > support functions to make it clearer as to what it's doing) that
> > solves the 'mtn add apple Apple' on Mac OS X.
>
> What exactly is the problem? Is Mac OS X file system case insensitive?

It is insensitive by default, and a number of programs won't work
if you set it to case-sensitive, so the vast majority of people
will not set it that way. According to
http://www.venge.net/mtn-wiki/CaseInsensitiveFilesystems
case insensitivity is also an issue on windows.

Also, HFS+ stores files in NFD utf-16 (it makes case insensitive
sorting/searching much quicker), this means that you will also have
issues with filenames entered as precomposed unicode (and my patch
covers that case too).

> > The idea is to have a function
> >         void plaform_fs_normalisation_hook( std::string & const in,
> > std::string & out )
> > that can be called from
> >         args_to_paths() (cmd.hh:146 on nvm
> > 93e7e626c6ee8db33a21eabcfab00d97f1bed8c2).
>
> This is what paths::to_external is supposed to do, mostly. Does that
> not work here?
>
> On the other hand, args should already be in the fs format, so I don't
> understand why you need to do anything.
>
> "paths" in mtn are supposed to be in a filesystem-neutral internal
> format; to_external converts them to the current filesystem format.

grep -r to_external . returns nothing, I assume that you meant

        string
        any_path::as_external() const
        {
        #ifdef __APPLE__
	  // on OS X paths for the filesystem/kernel are UTF-8 encoded,  
regardless of
          // locale.
          return data;
        #else
          // on normal systems we actually have some work to do, alas.
	  // not much, though, because utf8_to_system_string does all the  
hard work.
          // it is carefully optimized.  do not screw it up.
          external out;
          utf8_to_system_strict(utf8(data), out);
          return out();
        #endif
        }

(paths.cc:639)

Which is, as should be obvious, not all that useful. as_external()
wasn't anywhere obvious in normalise_external_path(), so even if
it did the right thing you could still add the same file several
times.

To demonstrate what I'm trying to solve:

# make a new scratch directory
##############
g5:test_paths peter$ mkdir temp
g5:test_paths peter$ cd temp

# create a new db and workspace
##############
g5:temp peter$ mtn db init --db=temp.mtn
g5:temp peter$ mtn setup --db=temp.mtn --branch=temp temp
g5:temp peter$ cd temp

# create a file add it to the workspace twice with
# the same name and verify that both names refer to the same contents
##############
g5:temp peter$ echo foo > apple
g5:temp peter$ mtn add apple APPLE
mtn: adding APPLE to workspace manifest
mtn: adding apple to workspace manifest
g5:temp peter$ mtn diff
#
# old_revision []
#
# add_dir ""
#
# add_file "APPLE"
#  content [f1d2d2f924e986ac86fdf7b36c94bcdf32beec15]
#
# add_file "apple"
#  content [f1d2d2f924e986ac86fdf7b36c94bcdf32beec15]
#
============================================================
--- APPLE       f1d2d2f924e986ac86fdf7b36c94bcdf32beec15
+++ APPLE       f1d2d2f924e986ac86fdf7b36c94bcdf32beec15
@@ -0,0 +1 @@
+foo
============================================================
--- apple       f1d2d2f924e986ac86fdf7b36c94bcdf32beec15
+++ apple       f1d2d2f924e986ac86fdf7b36c94bcdf32beec15
@@ -0,0 +1 @@
+foo
g5:temp peter$ mtn commit -m 'temp'
mtn: beginning commit on branch 'temp'
mtn: committed revision c28046695cac17a722edf6a591d5f52e13d331b7

# create a dummy child revision to demonstrate that we can't update
# to the old revision
##############
g5:temp peter$ echo bar > apple
g5:temp peter$ mtn diff
#
# old_revision [c28046695cac17a722edf6a591d5f52e13d331b7]
#
# patch "APPLE"
#  from [f1d2d2f924e986ac86fdf7b36c94bcdf32beec15]
#    to [e242ed3bffccdf271b7fbaf34ed72d089537b42f]
#
# patch "apple"
#  from [f1d2d2f924e986ac86fdf7b36c94bcdf32beec15]
#    to [e242ed3bffccdf271b7fbaf34ed72d089537b42f]
#
============================================================
--- APPLE       f1d2d2f924e986ac86fdf7b36c94bcdf32beec15
+++ APPLE       e242ed3bffccdf271b7fbaf34ed72d089537b42f
@@ -1 +1 @@
-foo
+bar
============================================================
--- apple       f1d2d2f924e986ac86fdf7b36c94bcdf32beec15
+++ apple       e242ed3bffccdf271b7fbaf34ed72d089537b42f
@@ -1 +1 @@
-foo
+bar
g5:temp peter$ mtn commit -m temp2
mtn: beginning commit on branch 'temp'
mtn: committed revision 84ae9cb7bcfcd84343993a66f09a60629075922b
g5:temp peter$ mtn update -r p:
mtn: expanding selection 'p:'
mtn: expanded to 'c28046695cac17a722edf6a591d5f52e13d331b7'
mtn: selected update target c28046695cac17a722edf6a591d5f52e13d331b7
mtn: [left]  6d7859995aaff97d24086a456d0d9e5cc22c44d4
mtn: [right] c28046695cac17a722edf6a591d5f52e13d331b7
mtn: modifying APPLE
mtn: error: content of file 'apple' has changed, not overwriting

# since that didn't work, let's try checking it out into a new
# workspace
##############
g5:temp peter$ cd ..
g5:temp peter$ mtn co --db=temp.mtn --branch=temp temp2
mtn: misuse: rename target 'apple' already exists

# nuke from orbit
##############
g5:temp peter$ cd ..
g5:test_paths peter$ rm -r temp

> > If this code is eventually included in monotone, then it would be
> > possible to identify 2 files in a manifest that will conflict on
> > disk,
>
> This is done now, although not in a very friendly way.

As you can read above, there's no hint of friendliness
in the diagnostic

> > and perhaps perform an automatic rename.
>
> That would be conflict resolution for 'mtn update', which is a logical
> step.
>
> > Currently it is appears impossible to checkout a revision that has
> > conflicting filenames.
>
> yes, if they conflict according to the local filesystem.

And you don't think this is a problem?

> > The update aborts when it notices that a file with the second
> > filename already exists, leaving the workspace with a _MTN/ detached
>
> How does your code solve that?

I didn't say that it did that, I said that it could be done later,
if this code gets into the monotone tree. It seemed silly to build
castles in the sky before obtaining a positive response.

It may just have been a side-effect of the point-by-point examination,
but your email came across as quite hostile. I'm only trying to
improve a defect that affects OS X, which has the potential as
a start toward windows too. To that end, can someone point me as to
where I should change the build configuration scripts to add
this file, and the required linker argument? I need that so that I
can check whether it works (ie hook in the right place), and
hopefully write some tests to show that it works.