drop, move, mirror: when two files have the same content, honor the max numcopies...

Eg, before with a .gitattributes like:

*.2 annex.numcopies=2
*.1 annex.numcopies=1

And foo.1 and foo.2 having the same content and key, git-annex drop foo.1 foo.2
would succeed, leaving just 1 copy, despite foo.2 needing 2 copies.
It dropped foo.1 first and then skipped foo.2 since its content was gone.

Now that the keys database includes locked files, this longstanding wart
can be fixed.

Sponsored-by: Noam Kremen on Patreon

diff --git a/Annex/Drop.hs b/Annex/Drop.hs
index dc6b7b64ef620a5bb0c1d848ccc342d0a35724a6..79d6d63ff03a429674d07d007f6e55bb7d319a85 100644 (file)
--- a/Annex/Drop.hs
+++ b/Annex/Drop.hs
@@ -60,7 +60,7 @@ handleDropsFrom locs rs reason fromhere key afile si preverified runner = do
   where
        getcopies fs = do
                (untrusted, have) <- trustPartition UnTrusted locs
-               (numcopies, mincopies) <- getSafestNumMinCopies' key fs
+               (numcopies, mincopies) <- getSafestNumMinCopies' afile key fs
                return (length have, numcopies, mincopies, S.fromList untrusted)
 
        {- Check that we have enough copies still to drop the content.

diff --git a/Annex/NumCopies.hs b/Annex/NumCopies.hs
index bbdd826e8e4300fbc817ab855fb0bd0ab9bcdfd5..a912028930e51a1989715457aaec180bf312965a 100644 (file)
--- a/Annex/NumCopies.hs
+++ b/Annex/NumCopies.hs
@@ -11,7 +11,6 @@ module Annex.NumCopies (
        module Types.NumCopies,
        module Logs.NumCopies,
        getFileNumMinCopies,
-       getAssociatedFileNumMinCopies,
        getSafestNumMinCopies,
        getSafestNumMinCopies',
        getGlobalFileNumCopies,
@@ -123,33 +122,21 @@ getFileNumMinCopies f = do
                                        <$> fallbacknum
                                        <*> fallbackmin
 
-{- NumCopies and MinCopies value for an associated file, or the default
- - when there is no associated file.
- -
- - This does not include other associated files using the same key.
- -}
-getAssociatedFileNumMinCopies :: AssociatedFile -> Annex (NumCopies, MinCopies)
-getAssociatedFileNumMinCopies (AssociatedFile (Just file)) =
-       getFileNumMinCopies file
-getAssociatedFileNumMinCopies (AssociatedFile Nothing) = (,)
-       <$> getNumCopies
-       <*> getMinCopies
-
 {- Gets the highest NumCopies and MinCopies value for all files
  - associated with a key. Provide any known associated file;
  - the rest are looked up from the database.
  -
- - Using this when dropping avoids dropping one file that
- - has a smaller value violating the value set for another file
- - that uses the same content.
+ - Using this when dropping, rather than getFileNumMinCopies
+ - avoids dropping one file that has a smaller value violating
+ - the value set for another file that uses the same content.
  -}
 getSafestNumMinCopies :: AssociatedFile -> Key -> Annex (NumCopies, MinCopies)
 getSafestNumMinCopies afile k =
        Database.Keys.getAssociatedFilesIncluding afile k
-               >>= getSafestNumMinCopies' k
+               >>= getSafestNumMinCopies' afile k
 
-getSafestNumMinCopies' :: Key -> [RawFilePath] -> Annex (NumCopies, MinCopies)
-getSafestNumMinCopies' k fs = do
+getSafestNumMinCopies' :: AssociatedFile -> Key -> [RawFilePath] -> Annex (NumCopies, MinCopies)
+getSafestNumMinCopies' afile k fs = do
        l <- mapM getFileNumMinCopies fs
        let l' = zip l fs
        (,)
@@ -158,9 +145,14 @@ getSafestNumMinCopies' k fs = do
   where
        -- Some associated files in the keys database may no longer
        -- correspond to files in the repository.
-       stillassociated f = catKeyFile f >>= \case
-               Just k' | k' == k -> return True
-               _ -> return False
+       -- (But the AssociatedFile passed to this is known to be
+       -- an associated file, which may not be in the keys database
+       -- yet, so checking it is skipped.)
+       stillassociated f
+               | AssociatedFile (Just f) == afile = return True
+               | otherwise = catKeyFile f >>= \case
+                       Just k' | k' == k -> return True
+                       _ -> return False
        
        -- Avoid calling stillassociated on every file; just make sure
        -- that the one with the highest value is still associated.

diff --git a/CHANGELOG b/CHANGELOG
index 3cf6b425190a39b149ded09b5c8795c53e15209b..9e6b2358a1c2eec55329e440fac482cd821ccab1 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -4,7 +4,7 @@ git-annex (8.20210429) UNRELEASED; urgency=medium
   * When two files have the same content, and a required content expression
     matches one but not the other, dropping the latter file will fail as it
     would also remove the content of the required file.
-  * drop, move, import: When two files have the same content, and
+  * drop, move, mirror: When two files have the same content, and
     different numcopies or requiredcopies values, use the higher value.
   * drop --auto: When two files have the same content, and a preferred content
     expression matches one but not the other, do not drop the content.

diff --git a/Command/Drop.hs b/Command/Drop.hs
index f30b6f4c08d2654fd14ad22e337c690053ddbe6c..890b9e0046c669d1ae2ab957207001b3e4f8e661 100644 (file)
--- a/Command/Drop.hs
+++ b/Command/Drop.hs
@@ -227,7 +227,7 @@ checkRequiredContent (PreferredContentChecked False) u k afile =
  - copies on other semitrusted repositories. -}
 checkDropAuto :: Bool -> Maybe Remote -> AssociatedFile -> Key -> (NumCopies -> MinCopies -> CommandStart) -> CommandStart
 checkDropAuto automode mremote afile key a =
-       go =<< getAssociatedFileNumMinCopies afile
+       go =<< getSafestNumMinCopies afile key
   where
        go (numcopies, mincopies)
                | automode = do

diff --git a/Command/Mirror.hs b/Command/Mirror.hs
index 4fe8c31f4c6787ecc7cb004706bda176dc134f86..2e31efa65f2921d4e4038d73785ffc6a25445a5e 100644 (file)
--- a/Command/Mirror.hs
+++ b/Command/Mirror.hs
@@ -68,7 +68,7 @@ startKey o afile (si, key, ai) = case fromToOptions o of
        ToRemote r -> checkFailedTransferDirection ai Upload $ ifM (inAnnex key)
                ( Command.Move.toStart Command.Move.RemoveNever afile key ai si =<< getParsed r
                , do
-                       (numcopies, mincopies) <- getAssociatedFileNumMinCopies afile
+                       (numcopies, mincopies) <- getSafestNumMinCopies afile key
                        Command.Drop.startRemote pcc afile ai si numcopies mincopies key =<< getParsed r
                )
        FromRemote r -> checkFailedTransferDirection ai Download $ do
@@ -81,7 +81,7 @@ startKey o afile (si, key, ai) = case fromToOptions o of
                                )
                        Right False -> ifM (inAnnex key)
                                ( do
-                                       (numcopies, mincopies) <- getAssociatedFileNumMinCopies afile
+                                       (numcopies, mincopies) <- getSafestNumMinCopies afile key
                                        Command.Drop.startLocal pcc afile ai si numcopies mincopies key []
                                , stop
                                )

