From 04a27ad9267a95e142a688f2eede15ef08f2d552 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Wed, 10 Apr 2013 19:57:11 -0400 Subject: [PATCH] assistant: Bug fix to avoid annexing the files that git uses to stand in for symlinks on FAT and other filesystem not supporting symlinks. also, blog for the day.. --- Assistant/Threads/Watcher.hs | 60 +++++++++++++------ debian/changelog | 3 + ...ository_on_USB_drive_causes_sync_loop.mdwn | 4 ++ .../blog/day_234__clean_shutdown.mdwn | 29 +++++++++ 4 files changed, 79 insertions(+), 17 deletions(-) create mode 100644 doc/design/assistant/blog/day_234__clean_shutdown.mdwn diff --git a/Assistant/Threads/Watcher.hs b/Assistant/Threads/Watcher.hs index 9bd78b62b9..2ee3fb5f76 100644 --- a/Assistant/Threads/Watcher.hs +++ b/Assistant/Threads/Watcher.hs @@ -80,7 +80,10 @@ runWatcher = do startup <- asIO1 startupScan matcher <- liftAnnex $ largeFilesMatcher direct <- liftAnnex isDirect - addhook <- hook $ if direct then onAddDirect matcher else onAdd matcher + symlinkssupported <- liftAnnex $ coreSymlinks <$> Annex.getGitConfig + addhook <- hook $ if direct + then onAddDirect symlinkssupported matcher + else onAdd matcher delhook <- hook onDel addsymlinkhook <- hook $ onAddSymlink direct deldirhook <- hook onDelDir @@ -188,9 +191,10 @@ onAdd matcher file filestatus | otherwise = noChange {- In direct mode, add events are received for both new files, and - - modified existing files. -} -onAddDirect :: FileMatcher -> Handler -onAddDirect matcher file fs = do + - modified existing files. + -} +onAddDirect :: Bool -> FileMatcher -> Handler +onAddDirect symlinkssupported matcher file fs = do v <- liftAnnex $ catKeyFile file case (v, fs) of (Just key, Just filestatus) -> @@ -203,37 +207,59 @@ onAddDirect matcher file fs = do ( do link <- liftAnnex $ inRepo $ gitAnnexLink file key addLink file link (Just key) - , do + , guardSymlinkStandin (Just key) $ do debug ["changed direct", file] liftAnnex $ changedDirect key file add matcher file ) - _ -> do + _ -> guardSymlinkStandin Nothing $ do debug ["add direct", file] add matcher file + where + {- On a filesystem without symlinks, we'll get changes for regular + - files that git uses to stand-in for symlinks. Detect when + - this happens, and stage the symlink, rather than annexing the + - file. -} + guardSymlinkStandin mk a + | symlinkssupported = a + | otherwise = do + linktarget <- liftAnnex $ getAnnexLinkTarget file + liftIO $ print (file, linktarget) + case linktarget of + Nothing -> a + Just lt -> do + case fileKey $ takeFileName lt of + Nothing -> noop + Just key -> void $ liftAnnex $ + addAssociatedFile key file + onAddSymlink' linktarget mk True file fs {- A symlink might be an arbitrary symlink, which is just added. - Or, if it is a git-annex symlink, ensure it points to the content - before adding it. -} onAddSymlink :: Bool -> Handler -onAddSymlink isdirect file filestatus = go =<< liftAnnex (Backend.lookupFile file) +onAddSymlink isdirect file filestatus = do + linktarget <- liftIO (catchMaybeIO $ readSymbolicLink file) + kv <- liftAnnex (Backend.lookupFile file) + onAddSymlink' linktarget (fmap fst kv) isdirect file filestatus + +onAddSymlink' :: Maybe String -> Maybe Key -> Bool -> Handler +onAddSymlink' linktarget mk isdirect file filestatus = go mk where - go (Just (key, _)) = do + go (Just key) = do when isdirect $ liftAnnex $ void $ addAssociatedFile key file link <- liftAnnex $ inRepo $ gitAnnexLink file key - ifM ((==) (Just link) <$> liftIO (catchMaybeIO $ readSymbolicLink file)) - ( ensurestaged (Just link) (Just key) =<< getDaemonStatus - , do + if linktarget == Just link + then ensurestaged (Just link) =<< getDaemonStatus + else do unless isdirect $ liftAnnex $ replaceFile file $ makeAnnexLink link addLink file link (Just key) - ) - go Nothing = do -- other symlink - mlink <- liftIO (catchMaybeIO $ readSymbolicLink file) - ensurestaged mlink Nothing =<< getDaemonStatus + -- other symlink, not git-annex + go Nothing = ensurestaged linktarget =<< getDaemonStatus {- This is often called on symlinks that are already - staged correctly. A symlink may have been deleted @@ -246,13 +272,13 @@ onAddSymlink isdirect file filestatus = go =<< liftAnnex (Backend.lookupFile fil - (If the daemon has never ran before, avoid staging - links too.) -} - ensurestaged (Just link) mk daemonstatus + ensurestaged (Just link) daemonstatus | scanComplete daemonstatus = addLink file link mk | otherwise = case filestatus of Just s | not (afterLastDaemonRun (statusChangeTime s) daemonstatus) -> noChange _ -> addLink file link mk - ensurestaged Nothing _ _ = noChange + ensurestaged Nothing _ = noChange {- For speed, tries to reuse the existing blob for symlink target. -} addLink :: FilePath -> FilePath -> Maybe Key -> Assistant (Maybe Change) diff --git a/debian/changelog b/debian/changelog index 7b213cdded..35a77be7f9 100644 --- a/debian/changelog +++ b/debian/changelog @@ -19,6 +19,9 @@ git-annex (4.20130406) UNRELEASED; urgency=low * webapp: Added animations. * assistant: Stop any transfers the assistant initiated on shutdown. * assistant: Added sequence numbers to XMPP git push packets. (Not yet used.) + * assistant: Bug fix to avoid annexing the files that git uses + to stand in for symlinks on FAT and other filesystem not supporting + symlinks. -- Joey Hess Sat, 06 Apr 2013 15:24:15 -0400 diff --git a/doc/bugs/Add_another_repository_on_USB_drive_causes_sync_loop.mdwn b/doc/bugs/Add_another_repository_on_USB_drive_causes_sync_loop.mdwn index 5125a030e0..c5bff196ef 100644 --- a/doc/bugs/Add_another_repository_on_USB_drive_causes_sync_loop.mdwn +++ b/doc/bugs/Add_another_repository_on_USB_drive_causes_sync_loop.mdwn @@ -16,3 +16,7 @@ Linux Please provide any additional information below. +> Reproduced the core bug, which is that the assistant saw symlink standin +> files as new files, and annexed them. Now it doesn't, and I have +> it running on FAT with no trouble; can even rename symlink standin files +> and it commits symlink changes. Calling this [[done]]. --[[Joey]] diff --git a/doc/design/assistant/blog/day_234__clean_shutdown.mdwn b/doc/design/assistant/blog/day_234__clean_shutdown.mdwn new file mode 100644 index 0000000000..61b765b593 --- /dev/null +++ b/doc/design/assistant/blog/day_234__clean_shutdown.mdwn @@ -0,0 +1,29 @@ +Short day because I spent 3 hours this morning explaining free software +and kickstarter to an accountant. And was away until 3 pm, so how did I get +all this done‽ + +Eliot pointed out that shutting down the assistant could leave transfers +running. This happened because `git annex transferkeys` is a separate +process, and so it was left to finish up any transfer that was in +process. I've made shutdown stop all transfers that the +assistant started. (Other paired computers could still be connecting to +make transfers even when the assistant is not running, and those are not +affected.) + +Added sequence numbers to the XMPP messages used for git pushes. While +these numbers are not used yet, they're available for debugging, and will +help me determine if packets are lost or come out of order. So if you have +experienced problems with XMPP syncing sometimes failing, run tonight's +build of the assistant with `--debug` (or turn on debugging in the webapp +configuration screen), and send me a log by email to + + +Changed the way that autobuilds and manual builds report their version +number. It now includes the date of the last commit, and the abbreviated +commit ID, rather than being some random date after the last release. + +Frederik found a bug using the assistant on a FAT filesystem. It didn't +properly handle the files that git uses to stand-in for symlinks in that +situation, and annexed those files. I've fixed this, and even moving +around symlink stand-in files on a FAT filesystem now results in correct +changes to symlinks being committed.