review
This commit is contained in:
parent
5a9e6b1fd4
commit
3aaf6ade30
1 changed files with 49 additions and 0 deletions
|
@ -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.
|
||||||
|
"""]]
|
Loading…
Add table
Add a link
Reference in a new issue