@matheusbsbs / matheus-bagliano / commits / 505a276596

submodules: fix of regression on fetching of non-init subsub-repo

A regression has been introduced by a62387b (submodule.c: fetch in submodules git directory instead of in worktree, 2018-11-28). The scenario in which it triggers is when one has a repository with a submodule inside a submodule like this: superproject/middle_repo/inner_repo Person A and B have both a clone of it, while Person B is not working with the inner_repo and thus does not have it initialized in his working copy. Now person A introduces a change to the inner_repo and propagates it through the middle_repo and the superproject. Once person A pushed the changes and person B wants to fetch them using "git fetch" at the superproject level, B's git call will return with error saying: Could not access submodule 'inner_repo' Errors during submodule fetch: middle_repo Expectation is that in this case the inner submodule will be recognized as uninitialized submodule and skipped by the git fetch command. This used to work correctly before 'a62387b (submodule.c: fetch in submodules git directory instead of in worktree, 2018-11-28)'. Starting with a62387b the code wants to evaluate "is_empty_dir()" inside .git/modules for a directory only existing in the worktree, delivering then of course wrong return value. This patch ensures is_empty_dir() is getting the correct path of the uninitialized submodule by concatenation of the actual worktree and the name of the uninitialized submodule. The first attempt to fix this regression, in 1b7ac4e6d4 (submodules: fix of regression on fetching of non-init subsub-repo, 2020-11-12), by simply reverting a62387b, resulted in an infinite loop of submodule fetches in the simpler case of a recursive fetch of a superproject with uninitialized submodules, and so this commit was reverted in 7091499bc0 (Revert "submodules: fix of regression on fetching of non-init subsub-repo", 2020-12-02). To prevent future breakages, also add a regression test for this scenario. Signed-off-by: Peter Kaestle <peter.kaestle@nokia.com> CC: Junio C Hamano <gitster@pobox.com> CC: Philippe Blain <levraiphilippeblain@gmail.com> CC: Ralf Thielow <ralf.thielow@gmail.com> CC: Eric Sunshine <sunshine@sunshineco.us> Reviewed-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Peter Kaestle committed Dec 9, 2020 at 11:58 UTC 505a27659638614157a36b218fdaf25fe9fed0ce

2 files changed +123 -1

submodule.c

+6 -1

@@ -1477,6 +1477,7 @@ static int get_next_submodule(struct child_process *cp,

1477 strbuf_release(&submodule_prefix);

1478 return 1;

1479 } else {

1480 + struct strbuf empty_submodule_path = STRBUF_INIT;

1481

1482 fetch_task_release(task);

1483 free(task);

@@ -1485,13 +1486,17 @@ static int get_next_submodule(struct child_process *cp,

1486 * An empty directory is normal,

1487 * the submodule is not initialized

1488 */

1489 + strbuf_addf(&empty_submodule_path, "%s/%s/",

1490 + spf->r->worktree,

1491 + ce->name);

1492 if (S_ISGITLINK(ce->ce_mode) &&

1489 - !is_empty_dir(ce->name)) {

1493 + !is_empty_dir(empty_submodule_path.buf)) {

1494 spf->result = 1;

1495 strbuf_addf(err,

1496 _("Could not access submodule '%s'\n"),

1497 ce->name);

1498 }

1499 + strbuf_release(&empty_submodule_path);

1500 }

1501 }

1502

t/t5526-fetch-submodules.sh

+117

@@ -719,4 +719,121 @@ test_expect_success 'fetch new submodule commit intermittently referenced by sup

719 )

720 '

721

722 +add_commit_push () {

723 + dir="$1" &&

724 + msg="$2" &&

725 + shift 2 &&

726 + git -C "$dir" add "$@" &&

727 + git -C "$dir" commit -a -m "$msg" &&

728 + git -C "$dir" push

729 +}

730 +

731 +compare_refs_in_dir () {

732 + fail= &&

733 + if test "x$1" = 'x!'

734 + then

735 + fail='!' &&

736 + shift

737 + fi &&

738 + git -C "$1" rev-parse --verify "$2" >expect &&

739 + git -C "$3" rev-parse --verify "$4" >actual &&

740 + eval $fail test_cmp expect actual

741 +}

