diff --submodule=diff: do not fail on ever-initialied deleted submodules
If you have ever initialized a submodule, open_submodule will open it. If you then delete the submodule's worktree directory (but don't remove it from .gitmodules), git diff --submodule=diff would error out as it attempted to chdir into the now-deleted working tree directory. This only matters if the submodules git dir is absorbed. If not, then we no longer have anywhere to run the diff. But that case does not trigger this error, because in that case, open_submodule fails, so we don't resolve a left commit, so we exit early, which is the only thing we could do. If absorbed, then we can run the diff from the submodule's absorbed git dir (.git/modules/sm2). In practice, that's a bit more complicated, because `git diff` expects to be run from inside a working directory, not a git dir. So it looks in the config for core.worktree, and does chdir("../../../sm2"), which is the very dir that we're trying to avoid visiting because it's been deleted. We work around this by setting GIT_WORK_TREE (and GIT_DIR) to ".". It is little weird to set GIT_WORK_TREE to something that is not a working tree just to avoid an unnecessary chdir, but it works. Signed-off-by: David Turner <dturner@twosigma.com Signed-off-by: Junio C Hamano <gitster@pobox.com>
David Turner committed
Aug 31, 2021 at 09:12 UTC
f1c0368da4d64f394e4c099cb3c232671040d725
2 files changed
+161
-7
submodule.c
+10
@@ -710,6 +710,16 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,
710
strvec_push(&cp.args, oid_to_hex(new_oid));
711
712
prepare_submodule_repo_env(&cp.env_array);
713
+
714
+ if (!is_directory(path)) {
715
+ /* fall back to absorbed git dir, if any */
716
+ if (!sub)
717
+ goto done;
718
+ cp.dir = sub->gitdir;
719
+ strvec_push(&cp.env_array, GIT_DIR_ENVIRONMENT "=.");
720
+ strvec_push(&cp.env_array, GIT_WORK_TREE_ENVIRONMENT "=.");
721
+ }
722
+
723
if (start_command(&cp))
724
diff_emit_submodule_error(o, "(diff failed)\n");
725
t/t4060-diff-submodule-option-diff-format.sh
+151
-7
@@ -703,10 +703,26 @@ test_expect_success 'path filter' '
703
diff_cmp expected actual
704
'
705
706
-commit_file sm2
706
+cat >.gitmodules <<-EOF
707
+[submodule "sm2"]
708
+ path = sm2
709
+ url = bogus_url
710
+EOF
711
+git add .gitmodules
712
+commit_file sm2 .gitmodules
713
+
714
test_expect_success 'given commit' '
715
git diff-index -p --submodule=diff HEAD^ >actual &&
716
cat >expected <<-EOF &&
717
+ diff --git a/.gitmodules b/.gitmodules
718
+ new file mode 100644
719
+ index 1234567..89abcde
720
+ --- /dev/null
721
+ +++ b/.gitmodules
722
+ @@ -0,0 +1,3 @@
723
+ +[submodule "sm2"]
724
+ +path = sm2
725
+ +url = bogus_url
726
Submodule sm1 $head7...0000000 (submodule deleted)
727
Submodule sm2 0000000...$head9 (new submodule)
728
diff --git a/sm2/foo8 b/sm2/foo8
@@ -728,15 +744,21 @@ test_expect_success 'given commit' '
744
'
745
746
test_expect_success 'setup .git file for sm2' '
731
- (cd sm2 &&
732
- REAL="$(pwd)/../.real" &&
733
- mv .git "$REAL" &&
734
- echo "gitdir: $REAL" >.git)
747
+ git submodule absorbgitdirs sm2
748
'
749
750
test_expect_success 'diff --submodule=diff with .git file' '
751
git diff --submodule=diff HEAD^ >actual &&
752
cat >expected <<-EOF &&
753
+ diff --git a/.gitmodules b/.gitmodules
754
+ new file mode 100644
755
+ index 1234567..89abcde
756
+ --- /dev/null
757
+ +++ b/.gitmodules
758
+ @@ -0,0 +1,3 @@
759
+ +[submodule "sm2"]
760
+ +path = sm2
761
+ +url = bogus_url
762
Submodule sm1 $head7...0000000 (submodule deleted)
763
Submodule sm2 0000000...$head9 (new submodule)
764
diff --git a/sm2/foo8 b/sm2/foo8
@@ -757,9 +779,67 @@ test_expect_success 'diff --submodule=diff with .git file' '
779
diff_cmp expected actual
780
'
781
782
+mv sm2 sm2-bak
783
+
784
+test_expect_success 'deleted submodule with .git file' '
785
+ git diff-index -p --submodule=diff HEAD >actual &&
786
+ cat >expected <<-EOF &&
787
+ Submodule sm1 $head7...0000000 (submodule deleted)
788
+ Submodule sm2 $head9...0000000 (submodule deleted)
789
+ diff --git a/sm2/foo8 b/sm2/foo8
790
+ deleted file mode 100644
791
+ index 1234567..89abcde
792
+ --- a/sm2/foo8
793
+ +++ /dev/null
794
+ @@ -1 +0,0 @@
795
+ -foo8
796
+ diff --git a/sm2/foo9 b/sm2/foo9
797
+ deleted file mode 100644
798
+ index 1234567..89abcde
799
+ --- a/sm2/foo9
800
+ +++ /dev/null
801
+ @@ -1 +0,0 @@
802
+ -foo9
803
+ EOF
804
+ diff_cmp expected actual
805
+'
806
+
807
+echo submodule-to-blob>sm2
808
+
809
+test_expect_success 'typechanged(submodule->blob) submodule with .git file' '
810
+ git diff-index -p --submodule=diff HEAD >actual &&
811
+ cat >expected <<-EOF &&
812
+ Submodule sm1 $head7...0000000 (submodule deleted)
813
+ Submodule sm2 $head9...0000000 (submodule deleted)
814
+ diff --git a/sm2/foo8 b/sm2/foo8
815
+ deleted file mode 100644
816
+ index 1234567..89abcde
817
+ --- a/sm2/foo8
818
+ +++ /dev/null
819
+ @@ -1 +0,0 @@
820
+ -foo8
821
+ diff --git a/sm2/foo9 b/sm2/foo9
822
+ deleted file mode 100644
823
+ index 1234567..89abcde
824
+ --- a/sm2/foo9
825
+ +++ /dev/null
826
+ @@ -1 +0,0 @@
827
+ -foo9
828
+ diff --git a/sm2 b/sm2
829
+ new file mode 100644
830
+ index 1234567..89abcde
831
+ --- /dev/null
832
+ +++ b/sm2
833
+ @@ -0,0 +1 @@
834
+ +submodule-to-blob
835
+ EOF
836
+ diff_cmp expected actual
837
+'
838
+
839
+rm sm2
840
+mv sm2-bak sm2
841
+
842
test_expect_success 'setup nested submodule' '
761
- git submodule add -f ./sm2 &&
762
- git commit -a -m "add sm2" &&
843
git -C sm2 submodule add ../sm2 nested &&
844
git -C sm2 commit -a -m "nested sub" &&
845
head10=$(git -C sm2 rev-parse --short --verify HEAD)
@@ -790,6 +870,7 @@ test_expect_success 'diff --submodule=diff with moved nested submodule HEAD' '
870
871
test_expect_success 'diff --submodule=diff recurses into nested submodules' '
872
cat >expected <<-EOF &&
873
+ Submodule sm1 $head7...0000000 (submodule deleted)
874
Submodule sm2 contains modified content
875
Submodule sm2 $head9..$head10:
876
diff --git a/sm2/.gitmodules b/sm2/.gitmodules
@@ -829,4 +910,67 @@ test_expect_success 'diff --submodule=diff recurses into nested submodules' '
910
diff_cmp expected actual
911
'
912
913
+(cd sm2; commit_file nested)
914
+commit_file sm2
915
+head12=$(cd sm2; git rev-parse --short --verify HEAD)
916
+
917
+mv sm2 sm2-bak
918
+
919
+test_expect_success 'diff --submodule=diff recurses into deleted nested submodules' '
920
+ cat >expected <<-EOF &&
921
+ Submodule sm1 $head7...0000000 (submodule deleted)
922
+ Submodule sm2 $head12...0000000 (submodule deleted)
923
+ diff --git a/sm2/.gitmodules b/sm2/.gitmodules
924
+ deleted file mode 100644
925
+ index 3a816b8..0000000
926
+ --- a/sm2/.gitmodules
927
+ +++ /dev/null
928
+ @@ -1,3 +0,0 @@
929
+ -[submodule "nested"]
930
+ - path = nested
931
+ - url = ../sm2
932
+ diff --git a/sm2/foo8 b/sm2/foo8
933
+ deleted file mode 100644
934
+ index db9916b..0000000
935
+ --- a/sm2/foo8
936
+ +++ /dev/null
937
+ @@ -1 +0,0 @@
938
+ -foo8
939
+ diff --git a/sm2/foo9 b/sm2/foo9
940
+ deleted file mode 100644
941
+ index 9c3b4f6..0000000
942
+ --- a/sm2/foo9
943
+ +++ /dev/null
944
+ @@ -1 +0,0 @@
945
+ -foo9
946
+ Submodule nested $head11...0000000 (submodule deleted)
947
+ diff --git a/sm2/nested/file b/sm2/nested/file
948
+ deleted file mode 100644
949
+ index ca281f5..0000000
950
+ --- a/sm2/nested/file
951
+ +++ /dev/null
952
+ @@ -1 +0,0 @@
953
+ -nested content
954
+ diff --git a/sm2/nested/foo8 b/sm2/nested/foo8
955
+ deleted file mode 100644
956
+ index db9916b..0000000
957
+ --- a/sm2/nested/foo8
958
+ +++ /dev/null
959
+ @@ -1 +0,0 @@
960
+ -foo8
961
+ diff --git a/sm2/nested/foo9 b/sm2/nested/foo9
962
+ deleted file mode 100644
963
+ index 9c3b4f6..0000000
964
+ --- a/sm2/nested/foo9
965
+ +++ /dev/null
966
+ @@ -1 +0,0 @@
967
+ -foo9
968
+ EOF
969
+ git diff --submodule=diff >actual 2>err &&
970
+ test_must_be_empty err &&
971
+ diff_cmp expected actual
972
+'
973
+
974
+mv sm2-bak sm2
975
+
976
test_done