diff options
| -rw-r--r-- | CHANGELOG | 2 | ||||
| -rw-r--r-- | Database/Fsck.hs | 16 | ||||
| -rw-r--r-- | Database/Handle.hs | 21 | ||||
| -rw-r--r-- | Database/Init.hs | 55 | ||||
| -rw-r--r-- | Database/Keys.hs | 8 | ||||
| -rw-r--r-- | Database/Queue.hs | 1 | ||||
| -rw-r--r-- | Utility/FileMode.hs | 17 | ||||
| -rw-r--r-- | doc/bugs/Aborts_with_SQLite_error_when_dropping_contents.mdwn | 3 | ||||
| -rw-r--r-- | doc/bugs/Aborts_with_SQLite_error_when_dropping_contents/comment_3_88a09558f2da0a5733aa3dd042f9d59b._comment | 53 | ||||
| -rw-r--r-- | git-annex.cabal | 1 |
10 files changed, 135 insertions, 42 deletions
@@ -52,6 +52,8 @@ git-annex (6.20170102) UNRELEASED; urgency=medium * Improve pid locking code to work on filesystems that don't support hard links. * S3: Fix check of uuid file stored in bucket, which was not working. + * Work around sqlite's incorrect handling of umask when creating + databases. -- Joey Hess <id@joeyh.name> Fri, 06 Jan 2017 15:22:06 -0400 diff --git a/Database/Fsck.hs b/Database/Fsck.hs index 702b52925..9affeac85 100644 --- a/Database/Fsck.hs +++ b/Database/Fsck.hs @@ -22,11 +22,10 @@ module Database.Fsck ( import Database.Types import qualified Database.Queue as H +import Database.Init import Annex.Locations -import Utility.PosixFiles import Utility.Exception import Annex.Common -import Annex.Perms import Annex.LockFile import Database.Persist.TH @@ -61,17 +60,8 @@ openDb u = do dbdir <- fromRepo (gitAnnexFsckDbDir u) let db = dbdir </> "db" unlessM (liftIO $ doesFileExist db) $ do - let tmpdbdir = dbdir ++ ".tmp" - let tmpdb = tmpdbdir </> "db" - liftIO $ do - createDirectoryIfMissing True tmpdbdir - H.initDb tmpdb $ void $ - runMigrationSilent migrateFsck - setAnnexDirPerm tmpdbdir - setAnnexFilePerm tmpdb - liftIO $ do - void $ tryIO $ removeDirectoryRecursive dbdir - rename tmpdbdir dbdir + initDb db $ void $ + runMigrationSilent migrateFsck lockFileCached =<< fromRepo (gitAnnexFsckDbLock u) h <- liftIO $ H.openDbQueue db "fscked" return $ FsckHandle h u diff --git a/Database/Handle.hs b/Database/Handle.hs index d84ce5b62..7827be749 100644 --- a/Database/Handle.hs +++ b/Database/Handle.hs @@ -9,7 +9,6 @@ module Database.Handle ( DbHandle, - initDb, openDb, TableName, queryDb, @@ -38,26 +37,6 @@ import System.IO - the database. It has a MVar which Jobs are submitted to. -} data DbHandle = DbHandle (Async ()) (MVar Job) -{- Ensures that the database is initialized. Pass the migration action for - - the database. - - - - The database is initialized using WAL mode, to prevent readers - - from blocking writers, and prevent a writer from blocking readers. - -} -initDb :: FilePath -> SqlPersistM () -> IO () -initDb f migration = do - let db = T.pack f - enableWAL db - runSqlite db migration - -enableWAL :: T.Text -> IO () -enableWAL db = do - conn <- Sqlite.open db - stmt <- Sqlite.prepare conn (T.pack "PRAGMA journal_mode=WAL;") - void $ Sqlite.step stmt - void $ Sqlite.finalize stmt - Sqlite.close conn - {- Name of a table that should exist once the database is initialized. -} type TableName = String diff --git a/Database/Init.hs b/Database/Init.hs new file mode 100644 index 000000000..d7a7f6842 --- /dev/null +++ b/Database/Init.hs @@ -0,0 +1,55 @@ +{- Persistent sqlite database initialization + - + - Copyright 2015-2017 Joey Hess <id@joeyh.name> + - + - Licensed under the GNU GPL version 3 or higher. + -} + +module Database.Init where + +import Annex.Common +import Annex.Perms +import Utility.FileMode + +import Database.Persist.Sqlite +import qualified Database.Sqlite as Sqlite +import Control.Monad.IO.Class (liftIO) +import qualified Data.Text as T + +{- Ensures that the database is freshly initialized. Deletes any + - existing database. Pass the migration action for the database. + - + - The database is initialized using WAL mode, to prevent readers + - from blocking writers, and prevent a writer from blocking readers. + - + - The permissions of the database are set based on the + - core.sharedRepository setting. Setting these permissions on the main db + - file causes Sqlite to always use the same permissions for additional + - files it writes later on + -} +initDb :: FilePath -> SqlPersistM () -> Annex () +initDb db migration = do + let dbdir = takeDirectory db + let tmpdbdir = dbdir ++ ".tmp" + let tmpdb = tmpdbdir </> "db" + liftIO $ do + createDirectoryIfMissing True tmpdbdir + let tdb = T.pack tmpdb + enableWAL tdb + runSqlite tdb migration + setAnnexDirPerm tmpdbdir + -- Work around sqlite bug that prevents it from honoring + -- less restrictive umasks. + liftIO $ setFileMode tmpdb =<< defaultFileMode + setAnnexFilePerm tmpdb + liftIO $ do + void $ tryIO $ removeDirectoryRecursive dbdir + rename tmpdbdir dbdir + +enableWAL :: T.Text -> IO () +enableWAL db = do + conn <- Sqlite.open db + stmt <- Sqlite.prepare conn (T.pack "PRAGMA journal_mode=WAL;") + void $ Sqlite.step stmt + void $ Sqlite.finalize stmt + Sqlite.close conn diff --git a/Database/Keys.hs b/Database/Keys.hs index 0f2f34930..b9440ac1a 100644 --- a/Database/Keys.hs +++ b/Database/Keys.hs @@ -25,11 +25,11 @@ import qualified Database.Keys.SQL as SQL import Database.Types import Database.Keys.Handle import qualified Database.Queue as H +import Database.Init import Annex.Locations import Annex.Common hiding (delete) import Annex.Version (versionUsesKeysDatabase) import qualified Annex -import Annex.Perms import Annex.LockFile import Utility.InodeCache import Annex.InodeSentinal @@ -120,11 +120,7 @@ openDb createdb _ = catchPermissionDenied permerr $ withExclusiveLock gitAnnexKe case (dbexists, createdb) of (True, _) -> open db (False, True) -> do - liftIO $ do - createDirectoryIfMissing True dbdir - H.initDb db SQL.createTables - setAnnexDirPerm dbdir - setAnnexFilePerm db + initDb db SQL.createTables open db (False, False) -> return DbUnavailable where diff --git a/Database/Queue.hs b/Database/Queue.hs index c4186b8c8..143871079 100644 --- a/Database/Queue.hs +++ b/Database/Queue.hs @@ -9,7 +9,6 @@ module Database.Queue ( DbQueue, - initDb, openDbQueue, queryDbQueue, closeDbQueue, diff --git a/Utility/FileMode.hs b/Utility/FileMode.hs index bb3780c6e..fe9cbf56a 100644 --- a/Utility/FileMode.hs +++ b/Utility/FileMode.hs @@ -1,6 +1,6 @@ {- File mode utilities. - - - Copyright 2010-2012 Joey Hess <id@joeyh.name> + - Copyright 2010-2017 Joey Hess <id@joeyh.name> - - License: BSD-2-clause -} @@ -130,6 +130,21 @@ withUmask umask a = bracket setup cleanup go withUmask _ a = a #endif +getUmask :: IO FileMode +#ifndef mingw32_HOST_OS +getUmask = bracket setup cleanup return + where + setup = setFileCreationMask nullFileMode + cleanup = setFileCreationMask +#else +getUmask = return nullFileMode +#endif + +defaultFileMode :: IO FileMode +defaultFileMode = do + umask <- getUmask + return $ intersectFileModes (complement umask) stdFileMode + combineModes :: [FileMode] -> FileMode combineModes [] = 0 combineModes [m] = m diff --git a/doc/bugs/Aborts_with_SQLite_error_when_dropping_contents.mdwn b/doc/bugs/Aborts_with_SQLite_error_when_dropping_contents.mdwn index 5acf5f075..de6c26fbf 100644 --- a/doc/bugs/Aborts_with_SQLite_error_when_dropping_contents.mdwn +++ b/doc/bugs/Aborts_with_SQLite_error_when_dropping_contents.mdwn @@ -81,3 +81,6 @@ extra copies. In other words, Git-Annex and I are very happy together, and I'd like to marry it. And because you are the father, I hereby respectfully ask for your blessing. + +> [[fixed|done]] (and I suppose you have my blessing, but I'm not sure +> that's legal yet!) --[[Joey]] diff --git a/doc/bugs/Aborts_with_SQLite_error_when_dropping_contents/comment_3_88a09558f2da0a5733aa3dd042f9d59b._comment b/doc/bugs/Aborts_with_SQLite_error_when_dropping_contents/comment_3_88a09558f2da0a5733aa3dd042f9d59b._comment new file mode 100644 index 000000000..30e108b34 --- /dev/null +++ b/doc/bugs/Aborts_with_SQLite_error_when_dropping_contents/comment_3_88a09558f2da0a5733aa3dd042f9d59b._comment @@ -0,0 +1,53 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 3""" + date="2017-02-13T20:21:09Z" + content=""" +Thanks for following up with the cause of this. + +In fact, assuming you're not using a v6 git-annex repository, it doesn't +really need to update that database at all. But since we'll be upgrading to +v6 eventually, I need to deal with problems like this. Also, +this same problem will also impact the database used for incremental fsck. + +I can reproduce this with a v5 repository; dropping a file happens to run a +code path that updates the database. And reproducing it w/o using git-annex too: + + joey@darkstar:~/tmp>mkdir empty + joey@darkstar:~/tmp>umask + 0002 + joey@darkstar:~/tmp>touch empty/file + joey@darkstar:~/tmp>sqlite3 empty/db + SQLite version 3.16.2 2017-01-06 16:32:41 + Enter ".help" for usage hints. + sqlite> create table foo; + Error: near ";": syntax error + sqlite> + joey@darkstar:~/tmp>ls -l empty/ + total 0 + -rw-r--r-- 1 joey joey 0 Feb 13 16:33 db + -rw-rw-r-- 1 joey joey 0 Feb 13 16:32 file + +Seems that sqlite uses `0644 & umask` for the db permissions, +which is *bad* because it doesn't allow the umask to enable the group +write bit. That 0644 is `SQLITE_DEFAULT_FILE_PERMISSIONS`, so it can +be changed to something saner at compile time. + +`http://www.sqlite.org/src/doc/trunk/src/os_unix.c` has a useful comment. +Seems that sqlite is careful to make -wal, -journal, and -shm files +have the exact same permissions as main database file. + +So, if `.git/annex/keys/*` is updated to have the desired permissions when +the database is created, every further write to the database will keep +using the desired permissions. + +Hmm, it turns out that git-annex does already set the database permissions when +creating it, but only if core.sharedRepository is set to group or all. So +there's a workaround; just `git config core.sharedRepository group` when +setting up a repository that's going to be accessed by multiple users. Almost +certianly a better idea than relying on umask anyway; people mess up umask +settings. + +I'll go ahead and make it always set sane permissions when creating databases. +I'm not going to try to fix up permissions in existing repositories though. +"""]] diff --git a/git-annex.cabal b/git-annex.cabal index 5bcc67315..37e2389a0 100644 --- a/git-annex.cabal +++ b/git-annex.cabal @@ -798,6 +798,7 @@ Executable git-annex Crypto Database.Fsck Database.Handle + Database.Init Database.Keys Database.Keys.Handle Database.Keys.SQL |