Put a workaround in the directory special remote for strange behavior with VFAT filesystems on Linux (mounted with shortname=mixed)

3 files changed, 76 insertions, 27 deletions

diff --git a/Remote/Directory.hs b/Remote/Directory.hs
index b592f41ff..cadd5e759 100644
--- a/Remote/Directory.hs
+++ b/Remote/Directory.hs
@@ -62,56 +62,81 @@ directorySetup u c = do
 	gitConfigSpecialRemote u c' "directory" dir
 	return $ M.delete "directory" c'
 
-dirKey :: FilePath -> Key -> FilePath
-dirKey d k = d </> hashDirMixed k </> f </> f
+{- Where to store a given Key in the Directory.
+ -
+ - There are two possible locations to try; this had to be done because
+ - on Linux, vfat filesystem mounted with shortname=mixed have a
+ - variant of case insensativity that causes miserable failure when
+ - hashDirMixed produces eg, "xx" and "XX". The first directory to be
+ - created wins the namespace, and the second one cannot then be created.
+ - But unlike behavior with shortname=lower, "XX/foo" won't look in
+ - "xx/foo".
+ -}
+locations :: FilePath -> Key -> [FilePath]
+locations d k = [using hashDirMixed, using hashDirLower]
 	where
+		using h = d </> h k </> f </> f
 		f = keyFile k
 
+withCheckedFile :: (FilePath -> IO Bool) -> FilePath -> Key -> (FilePath -> IO Bool) -> IO Bool
+withCheckedFile _ [] _ _ = return False
+withCheckedFile check d k a = go $ locations d k
+	where
+		go [] = return False
+		go (f:fs) = do
+			use <- check f
+			if use
+				then a f
+				else go fs
+
+withStoredFile :: FilePath -> Key -> (FilePath -> IO Bool) -> IO Bool
+withStoredFile = withCheckedFile doesFileExist
+
 store :: FilePath -> Key -> Annex Bool
 store d k = do
 	src <- fromRepo $ gitAnnexLocation k
-	let dest = dirKey d k
-	liftIO $ catchBoolIO $ storeHelper dest $ copyFileExternal src dest
+	liftIO $ catchBoolIO $ storeHelper d k $ copyFileExternal src
 
 storeEncrypted :: FilePath -> (Cipher, Key) -> Key -> Annex Bool
 storeEncrypted d (cipher, enck) k = do
 	src <- fromRepo $ gitAnnexLocation k
-	let dest = dirKey d enck
-	liftIO $ catchBoolIO $ storeHelper dest $ encrypt src dest
+	liftIO $ catchBoolIO $ storeHelper d enck $ encrypt src
 	where
 		encrypt src dest = do
 			withEncryptedContent cipher (L.readFile src) $ L.writeFile dest
 			return True
 
-storeHelper :: FilePath -> IO Bool -> IO Bool
-storeHelper dest a = do
-	let dir = parentDir dest
-	createDirectoryIfMissing True dir
-	allowWrite dir	
-	ok <- a
-	when ok $ do
-		preventWrite dest
-		preventWrite dir
-	return ok
+storeHelper :: FilePath -> Key -> (FilePath -> IO Bool) -> IO Bool
+storeHelper d key a = withCheckedFile check d key go
+	where
+		check dest = isJust <$> mkdir (parentDir dest)
+		mkdir = catchMaybeIO . createDirectoryIfMissing True
+		go dest = do
+			let dir = parentDir dest
+			allowWrite dir
+			ok <- a dest
+			when ok $ do
+				preventWrite dest
+				preventWrite dir
+			return ok
 
 retrieve :: FilePath -> Key -> FilePath -> Annex Bool
-retrieve d k f = liftIO $ copyFileExternal (dirKey d k) f
+retrieve d k f = liftIO $ withStoredFile d k $ \file -> copyFileExternal file f
 
 retrieveEncrypted :: FilePath -> (Cipher, Key) -> FilePath -> Annex Bool
 retrieveEncrypted d (cipher, enck) f =
-	liftIO $ catchBoolIO $ do
-		withDecryptedContent cipher (L.readFile (dirKey d enck)) $ L.writeFile f
+	liftIO $ withStoredFile d enck $ \file -> catchBoolIO $ do
+		withDecryptedContent cipher (L.readFile file) $ L.writeFile f
 		return True
 
 remove :: FilePath -> Key -> Annex Bool
-remove d k = liftIO $ catchBoolIO $ do
+remove d k = liftIO $ withStoredFile d k $ \file -> catchBoolIO $ do
+	let dir = parentDir file
 	allowWrite dir
 	removeFile file
 	removeDirectory dir
 	return True
-	where
-		file = dirKey d k
-		dir = parentDir file
 
 checkPresent :: FilePath -> Key -> Annex (Either String Bool)
-checkPresent d k = liftIO $ catchMsgIO $ doesFileExist (dirKey d k)
+checkPresent d k = liftIO $ catchMsgIO $ withStoredFile d k $
+	const $ return True -- withStoredFile checked that it exists
diff --git a/debian/changelog b/debian/changelog
index 185ffeceb..e043f6c9f 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+git-annex (3.20111123) UNRELEASED; urgency=low
+
+  * Put a workaround in the directory special remote for strange behavior
+    with VFAT filesystems on Linux (mounted with shortname=mixed)
+
+ -- Joey Hess <joeyh@debian.org>  Tue, 22 Nov 2011 17:53:42 -0400
+
 git-annex (3.20111122) unstable; urgency=low
 
   * merge: Improve commit messages to mention what was merged.
diff --git a/doc/bugs/directory_remote_and_case_sensitivity_on_FAT.mdwn b/doc/bugs/directory_remote_and_case_sensitivity_on_FAT.mdwn
index b686304e5..ae653d619 100644
--- a/doc/bugs/directory_remote_and_case_sensitivity_on_FAT.mdwn
+++ b/doc/bugs/directory_remote_and_case_sensitivity_on_FAT.mdwn
@@ -18,11 +18,28 @@ I wonder if the directory remote should use hashDirLower instead of hashDirMixed
 
 > git-annex intentionally uses the same layout for directory and rsync
 > special remotes as it does for the .git/annex directory. As far
-> as I know it works ok on case-insensative filesystems.
+> as I know it works ok on (truely) case-insensative filesystems.
 > 
 > Based on your strace, if you `ls /media/annex/Zp`, you will see
 > "No such file or directory", but if you `mkdir /media/annex/Zp` it will
 > fail with "File exists". Doesn't make much sense to me.
 > 
-> I cannot reproduce this problem using a vfat filesystem 
-> mounted using the same options you show (linux 3.0.0). --[[Joey]]
+> The (default) VFAT mount option shortname=mixed causes this behavior.
+> With shortname=lower it works ok. --[[Joey]]
+> 
+>> So, the options for fixing this bug seem to be to fix Linux (which would
+>> be a good idea IMHO but I don't really want to go there), or generally
+>> convert git-annex to using lowercase for its hashing (which would be a
+>> large amount of pain to rewrite all the symlinks in every git repo),
+>> or some special hack around this problem. 
+>> 
+>> I've put in a workaround for the problem in the directory special
+>> remote; it will use mixed case but fall-back to lowercase as necessary.
+>> 
+>> That does leave the case of a bare git repository with annexed content
+>> stored on VFAT. More special casing could fix it, but that is, I
+>> think, an unusual configuration. Leaving the bug open for that case,
+>> and for the even more unlikely configuration of a rsync special remote
+>> stored on VFAT. --[[Joey]] 
+
+[[!meta title="bare git repository not supported on VFAT"]]