diff --git a/Annex/AutoMerge.hs b/Annex/AutoMerge.hs index 159d4f96ab..f4f95ff441 100644 --- a/Annex/AutoMerge.hs +++ b/Annex/AutoMerge.hs @@ -137,7 +137,7 @@ resolveMerge us them inoverlay = do (fs, cleanup) <- inRepo (LsFiles.unmerged [top]) srcmap <- if inoverlay then pure M.empty - else inodeMap $ pure (map LsFiles.unmergedFile fs, return True) + else inodeMap $ pure (concatMap getunmergedfiles fs, return True) (mergedks, mergedfs) <- unzip <$> mapM (resolveMerge' srcmap us them inoverlay) fs let mergedks' = concat mergedks let mergedfs' = catMaybes mergedfs @@ -160,6 +160,11 @@ resolveMerge us them inoverlay = do cleanConflictCruft mergedks' mergedfs' unstagedmap showLongNote "Merge conflict was automatically resolved; you may want to examine the result." return merged + where + getunmergedfiles u = catMaybes + [ Just (LsFiles.unmergedFile u) + , LsFiles.unmergedSiblingFile u + ] resolveMerge' :: InodeMap -> Maybe Git.Ref -> Git.Ref -> Bool -> LsFiles.Unmerged -> Annex ([Key], Maybe FilePath) resolveMerge' _ Nothing _ _ _ = return ([], Nothing) @@ -177,7 +182,7 @@ resolveMerge' unstagedmap (Just us) them inoverlay u = do unless inoverlay $ unless (islocked LsFiles.valUs) $ liftIO $ removeWhenExistsWith R.removeLink (toRawFilePath file) - | otherwise -> do + | otherwise -> resolveby [keyUs, keyThem] $ -- Only resolve using symlink when both -- were locked, otherwise use unlocked -- pointer. @@ -185,13 +190,12 @@ resolveMerge' unstagedmap (Just us) them inoverlay u = do if islocked LsFiles.valUs && islocked LsFiles.valThem then makesymlink keyUs file else makepointer keyUs file (combinedmodes) - return ([keyUs, keyThem], Just file) -- Our side is annexed file, other side is not. -- Make the annexed file into a variant file and graft in the -- other file/directory as it was. (Just keyUs, Nothing) -> resolveby [keyUs] $ do graftin them file LsFiles.valThem LsFiles.valThem LsFiles.valUs - makevariantannexlink keyUs LsFiles.valUs + makevariantannexlink keyUs LsFiles.valUs -- Our side is not annexed file, other side is. (Nothing, Just keyThem) -> resolveby [keyThem] $ do graftin us file LsFiles.valUs LsFiles.valUs LsFiles.valThem @@ -200,6 +204,7 @@ resolveMerge' unstagedmap (Just us) them inoverlay u = do (Nothing, Nothing) -> return ([], Nothing) where file = fromRawFilePath $ LsFiles.unmergedFile u + sibfile = fromRawFilePath <$> LsFiles.unmergedSiblingFile u getkey select = case select (LsFiles.unmergedSha u) of @@ -256,21 +261,27 @@ resolveMerge' unstagedmap (Just us) them inoverlay u = do graftin b item selectwant selectwant' selectunwant = do Annex.Queue.addUpdateIndex =<< fromRepo (UpdateIndex.lsSubTree b item) + + let replacefile isexecutable = case selectwant' (LsFiles.unmergedSha u) of + Nothing -> noop + Just sha -> replaceWorkTreeFile item $ \tmp -> do + c <- catObject sha + liftIO $ L.writeFile tmp c + when isexecutable $ + liftIO $ void $ tryIO $ + modifyFileMode (toRawFilePath tmp) $ + addModes executeModes -- Update the work tree to reflect the graft. unless inoverlay $ case (selectwant (LsFiles.unmergedTreeItemType u), selectunwant (LsFiles.unmergedTreeItemType u)) of - -- Symlinks are never left in work tree when - -- there's a conflict with anything else. - -- So, when grafting in a symlink, we must create it: (Just TreeSymlink, _) -> do case selectwant' (LsFiles.unmergedSha u) of Nothing -> noop Just sha -> do link <- catSymLinkTarget sha replacewithsymlink item link - -- And when grafting in anything else vs a symlink, - -- the work tree already contains what we want. - (_, Just TreeSymlink) -> noop + (Just TreeFile, Just TreeSymlink) -> replacefile False + (Just TreeExecutable, Just TreeSymlink) -> replacefile True _ -> ifM (liftIO $ doesDirectoryExist item) -- a conflict between a file and a directory -- leaves the directory, so since a directory @@ -278,23 +289,24 @@ resolveMerge' unstagedmap (Just us) them inoverlay u = do ( noop -- probably a file with conflict markers is -- in the work tree; replace with grafted - -- file content - , case selectwant' (LsFiles.unmergedSha u) of - Nothing -> noop - Just sha -> replaceWorkTreeFile item $ \tmp -> do - c <- catObject sha - liftIO $ L.writeFile tmp c + -- file content (this is needed when + -- the annexed file is unlocked) + , replacefile False ) resolveby ks a = do - {- Remove conflicted file from index so merge can be resolved. -} + {- Remove conflicted file from index so merge can be resolved. + - If there's a sibling conflicted file, remove it too. -} Annex.Queue.addCommand [] "rm" [ Param "--quiet" , Param "-f" , Param "--cached" , Param "--" ] - [file] + (catMaybes [Just file, sibfile]) + liftIO $ maybe noop + (removeWhenExistsWith R.removeLink . toRawFilePath) + sibfile void a return (ks, Just file) diff --git a/CHANGELOG b/CHANGELOG index 57f98c424f..e67a08b44b 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -3,6 +3,9 @@ git-annex (8.20211118) UNRELEASED; urgency=medium * Bugfix: When -J was enabled, getting files could leak an ever-growing number of git cat-file processes. * importfeed: Display url before starting youtube-dl download. + * Support git's new "ort" resolver, which became the default in git 2.34.0, + and broke the test suite and automatic merge resolution of a conflict + between an annexed file and a non-annexed file. -- Joey Hess Wed, 17 Nov 2021 13:18:44 -0400 diff --git a/Git/LsFiles.hs b/Git/LsFiles.hs index aff408deb6..42115de6e7 100644 --- a/Git/LsFiles.hs +++ b/Git/LsFiles.hs @@ -5,6 +5,8 @@ - Licensed under the GNU AGPL version 3 or higher. -} +{-# LANGUAGE OverloadedStrings #-} + module Git.LsFiles ( Options(..), inRepo, @@ -236,7 +238,14 @@ data Unmerged = Unmerged { unmergedFile :: RawFilePath , unmergedTreeItemType :: Conflicting TreeItemType , unmergedSha :: Conflicting Sha - } + , unmergedSiblingFile :: Maybe RawFilePath + -- ^ Normally this is Nothing, because a + -- merge conflict is represented as a single file with two + -- stages. However, git resolvers sometimes choose to stage + -- two files, one for each side of the merge conflict. In such a case, + -- this is used for the name of the second file, which is related + -- to the first file. (Eg, "foo" and "foo~ref") + } deriving (Show) {- Returns a list of the files in the specified locations that have - unresolved merge conflicts. @@ -246,7 +255,7 @@ data Unmerged = Unmerged - 1 = old version, can be ignored - 2 = us - 3 = them - - If a line is omitted, that side removed the file. + - If line 2 or 3 is omitted, that side removed the file. -} unmerged :: [RawFilePath] -> Repo -> IO ([Unmerged], IO Bool) unmerged l repo = guardSafeForLsFiles repo $ do @@ -265,7 +274,7 @@ data InternalUnmerged = InternalUnmerged , ifile :: RawFilePath , itreeitemtype :: Maybe TreeItemType , isha :: Maybe Sha - } + } deriving (Show) parseUnmerged :: String -> Maybe InternalUnmerged parseUnmerged s @@ -296,16 +305,25 @@ reduceUnmerged c (i:is) = reduceUnmerged (new:c) rest { unmergedFile = ifile i , unmergedTreeItemType = Conflicting treeitemtypeA treeitemtypeB , unmergedSha = Conflicting shaA shaB + , unmergedSiblingFile = if ifile sibi == ifile i + then Nothing + else Just (ifile sibi) } findsib templatei [] = ([], removed templatei) findsib templatei (l:ls) - | ifile l == ifile templatei = (ls, l) + | ifile l == ifile templatei || issibfile templatei l = (ls, l) | otherwise = (l:ls, removed templatei) removed templatei = templatei { isus = not (isus templatei) , itreeitemtype = Nothing , isha = Nothing } + -- foo~ are unmerged sibling files of foo + -- Some versions or resolvers of git stage the sibling files, + -- other versions or resolvers do not. + issibfile x y = (ifile x <> "~") `S.isPrefixOf` ifile y + && isus x || isus y + && not (isus x && isus y) {- Gets the InodeCache equivilant information stored in the git index. - diff --git a/doc/bugs/git_2.34__58___some_conflict_resolution_unit_tests_fail.mdwn b/doc/bugs/git_2.34__58___some_conflict_resolution_unit_tests_fail.mdwn index 0cf1b125fc..9d8f0a7731 100644 --- a/doc/bugs/git_2.34__58___some_conflict_resolution_unit_tests_fail.mdwn +++ b/doc/bugs/git_2.34__58___some_conflict_resolution_unit_tests_fail.mdwn @@ -137,3 +137,5 @@ I'm also building select versions of git-annex for both Windows native and WSL1/ development on master. A fine piece of software it definitely is. [[!meta author=jkniiv]] + +> [[fixed|done]] --[[Joey]] diff --git a/doc/bugs/git_2.34__58___some_conflict_resolution_unit_tests_fail/comment_3_84a9988ee5ac6fb4b8ded7aa3164846f._comment b/doc/bugs/git_2.34__58___some_conflict_resolution_unit_tests_fail/comment_3_84a9988ee5ac6fb4b8ded7aa3164846f._comment new file mode 100644 index 0000000000..4e48f50feb --- /dev/null +++ b/doc/bugs/git_2.34__58___some_conflict_resolution_unit_tests_fail/comment_3_84a9988ee5ac6fb4b8ded7aa3164846f._comment @@ -0,0 +1,38 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 3""" + date="2021-11-22T17:31:54Z" + content=""" +On to the second test failure. When there's a conflict between +a symlink and a regular file, the ort resolver uses +the names `foo` and `foo~`. +(Again not using stable naming, alas), and +the files are staged in conflict, one "added by us" +and the other "added by them". + +The old resolver's behavior is to leave a single file +`foo` in conflict state "both added" and containing the content of the +file; the content of the symlink is staged as added by them. + +git-annex's merge conflict resolution does not deal with this well, +because it doesn't know those two files are related. So it sees +a file eg `foo~HEAD` that is in conflict, but the conflict does +not involve an annexed file. So it does not try to resolve that merge, +because resolving a merge not involving an annexed file is out of scope. + +Ugh. I think something has to be done about this, making the test suite +use the old resolver is not sufficient because git-annex is supposed to +recover from this kind of merge conflict. + +Bear in mind that a non-annexed file with a name like `foo~HEAD` +that is in "added by us" state can also happen when a file is modified by +us, and deleted by them. So resolving such a file by adding it makes a +decision that git-annex does not want to make about a non-annexed file. + +So, it seems that to fix this, git-annex will have to somehow learn +that `foo` and `foo~` are the two sides of a merge +conflict. It would have to base that on the filenames that git uses +and the fact that one is a symlink and the other is a normal file. + +Ok, done.. +"""]]