diff --git a/Command/Move.hs b/Command/Move.hs
index 6d2cc50c3097e7e20c07ef53d86ca99feee3cab8..ab6cd0aeb693b33e14371a3440918ca6d83a00b0 100644 (file)
--- a/Command/Move.hs
+++ b/Command/Move.hs
@@ -166,7 +166,7 @@ toPerform dest removewhen key afile fastcheck isthere = do
                        willDropMakeItWorse srcuuid destuuid deststartedwithcopy key afile >>= \case
                                DropAllowed -> drophere setpresentremote contentlock "moved"
                                DropCheckNumCopies -> do
-                                       (numcopies, mincopies) <- getAssociatedFileNumMinCopies afile
+                                       (numcopies, mincopies) <- getSafestNumMinCopies afile key
                                        (tocheck, verified) <- verifiableCopies key [srcuuid]
                                        verifyEnoughCopiesToDrop "" key (Just contentlock)
                                                 numcopies mincopies [srcuuid] verified
@@ -245,7 +245,7 @@ fromPerform src removewhen key afile = do
                willDropMakeItWorse srcuuid destuuid deststartedwithcopy key afile >>= \case
                        DropAllowed -> dropremote "moved"
                        DropCheckNumCopies -> do
-                               (numcopies, mincopies) <- getAssociatedFileNumMinCopies afile
+                               (numcopies, mincopies) <- getSafestNumMinCopies afile key
                                (tocheck, verified) <- verifiableCopies key [Remote.uuid src]
                                verifyEnoughCopiesToDrop "" key Nothing numcopies mincopies [Remote.uuid src] verified
                                        tocheck (dropremote . showproof) faileddropremote

diff --git a/doc/todo/numcopies_check_other_files_using_same_key.mdwn b/doc/todo/numcopies_check_other_files_using_same_key.mdwn
index 2d8782a0b258fed12b18e0b6fce9b766929256ba..64d8070a4ca262f3659803f2424dd76cad96565b 100644 (file)
--- a/doc/todo/numcopies_check_other_files_using_same_key.mdwn
+++ b/doc/todo/numcopies_check_other_files_using_same_key.mdwn
@@ -21,4 +21,6 @@ do say that it bypasses checking .gitattributes numcopies.
 > files. With the recent change to also track
 > associated files for locked files, they also handle it for those.
 > 
-> But, git-annex drop/move/import don't yet.
+> But, git-annex drop/move/mirror don't yet.
+> 
+> > [[fixed|done]] (did not change --all behavior) --[[Joey]]