From 3aaf6ade3084e11561ca43be1a49385a63fd7584 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Tue, 26 Oct 2021 14:08:56 -0400 Subject: [PATCH] review --- ..._8607154b7bd99c90f6fd13926c7a746d._comment | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 doc/todo/__91__PATCH__93___Call_freezeContent_after_move_into_annex/comment_1_8607154b7bd99c90f6fd13926c7a746d._comment diff --git a/doc/todo/__91__PATCH__93___Call_freezeContent_after_move_into_annex/comment_1_8607154b7bd99c90f6fd13926c7a746d._comment b/doc/todo/__91__PATCH__93___Call_freezeContent_after_move_into_annex/comment_1_8607154b7bd99c90f6fd13926c7a746d._comment new file mode 100644 index 0000000000..b8fcb1f51f --- /dev/null +++ b/doc/todo/__91__PATCH__93___Call_freezeContent_after_move_into_annex/comment_1_8607154b7bd99c90f6fd13926c7a746d._comment @@ -0,0 +1,49 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 1""" + date="2021-10-26T17:49:00Z" + content=""" +Thank you for putting this patch together. It is especially helpful to get +patches from a windows user, since it's far from my comfort zone. + +--- + +My first concern was what happens if git-annex is interrupted after moving +the object into place but before freezeContent. Leaving an object file +with possibly unsafe permissions. Looks like `git-annex fsck` will +corrrect that, if it's run. + +As you mentioned, when an unlocked file is added, and linkToAnnex +is called, it does move the object into the annex before freezeContent. +Although that may have been an oversight really. It could just as well +freeze before moving and so avoid leaving the file with the wrong +permissions when interrupted. + +And there are other situations where being interrupted can have the same +result. Eg, in lockContentForRemoval, it calls thawContent, then an action +that may take long enough to be interrupted, and then freezeContent. +And it's hard to see any other way that could work; it can't +move the object out of the object directory before thawing it. + +So, this seems ok, I suppose. + +--- + +In Annex.Ingest, `lockDown'` calls freezeContent on the file +when it's still in the work tree. So I think that would have the same +problem you're trying to prevent with this patch? + +Command.Import also has a call to freezeContent that is not on the final +object file location. + +A windows-specific feature like this risks getting broken, so maybe +it would be good to change freezeContent to avoid such problems. Eg, +it could be changed to take a Key, and freeze the object file +for that Key. But at least the call in Annex.Ingest needs to happen +before there is a Key. + +So perhaps there should be a freezeContent +and a separate freezeObject, which takes a Key. There could +then be a separate annex.freezeobject-command that gets run only +for freezeObject, not freezeContent. +"""]]