Re: bug: checkout --recurse-submodules discard uncommited changes in submodule

[Philippe Blain <levraiphilippeblain@gmail.com>]

Hi Kevin,

Le 2021-01-14 à 03:35, Kevin Bernard a écrit :
> Thank you for filling out a Git bug report!
> Please answer the following questions to help us understand your issue.
> 
> What did you do before the bug happened? (Steps to reproduce your issue)
> Make a git repository with a submodule in it.
> Make two different commits in the main repository with two different versions of
> the submodule, one of them is the head of the branch.
> Checkout the head of the branch in the main repository, and make a submodule update.
> Make a modification, do not commit it, in a submodule file that will not result
> in a "error: Your local changes to the following files would be overwritten by
> checkout" when the other version of the library will be checked out.

I assume that you confirmed that by running 'git checkout <other version>'
in the submodule, right ?

> Go back to the main repository and make a checkout of the other commit with the
> switch --recurse-submodules.
> 
> What did you expect to happen? (Expected behavior)
> The checkout with the switch --recurse-submodules should fail when there are
> uncommitted changes in the submodule.
> 
> What happened instead? (Actual behavior)
> Uncommitted changes in the submodule are discarded without any notifications.
> 
> What's different between what you expected and what actually happened?
> Loss of the uncommitted changes in the submodule.
> 
> Anything else you want to add:
> I stay at your disposal if you need more information.


Thanks for the report. This is a known problem that was reported in [1] and [2].
I'm currently working on fixing that bug. I'm not quite finished yet as I want
to polish the end-user experience a bit, but if you want to compile from source,
the heart of the fix is this change:

diff --git a/unpack-trees.c b/unpack-trees.c
index 323280dd48..a3e3d98de1 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1872,7 +1872,7 @@ static int verify_uptodate_1(const struct cache_entry *ce,
  
  		if (submodule_from_ce(ce)) {
  			int r = check_submodule_move_head(ce,
-				"HEAD", oid_to_hex(&ce->oid), o);
+				"HEAD", empty_tree_oid_hex(), o);
  			if (r)
  				return add_rejected_path(o, error_type, ce->name);
  			return 0;


This should disallow switching branches with '--recurse-submodules' if *any* file
in the submodule is modified.

However, I'm actually thinking that maybe it would
be better to let the checkout succeed in the exact case you mention, i.e. when
the modified files in the submodules are at the same version in both submodule
commits (i.e. git checkout <other version> in the submodule succeeds and the
modified files stay modified), and also carry over the modified files in the
submodule. This would mimic the 'git checkout' experience
without submodules, i.e. modified files that do not conflict are carried over
to the branch you are checking out.

If you can write a complete reproducer (complete steps starting with 'git init',
etc.) I will make sure that your scenario is adequately tested in my patch series.


Cheers,

Philippe.


[1] https://lore.kernel.org/git/CAHsG2VT4YB_nf8PrEmrHwK-iY-AQo0VDcvXGVsf8cEYXws4nig@mail.gmail.com/
[2] https://lore.kernel.org/git/20200525094019.22padbzuk7ukr5uv@overdrive.tratt.net/