From b63e3118d7cb8f3f3e72909a885fc83d903d38ef Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Mon, 25 Jan 2021 13:31:17 -0400 Subject: [PATCH] fix export overwrite on FAT Don't accept the cid of the temp file that the content has just been written to as something we will accept if another file has that same content. There's no reason to, and on FAT, due to mtime resolution, the test suite hit just such a case. This fixes a reversion from 73df633a6215faa093e4a7524c6d328aa988aed1 which removed inode from the ContentIdentifier. --- Remote/Directory.hs | 2 +- ...import_fails_on_crippled_FS_on_ubuntu.mdwn | 2 ++ ..._d949841feb6a035e20a3d0f31634d388._comment | 30 +++++++++++++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 doc/bugs/export__95__import_fails_on_crippled_FS_on_ubuntu/comment_3_d949841feb6a035e20a3d0f31634d388._comment diff --git a/Remote/Directory.hs b/Remote/Directory.hs index c4f8dcf45d..58c91edfc9 100644 --- a/Remote/Directory.hs +++ b/Remote/Directory.hs @@ -471,7 +471,7 @@ storeExportWithContentIdentifierM dir src _k loc overwritablecids p = do Nothing -> giveup "unable to generate content identifier" Just newcid -> do checkExportContent dir loc - (newcid:overwritablecids) + overwritablecids (giveup "unsafe to overwrite file") (const $ liftIO $ rename tmpf dest) return newcid diff --git a/doc/bugs/export__95__import_fails_on_crippled_FS_on_ubuntu.mdwn b/doc/bugs/export__95__import_fails_on_crippled_FS_on_ubuntu.mdwn index 4ed5427cd9..ea1b42dac2 100644 --- a/doc/bugs/export__95__import_fails_on_crippled_FS_on_ubuntu.mdwn +++ b/doc/bugs/export__95__import_fails_on_crippled_FS_on_ubuntu.mdwn @@ -26,3 +26,5 @@ on 8.20201129+git169-gaa07e68ed-1~ndall+1 (presumably... identified 3 regression [[!meta author=yoh]] [[!tag projects/datalad]] + +> [[fixed|done]] --[[Joey]] diff --git a/doc/bugs/export__95__import_fails_on_crippled_FS_on_ubuntu/comment_3_d949841feb6a035e20a3d0f31634d388._comment b/doc/bugs/export__95__import_fails_on_crippled_FS_on_ubuntu/comment_3_d949841feb6a035e20a3d0f31634d388._comment new file mode 100644 index 0000000000..25e9d0a879 --- /dev/null +++ b/doc/bugs/export__95__import_fails_on_crippled_FS_on_ubuntu/comment_3_d949841feb6a035e20a3d0f31634d388._comment @@ -0,0 +1,30 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 3""" + date="2021-01-25T16:54:58Z" + content=""" +Oh this is interesting: + + unexport foo import + unsafe to remove modified file + failed + export foo import ok + +So despite seeing the file is modified when it is asked to remove it, it +somehow does not notice that when writing to it, so overwrites it +with the content anyway. That's how it ends up with +unexpected content, despite the overall `git-annex export` +failing as expected. + +And the root cause is, the test suite writes to the file on the remote, +and then Remote.Directory writes to a temp file, and then gets its content +identifier (mtime), and for some reason accepts that content identifier as +one it can overwrite. Due to the timestamp resolution, that's the same +content identifier as the newly written file on the remote. So it +overwrites it. + +I can't find any good reason for it to accept the temp file's content +identifier as a known content identifier, which it's always done. Other remotes +don't do that. So I think this is a real bug in Remote.Directory and will +fix it there. +"""]] -- 2.30.2