When on an adjusted branch, git annex sync
will sync the submodule
commit for the initial addition of a submodule, but it doesn't seem to
sync changes in the recorded commit. Here's a demo of the issue based
off of Michael's script at DataLad's issue 4292.
cd "$(mktemp -d --tmpdir gx-sync-demo-XXXXXXX)"
git init
git annex init
git commit --allow-empty -mc0
git annex adjust --unlock
git init sub
git -C sub commit --allow-empty -mc0
git submodule add ./sub
git commit -m'add sub'
# A new submodule is sync'd.
git annex sync --no-commit --no-push --no-pull --no-content
git -C sub commit --allow-empty -mc1
git add sub
git commit -m'sub update'
# A change in the submodule ID is not.
git annex sync --no-commit --no-push --no-pull --no-content
git diff master..
With a git-annex built from 8.20200309-62-gc8fec6ab0, the submodule
remains modified between master
and adjusted/master(unlocked))
.
diff --git a/sub b/sub
index ed67764..a27fd83 160000
--- a/sub
+++ b/sub
@@ -1 +1 @@
-Subproject commit ed6776462d0842bf0a317d2e3ee1983572a217ff
+Subproject commit a27fd837958466ba4fb751a3de139c03548be1ad
I was able to resolve the issue (i.e. the above demo outputs an empty diff) with the patch below, though that might of course be the completely wrong way to fix this.
Thanks in advance for looking into this issue.
From fa3a4be5497300b9b39e121008b474e9dd405fd3 Mon Sep 17 00:00:00 2001
From: Kyle Meyer <kyle@kyleam.com>
Date: Tue, 17 Mar 2020 22:16:06 -0400
Subject: [PATCH] adjust: Propagate submodule changes back to original branch
When the recorded submodule commit changes on an adjusted branch, the
change is carried in the function that reverseAdjustedCommit passes
for adjustTree's adjusttreeitem parameter. Update the CommitObject
handling in adjustTree to consider adjusttreeitem so that a submodule
change is synced back.
---
Git/Tree.hs | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/Git/Tree.hs b/Git/Tree.hs
index da05a3fa5..9627ae969 100644
--- a/Git/Tree.hs
+++ b/Git/Tree.hs
@@ -237,8 +237,12 @@ adjustTree adjusttreeitem addtreeitems resolveaddconflict removefiles r repo =
let !modified' = modified || slmodified || wasmodified
go h modified' (subtree : c) depth intree is'
Just CommitObject -> do
- let ti = TreeCommit (LsTree.file i) (LsTree.mode i) (LsTree.sha i)
- go h wasmodified (ti:c) depth intree is
+ let ti = TreeItem (LsTree.file i) (LsTree.mode i) (LsTree.sha i)
+ v <- adjusttreeitem ti
+ let commit = tc $ fromMaybe ti v
+ go h wasmodified (commit:c) depth intree is
+ where
+ tc (TreeItem f m s) = TreeCommit f m s
_ -> error ("unexpected object type \"" ++ decodeBS (LsTree.typeobj i) ++ "\"")
| otherwise = return (c, wasmodified, i:is)
--
2.25.1
Little bit of a scary change, because calls to adjustTree could be not handling the case of a submodule correctly in their adjusttreeitem function.
So I checked them all..
Command.Export uses adjustTree with an adjusttreeitem function that runs catKey on the sha from the TreeItem. I think that would be ok, because catKey will see it's not a key, and in that case it passes back the TreeItem unchanged.
Each different adjusted branch has its own function. Most of those check if the TreeItem is a symlink, with a submodule is not, and they'll return it unchanged.
The PresenceAdjustment instead uses catKey, and again looks ok, because it falls back to returning the TreeItem unchanged.
The LockAdjustment, when the TreeItem is not a symlink, also uses catKey, which will see it's not a key, and also falls back to returning the TreeItem unchanged.
Think that's everything, so this seems a safe change to make.
But.. I don't actually understand how your change fixes the problem!
(Of course, I tried your patch, and it does work...)
I've been staring at it for 30 minutes and it seems to me that the TreeItem you have it construct gets passed to adjusttreeitem, which always returns it unchanged (per analysis above). Then you deconstruct the TreeItem, extracting the file mode and sha, and construct a TreeCommit from those. As far as I can see, that TreeCommit must be identical to the one it constructed before this patch.
Hmm, so I added a debug print and it's not the same, the sha is different.
What am I missing?
Ok, figured it out.. reverseAdjustedTree uses a
propchanges
function that looks at the diff from adjusted branch to basis, and substitutes the new sha for the old one. So that's how the sha get changed.My analysis was otherwise correct, I think. And your patch is fine, now that I understand that. Applying it as-is.
add --force-small
ones. Very much appreciated!