From 05684bdd6c7c440ad16a8feae6100c7eeefc14e3 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Sun, 12 May 2024 21:23:27 -0400 Subject: [PATCH 1/4] fsck: Fix recent reversion that made it say it was checksumming files whose content is not present Did not track down the commit that caused the problem, but git-annex version 10.20240431 didn't behave that way. --- CHANGELOG | 2 ++ Command/Fsck.hs | 23 +++++++++++++---------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 754da07f08..494e57ab85 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,7 @@ git-annex (10.20240431) UNRELEASED; urgency=medium + * fsck: Fix recent reversion that made it say it was checksumming files + whose content is not present. * Typo fixes. Thanks, Yaroslav Halchenko diff --git a/Command/Fsck.hs b/Command/Fsck.hs index 3e3de1ea86..de5859cd6c 100644 --- a/Command/Fsck.hs +++ b/Command/Fsck.hs @@ -504,16 +504,19 @@ checkKeyUpgrade _ _ _ (AssociatedFile Nothing) = checkBackend :: Key -> KeyStatus -> AssociatedFile -> Annex Bool checkBackend key keystatus afile = do content <- calcRepo (gitAnnexLocation key) - ifM (pure (isKeyUnlockedThin keystatus) <&&> (not <$> isUnmodified key content)) - ( nocheck - , do - mic <- withTSDelta (liftIO . genInodeCache content) - ifM (checkBackendOr badContent key content ai) - ( do - checkInodeCache key content mic ai - return True - , return False - ) + ifM (liftIO $ R.doesPathExist content) + ( ifM (pure (isKeyUnlockedThin keystatus) <&&> (not <$> isUnmodified key content)) + ( nocheck + , do + mic <- withTSDelta (liftIO . genInodeCache content) + ifM (checkBackendOr badContent key content ai) + ( do + checkInodeCache key content mic ai + return True + , return False + ) + ) + , nocheck ) where nocheck = return True From 8844372c23d1a0607fe9fa087c23fdde7ac4dc6a Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Sun, 12 May 2024 21:27:03 -0400 Subject: [PATCH 2/4] remove doesPathExist check in checkKeyCheckSum Before commit c565340adc1e98886ce8689814f832561c8418a1, it was statting the file in order to get its size, which was needed to use an external hasher. In that commit since it no longer needed the stat, I made it check doesPathExist since the old code implicitly checked if the file existed. But that is not necessary, this should only be ever called on files that exist. --- Backend/Hash.hs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/Backend/Hash.hs b/Backend/Hash.hs index fc0a8b8591..16b3b56c72 100644 --- a/Backend/Hash.hs +++ b/Backend/Hash.hs @@ -22,7 +22,6 @@ import Types.Backend import Types.KeySource import Utility.Hash import Utility.Metered -import qualified Utility.RawFilePath as R import qualified Data.ByteString as S import qualified Data.ByteString.Short as S (toShort, fromShort) @@ -125,14 +124,9 @@ keyValueE hash source meterupdate = checkKeyChecksum :: Hash -> Key -> RawFilePath -> Annex Bool checkKeyChecksum hash key file = catchIOErrorType HardwareFault hwfault $ do - fast <- Annex.getRead Annex.fast - exists <- liftIO $ R.doesPathExist file - case (exists, fast) of - (True, False) -> do - showAction (UnquotedString descChecksum) - sameCheckSum key - <$> hashFile hash file nullMeterUpdate - _ -> return True + showAction (UnquotedString descChecksum) + sameCheckSum key + <$> hashFile hash file nullMeterUpdate where hwfault e = do warning $ UnquotedString $ "hardware fault: " ++ show e From 0281f7f23e254d3c5a1b47249a8d22c0ef03f7dc Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Sun, 12 May 2024 21:36:48 -0400 Subject: [PATCH 3/4] Avoid the --fast option preventing checksumming in some cases it was not supposed to fsck --fast was intended to disable checksumming, but checksumming is done after transfers too. Due to the check being in the non-incremental path, it would only affect non-incremental checksumming during a transfer, and I'm not 100% sure that it was a problem. Also, when using an external backend that does checksumming, fsck --fast didn't disable it and now does. --- CHANGELOG | 2 ++ Command/Fsck.hs | 20 ++++++++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 494e57ab85..a18e598a6f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,8 @@ git-annex (10.20240431) UNRELEASED; urgency=medium * fsck: Fix recent reversion that made it say it was checksumming files whose content is not present. + * Avoid the --fast option preventing checksumming in some cases it + was not supposed to. * Typo fixes. Thanks, Yaroslav Halchenko diff --git a/Command/Fsck.hs b/Command/Fsck.hs index de5859cd6c..2212c83007 100644 --- a/Command/Fsck.hs +++ b/Command/Fsck.hs @@ -528,14 +528,18 @@ checkBackendRemote key remote ai localcopy = checkBackendOr (badContentRemote remote localcopy) key localcopy ai checkBackendOr :: (Key -> Annex String) -> Key -> RawFilePath -> ActionItem -> Annex Bool -checkBackendOr bad key file ai = do - ok <- verifyKeyContent' key file - unless ok $ do - msg <- bad key - warning $ actionItemDesc ai - <> ": Bad file content; " - <> UnquotedString msg - return ok +checkBackendOr bad key file ai = + ifM (Annex.getRead Annex.fast) + ( return True + , do + ok <- verifyKeyContent' key file + unless ok $ do + msg <- bad key + warning $ actionItemDesc ai + <> ": Bad file content; " + <> UnquotedString msg + return ok + ) {- Check, if there are InodeCaches recorded for a key, that one of them - matches the object file. There are situations where the InodeCache From e154c6da92407572bdc49d7ea3cbb3164c62e5aa Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Mon, 13 May 2024 17:11:34 -0400 Subject: [PATCH 4/4] bug report (copied from email) --- ...ck-present_repo_removes_+x_permission.mdwn | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 doc/bugs/adjust_or_sync_in_unlock-present_repo_removes_+x_permission.mdwn diff --git a/doc/bugs/adjust_or_sync_in_unlock-present_repo_removes_+x_permission.mdwn b/doc/bugs/adjust_or_sync_in_unlock-present_repo_removes_+x_permission.mdwn new file mode 100644 index 0000000000..5dd73cc7a0 --- /dev/null +++ b/doc/bugs/adjust_or_sync_in_unlock-present_repo_removes_+x_permission.mdwn @@ -0,0 +1,34 @@ +It seems that performing `git annex adjust --unlock-present` or `sync` +will remove the +x permission from files. + +Steps to reproduce: + +$ mkdir /tmp/ga; cd /tmp/ga; git init ; git annex init +$ touch a.txt s.sh +$ chmod +x s.sh +$ git annex add . && git annex sync +$ stat -Lc%A s.sh +-r-xr-xr-x +$ git annex adjust --unlock-present +$ stat -c%A s.sh +-rw-r--r-- + +The permission is removed also after a sync, but only in case other +files have been changed (and adjust called behind the scenes): + +$ chmod +x s.sh +$ git annex sync +$ stat -c%A s.sh +-rwxr-xr-x +$ git annex drop --force a.txt +$ stat -c%A s.sh +-rwxr-xr-x +$ git annex sync +$ stat -c%A s.sh +-rw-r--r-- + +I'm using git-annex version: 10.20240430-g5b36e6b4 with +annex.alwayscommit = false. + +PS: git-annex is so solid that this is the first data-related issue I've +ever seen. Kudos!