Re* ''git submodule sync'' should not add uninitialized submodules to .git/config

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Actually, shouldn't the fix be more like this patch, which is directly on
> top of 33f072f?  I think this is more in line with what the end user wants
> to tell the system with "submodule init", namely "I am interested in this
> submodule".

This, when merged with jl/submodule-add-relurl-wo-upstream topic in "pu",
breaks a test in t7400, which was introduced by 4d68932 (submodule add:
allow relative repository path even when no url is set, 2011-06-06).

A workaround patch (shown offset below) to make it "pass" exposes that the
test was relying on the too aggressive "submodule sync" behaviour I just
fixed with the previous patch.

And I do not think the particular workaround is correct.

Shouldn't "submodule add" add an entry for .git/config even when it cloned
from elsewhere?

When adding a local subdirectory that houses the subproject with
"submodule add", the code does add an entry to .git/config, but when the
moule is cloned from elsewhere (by calling module_clone), nobody adds the
corresponding entry to .git/config.  "submodule add $URL $path" should
result in exactly the same state as if the user said "submodule init
$path" and then "submodule update $path" to instantiate the path in a
superproject that was cloned from elsewhere, no?

I suspect this fix will cascade to breakage elsewhere, but I've run out of
energy and inclination to look at the submodule code tonight, so I'll let
the list to take it further from here.

-- >8 --
Subject: [RFC] submodule add: initialize .git/config entry

When "git submodule add $path" is run to add a subdirectory $path to the
superproject, and $path is already the top of the working tree of the
submodule repository, the command created submodule.$path.url entry in the
configuration file in the superproject. However, when adding a repository
$URL that is outside the respository of the superproject to $path that
does not exist (yet) with "git submodule add $URL $path", the command
forgot to set it up.

The user is expressing the interest in the submodule and wants to keep a
checkout; the "submodule add" command should consistently set up the
submodule.$path.url entry in either case.

Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---

    (A workaround patch that may not be a correct fix for the test breakage)

        diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
        index 9099e80..cfe81e3 100755
        --- a/t/t7400-submodule-basic.sh
        +++ b/t/t7400-submodule-basic.sh
        @@ -446,16 +446,17 @@ test_expect_success 'add should fail when pat...
                )
         '

         test_expect_success 'use superproject as upstream when path is rel...
                (
                        cd addtest &&
                        git submodule add ../repo relative &&
                        test "$(git config -f .gitmodules submodule.relativ...
        +		git submodule init relative &&
                        git submodule sync relative &&
                        test "$(git config submodule.relative.url)" = "$sub...
                )
         '

         test_expect_success 'set up for relative path tests' '
                mkdir reltest &&
                (

 git-submodule.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index b0a8e63..8a16ad1 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -238,7 +238,6 @@ cmd_add()
 			die "'$path' already exists and is not a valid git repo"
 		fi
 
-		git config submodule."$path".url "$realrepo"
 	else
 
 		module_clone "$path" "$realrepo" "$reference" || exit
@@ -253,6 +252,8 @@ cmd_add()
 		) || die "Unable to checkout submodule '$path'"
 	fi
 
+	git config submodule."$path".url "$realrepo"
+
 	git add $force "$path" ||
 	die "Failed to add submodule '$path'"
 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]