742 +

743 +

744 +test_expect_success 'setup nested submodule fetch test' '

745 + # does not depend on any previous test setups

746 +

747 + for repo in outer middle inner

748 + do

749 + git init --bare $repo &&

750 + git clone $repo ${repo}_content &&

751 + echo "$repo" >"${repo}_content/file" &&

752 + add_commit_push ${repo}_content "initial" file ||

753 + return 1

754 + done &&

755 +

756 + git clone outer A &&

757 + git -C A submodule add "$pwd/middle" &&

758 + git -C A/middle/ submodule add "$pwd/inner" &&

759 + add_commit_push A/middle/ "adding inner sub" .gitmodules inner &&

760 + add_commit_push A/ "adding middle sub" .gitmodules middle &&

761 +

762 + git clone outer B &&

763 + git -C B/ submodule update --init middle &&

764 +

765 + compare_refs_in_dir A HEAD B HEAD &&

766 + compare_refs_in_dir A/middle HEAD B/middle HEAD &&

767 + test_path_is_file B/file &&

768 + test_path_is_file B/middle/file &&

769 + test_path_is_missing B/middle/inner/file &&

770 +

771 + echo "change on inner repo of A" >"A/middle/inner/file" &&

772 + add_commit_push A/middle/inner "change on inner" file &&

773 + add_commit_push A/middle "change on inner" inner &&

774 + add_commit_push A "change on inner" middle

775 +'

776 +

777 +test_expect_success 'fetching a superproject containing an uninitialized sub/sub project' '

778 + # depends on previous test for setup

779 +

780 + git -C B/ fetch &&

781 + compare_refs_in_dir A origin/HEAD B origin/HEAD

782 +'

783 +

784 +fetch_with_recursion_abort () {

785 + # In a regression the following git call will run into infinite recursion.

786 + # To handle that, we connect the sed command to the git call by a pipe

787 + # so that sed can kill the infinite recursion when detected.

788 + # The recursion creates git output like:

789 + # Fetching submodule sub

790 + # Fetching submodule sub/sub <-- [1]

791 + # Fetching submodule sub/sub/sub

792 + # ...

793 + # [1] sed will stop reading and cause git to eventually stop and die

794 +

795 + git -C "$1" fetch --recurse-submodules 2>&1 |

796 + sed "/Fetching submodule $2[^$]/q" >out &&

797 + ! grep "Fetching submodule $2[^$]" out

798 +}

799 +

800 +test_expect_success 'setup recursive fetch with uninit submodule' '

801 + # does not depend on any previous test setups

802 +

803 + test_create_repo super &&

804 + test_commit -C super initial &&

805 + test_create_repo sub &&

806 + test_commit -C sub initial &&

807 + git -C sub rev-parse HEAD >expect &&

808 +

809 + git -C super submodule add ../sub &&

810 + git -C super commit -m "add sub" &&

811 +

812 + git clone super superclone &&

813 + git -C superclone submodule status >out &&

814 + sed -e "s/^-//" -e "s/ sub.*$//" out >actual &&

815 + test_cmp expect actual

816 +'

817 +

818 +test_expect_success 'recursive fetch with uninit submodule' '

819 + # depends on previous test for setup

820 +

821 + fetch_with_recursion_abort superclone sub &&

822 + git -C superclone submodule status >out &&

823 + sed -e "s/^-//" -e "s/ sub$//" out >actual &&

824 + test_cmp expect actual

825 +'

826 +

827 +test_expect_success 'recursive fetch after deinit a submodule' '

828 + # depends on previous test for setup

829 +

830 + git -C superclone submodule update --init sub &&

831 + git -C superclone submodule deinit -f sub &&

832 +

833 + fetch_with_recursion_abort superclone sub &&

834 + git -C superclone submodule status >out &&

835 + sed -e "s/^-//" -e "s/ sub$//" out >actual &&

836 + test_cmp expect actual

837 +'

838 +

839 test_done