{"thread":{"id":"27694","subject":"''git submodule sync'' should not add uninitialized submodules to .git/config","startedAt":"2011-06-23T11:13:50Z","lastAt":"2011-06-30T00:47:18Z","messageCount":15,"participants":["Maarten Billemont","Phil Hord","Junio C Hamano","Jens Lehmann","Andreas Köhler"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"170493","messageId":"0D2618D7-0681-4E71-B412-36D490D45B9D@gmail.com","threadId":"27694","inReplyTo":null,"subject":"''git submodule sync'' should not add uninitialized submodules to .git/config","fromName":"Maarten Billemont","fromEmail":"lhunath@gmail.com","sentAt":"2011-06-23T11:13:50Z","receivedAt":"2011-06-23T11:13:50Z","isPatch":false,"sender":{"key":"lhunath@gmail.com","avatar":"https://gravatar.com/avatar/bc5b6a8b0cdd6a29c7019afb479286896c37dd293484d641de4bdb186f8a9f80?d=mp&s=160"},"body":"When I initialize 2/3 submodules of my git repository and do git submodule update, all is fine: Only the 2 submodules that I need are updated.\n\nWhen I run a git submodule sync to update the URLs that may have been changed in .gitmodules, it ADDS the URL of the submodule that was NOT initialized, thus \"initializing\" it.\n\nNow, when I run git submodule update, it starts checking out the third module and my workflow is broken.\n\ngit submodule sync should not add entries to .git/config, only update existing ones."},{"id":"170495","messageId":"4E03421E.1000208@cisco.com","threadId":"27694","inReplyTo":"0D2618D7-0681-4E71-B412-36D490D45B9D@gmail.com","subject":"Re: ''git submodule sync'' should not add uninitialized submodules to .git/config","fromName":"Phil Hord","fromEmail":"hordp@cisco.com","sentAt":"2011-06-23T13:39:42Z","receivedAt":"2011-06-23T13:39:42Z","isPatch":false,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On 06/23/2011 07:13 AM, Maarten Billemont wrote:\n> When I initialize 2/3 submodules of my git repository and do git submodule update, all is fine: Only the 2 submodules that I need are updated.\n>\n> When I run a git submodule sync to update the URLs that may have been changed in .gitmodules, it ADDS the URL of the submodule that was NOT initialized, thus \"initializing\" it.\n>\n> Now, when I run git submodule update, it starts checking out the third module and my workflow is broken.\n>\n> git submodule sync should not add entries to .git/config, only update existing ones.--\n\nMaarten,\n\nIs it enough to similarly limit 'submodule sync' the same way you did\nwith 'submodule init'? Like this:\ngit submodule sync -- A B\n\nFrom 'git help submodule':\n\"git submodule sync\" synchronizes all submodules while \"git submodule\nsync — A\" synchronizes submodule \"A\" only.\n\nPhil\n"},{"id":"170496","messageId":"7v7h8c4nv3.fsf@alter.siamese.dyndns.org","threadId":"27694","inReplyTo":"0D2618D7-0681-4E71-B412-36D490D45B9D@gmail.com","subject":"Re: ''git submodule sync'' should not add uninitialized submodules to .git/config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-23T14:35:28Z","receivedAt":"2011-06-23T14:35:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Maarten Billemont <lhunath@gmail.com> writes:\n\n> When I initialize 2/3 submodules of my git repository and do git\n> submodule update, all is fine: Only the 2 submodules that I need are\n> updated.\n>\n> When I run a git submodule sync to update the URLs that may have been\n> changed in .gitmodules, it ADDS the URL of the submodule that was NOT\n> initialized, thus \"initializing\" it.\n>\n> Now, when I run git submodule update, it starts checking out the third\n> module and my workflow is broken.\n\nSee 33f072f (submodule sync: Update \"submodule.<name>.url\" for empty\ndirectories, 2010-10-08), which introduced this behaviour.\n\ncmd_update considers anything that has submodule.<name>.url defined as\n\"the user is interested\", so I suspect \"git submodule sync\" should not do\nthis.\n\nThe situation 33f072f cites as needing this behaviour can easily fixed by\nrunning 'submodule sync' after switching to the branch to which the\nsubmodule _matters_, no?\n\nJens, what do you think?\n"},{"id":"170511","messageId":"4E0390A7.8040505@web.de","threadId":"27694","inReplyTo":"7v7h8c4nv3.fsf@alter.siamese.dyndns.org","subject":"Re: ''git submodule sync'' should not add uninitialized submodules to .git/config","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-06-23T19:14:47Z","receivedAt":"2011-06-23T19:14:47Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 23.06.2011 16:35, schrieb Junio C Hamano:\n> Maarten Billemont <lhunath@gmail.com> writes:\n> \n>> When I initialize 2/3 submodules of my git repository and do git\n>> submodule update, all is fine: Only the 2 submodules that I need are\n>> updated.\n>>\n>> When I run a git submodule sync to update the URLs that may have been\n>> changed in .gitmodules, it ADDS the URL of the submodule that was NOT\n>> initialized, thus \"initializing\" it.\n>>\n>> Now, when I run git submodule update, it starts checking out the third\n>> module and my workflow is broken.\n> \n> See 33f072f (submodule sync: Update \"submodule.<name>.url\" for empty\n> directories, 2010-10-08), which introduced this behaviour.\n> \n> cmd_update considers anything that has submodule.<name>.url defined as\n> \"the user is interested\", so I suspect \"git submodule sync\" should not do\n> this.\n> \n> The situation 33f072f cites as needing this behaviour can easily fixed by\n> running 'submodule sync' after switching to the branch to which the\n> submodule _matters_, no?\n> \n> Jens, what do you think?\n\nI agree that 33f072f introduced a regression. One could argue if it was\na good idea to let \"git submodule init\" not do the clone itself but defer\nit to \"git submodule update\" by setting the url in .git/config, but that's\nthe way things are done now (and maybe there was a very good reason to do\nit that way I'm not aware of, because I didn't follow the list that closely\nback then).\n\nSo while I think 33f072f solved a problem for a valid use case, it breaks\nother use cases that worked so far. So unless we want to change init to do\nthe clone itself (which would be a pretty invasive change in behavior),\nI'd vote for a revert.\n"},{"id":"170516","messageId":"4E039BA7.8060302@cisco.com","threadId":"27694","inReplyTo":"7v7h8c4nv3.fsf@alter.siamese.dyndns.org","subject":"Re: ''git submodule sync'' should not add uninitialized submodules to .git/config","fromName":"Phil Hord","fromEmail":"hordp@cisco.com","sentAt":"2011-06-23T20:01:43Z","receivedAt":"2011-06-23T20:01:43Z","isPatch":false,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On 06/23/2011 10:35 AM, Junio C Hamano wrote:\n> Maarten Billemont <lhunath@gmail.com> writes:\n>> When I initialize 2/3 submodules of my git repository and do git\n>> submodule update, all is fine: Only the 2 submodules that I need are\n>> updated.\n>>\n>> When I run a git submodule sync to update the URLs that may have been\n>> changed in .gitmodules, it ADDS the URL of the submodule that was NOT\n>> initialized, thus \"initializing\" it.\n>>\n>> Now, when I run git submodule update, it starts checking out the third\n>> module and my workflow is broken.\n> See 33f072f (submodule sync: Update \"submodule.<name>.url\" for empty\n> directories, 2010-10-08), which introduced this behaviour.\n>\n> cmd_update considers anything that has submodule.<name>.url defined as\n> \"the user is interested\", so I suspect \"git submodule sync\" should not do\n> this.\n\nWhat about a compromise?  Change git-submodule-sync to skip submodules\nwhich are not already initialized.\n\nI have a patch to do this, but I need to test it still.  If it sounds\nright, I'll try to submit it later today.\n\nPhil\n"},{"id":"170520","messageId":"7vhb7g2pbf.fsf@alter.siamese.dyndns.org","threadId":"27694","inReplyTo":"4E039BA7.8060302@cisco.com","subject":"Re: ''git submodule sync'' should not add uninitialized submodules to .git/config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-23T21:47:00Z","receivedAt":"2011-06-23T21:47:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phil Hord <hordp@cisco.com> writes:\n\n> On 06/23/2011 10:35 AM, Junio C Hamano wrote:\n>>> Now, when I run git submodule update, it starts checking out the third\n>>> module and my workflow is broken.\n>> See 33f072f (submodule sync: Update \"submodule.<name>.url\" for empty\n>> directories, 2010-10-08), which introduced this behaviour.\n>>\n>> cmd_update considers anything that has submodule.<name>.url defined as\n>> \"the user is interested\", so I suspect \"git submodule sync\" should not do\n>> this.\n>\n> What about a compromise?  Change git-submodule-sync to skip submodules\n> which are not already initialized.\n\nYou completely lost me. How's that different from reverting 33f072f? I do\nnot see a room for \"compromise\" here.\n"},{"id":"170524","messageId":"7vboxo2ne9.fsf@alter.siamese.dyndns.org","threadId":"27694","inReplyTo":"4E0390A7.8040505@web.de","subject":"Re: ''git submodule sync'' should not add uninitialized submodules to .git/config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-23T22:28:30Z","receivedAt":"2011-06-23T22:28:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> I agree that 33f072f introduced a regression. One could argue if it was\n> a good idea to let \"git submodule init\" not do the clone itself but defer\n> it to \"git submodule update\" by setting the url in .git/config, but that's\n> the way things are done now (and maybe there was a very good reason to do\n> it that way I'm not aware of, because I didn't follow the list that closely\n> back then).\n\nActually, shouldn't the fix be more like this patch, which is directly on\ntop of 33f072f?  I think this is more in line with what the end user wants\nto tell the system with \"submodule init\", namely \"I am interested in this\nsubmodule\".\n\n-- >8 --\nSubject: submodule sync: do not auto-vivify uninteresting submodule\n\nEarlier 33f072f (submodule sync: Update \"submodule.<name>.url\" for empty\ndirectories, 2010-10-08) attempted to fix a bug where \"git submodule sync\"\ncommand does not update the URL if the current superproject does not have\na checkout of the submodule.\n\nHowever, it did so by unconditionally registering submodule.$name.url to\nevery submodule in the project, even the ones that the user has never\nshowed interest in at all by running 'git submodule init' command. This\ncaused subsequent 'git submodule update' to start cloning/updating submodules\nthat are not interesting to the user at all.\n\nUpdate the code so that the URL is updated from the .gitmodules file only\nfor submodules that already have submodule.$name.url entries, i.e. the\nones the user has showed interested in having a checkout.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n git-submodule.sh          |   23 +++++++++++++----------\n t/t7403-submodule-sync.sh |   13 +++++++++++--\n 2 files changed, 24 insertions(+), 12 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex c291eed..ce65971 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -836,17 +836,20 @@ cmd_sync()\n \t\t\t;;\n \t\tesac\n \n-\t\tsay \"Synchronizing submodule url for '$name'\"\n-\t\tgit config submodule.\"$name\".url \"$url\"\n-\n-\t\tif test -e \"$path\"/.git\n+\t\tif git config \"submodule.$name.url\" >/dev/null 2>/dev/null\n \t\tthen\n-\t\t(\n-\t\t\tclear_local_git_env\n-\t\t\tcd \"$path\"\n-\t\t\tremote=$(get_default_remote)\n-\t\t\tgit config remote.\"$remote\".url \"$url\"\n-\t\t)\n+\t\t\tsay \"Synchronizing submodule url for '$name'\"\n+\t\t\tgit config submodule.\"$name\".url \"$url\"\n+\n+\t\t\tif test -e \"$path\"/.git\n+\t\t\tthen\n+\t\t\t(\n+\t\t\t\tclear_local_git_env\n+\t\t\t\tcd \"$path\"\n+\t\t\t\tremote=$(get_default_remote)\n+\t\t\t\tgit config remote.\"$remote\".url \"$url\"\n+\t\t\t)\n+\t\t\tfi\n \t\tfi\n \tdone\n }\ndiff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh\nindex e5b1953..597a06f 100755\n--- a/t/t7403-submodule-sync.sh\n+++ b/t/t7403-submodule-sync.sh\n@@ -25,7 +25,8 @@ test_expect_success setup '\n \tgit clone super super-clone &&\n \t(cd super-clone && git submodule update --init) &&\n \tgit clone super empty-clone &&\n-\t(cd empty-clone && git submodule init)\n+\t(cd empty-clone && git submodule init) &&\n+\tgit clone super top-only-clone\n '\n \n test_expect_success 'change submodule' '\n@@ -66,7 +67,7 @@ test_expect_success '\"git submodule sync\" should update submodule URLs' '\n \t)\n '\n \n-test_expect_success '\"git submodule sync\" should update submodule URLs if not yet cloned' '\n+test_expect_success '\"git submodule sync\" should update known submodule URLs' '\n \t(cd empty-clone &&\n \t git pull &&\n \t git submodule sync &&\n@@ -74,4 +75,12 @@ test_expect_success '\"git submodule sync\" should update submodule URLs if not ye\n \t)\n '\n \n+test_expect_success '\"git submodule sync\" should not vivify uninteresting submodule' '\n+\t(cd top-only-clone &&\n+\t git pull &&\n+\t git submodule sync &&\n+\t test -z \"$(git config submodule.submodule.url)\"\n+\t)\n+'\n+\n test_done\n"},{"id":"170525","messageId":"4E03C6F7.2010506@cisco.com","threadId":"27694","inReplyTo":"7vhb7g2pbf.fsf@alter.siamese.dyndns.org","subject":"Re: ''git submodule sync'' should not add uninitialized submodules to .git/config","fromName":"Phil Hord","fromEmail":"hordp@cisco.com","sentAt":"2011-06-23T23:06:31Z","receivedAt":"2011-06-23T23:06:31Z","isPatch":false,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On 06/23/2011 05:47 PM, Junio C Hamano wrote:\n> Phil Hord <hordp@cisco.com> writes:\n>\n>> On 06/23/2011 10:35 AM, Junio C Hamano wrote:\n>>>> Now, when I run git submodule update, it starts checking out the third\n>>>> module and my workflow is broken.\n>>> See 33f072f (submodule sync: Update \"submodule.<name>.url\" for empty\n>>> directories, 2010-10-08), which introduced this behaviour.\n>>>\n>>> cmd_update considers anything that has submodule.<name>.url defined as\n>>> \"the user is interested\", so I suspect \"git submodule sync\" should not do\n>>> this.\n>> What about a compromise?  Change git-submodule-sync to skip submodules\n>> which are not already initialized.\n> You completely lost me. How's that different from reverting 33f072f? I do\n> not see a room for \"compromise\" here.\n\nYour patch does what I meant, so I assume you figured out my compromise.\n\nMy patch was simpler, but I haven't tested it properly yet.\n\nPhil\n"},{"id":"170526","messageId":"4E03C7E5.4050008@gmx.net","threadId":"27694","inReplyTo":"7vboxo2ne9.fsf@alter.siamese.dyndns.org","subject":"Re: ''git submodule sync'' should not add uninitialized submodules to .git/config","fromName":"Andreas Köhler","fromEmail":"andi5.py@gmx.net","sentAt":"2011-06-23T23:10:29Z","receivedAt":"2011-06-23T23:10:29Z","isPatch":false,"sender":{"key":"andi5.py@gmx.net","avatar":null},"body":"Am 24.06.2011 00:28, schrieb Junio C Hamano:\n---8<---\n> Subject: submodule sync: do not auto-vivify uninteresting submodule\n> \n> Earlier 33f072f (submodule sync: Update \"submodule.<name>.url\" for empty\n> directories, 2010-10-08) attempted to fix a bug where \"git submodule sync\"\n> command does not update the URL if the current superproject does not have\n> a checkout of the submodule.\n> \n> However, it did so by unconditionally registering submodule.$name.url to\n> every submodule in the project, even the ones that the user has never\n> showed interest in at all by running 'git submodule init' command. This\n> caused subsequent 'git submodule update' to start cloning/updating submodules\n> that are not interesting to the user at all.\n> \n> Update the code so that the URL is updated from the .gitmodules file only\n> for submodules that already have submodule.$name.url entries, i.e. the\n> ones the user has showed interested in having a checkout.\n---8<---\n\nI think that describes the situation pretty well and the patch looks\ngood. I will not be able to test it before next week though, do not wait\nfor me.\n\nThere are two minor issues for me left.\n(1) The man page of submodule sync talks about \"all submodules\" and not\n\"registered submodules\". I treated it as \"subsequent init\" and\npersonally do not restrict the list of submodules, simply because my\nbuild would not work without them.\n\n(2) It is confusing to have registered submodules in .git/config without\na matching gitlink. Say, you switch between branch a1 with a submodule s\nwith url u1, to a branch without s, you git clean -xdff (url u1 is still\nin .git/config, right?), and then to a branch a2 with s pointing to url\nu2. This bugged me.\n\nCiao,\n-- andi5\n"},{"id":"170527","messageId":"7vy60r27et.fsf_-_@alter.siamese.dyndns.org","threadId":"27694","inReplyTo":"7vboxo2ne9.fsf@alter.siamese.dyndns.org","subject":"Re* ''git submodule sync'' should not add uninitialized submodules to .git/config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-24T04:13:46Z","receivedAt":"2011-06-24T04:13:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Actually, shouldn't the fix be more like this patch, which is directly on\n> top of 33f072f?  I think this is more in line with what the end user wants\n> to tell the system with \"submodule init\", namely \"I am interested in this\n> submodule\".\n\nThis, when merged with jl/submodule-add-relurl-wo-upstream topic in \"pu\",\nbreaks a test in t7400, which was introduced by 4d68932 (submodule add:\nallow relative repository path even when no url is set, 2011-06-06).\n\nA workaround patch (shown offset below) to make it \"pass\" exposes that the\ntest was relying on the too aggressive \"submodule sync\" behaviour I just\nfixed with the previous patch.\n\nAnd I do not think the particular workaround is correct.\n\nShouldn't \"submodule add\" add an entry for .git/config even when it cloned\nfrom elsewhere?\n\nWhen adding a local subdirectory that houses the subproject with\n\"submodule add\", the code does add an entry to .git/config, but when the\nmoule is cloned from elsewhere (by calling module_clone), nobody adds the\ncorresponding entry to .git/config.  \"submodule add $URL $path\" should\nresult in exactly the same state as if the user said \"submodule init\n$path\" and then \"submodule update $path\" to instantiate the path in a\nsuperproject that was cloned from elsewhere, no?\n\nI suspect this fix will cascade to breakage elsewhere, but I've run out of\nenergy and inclination to look at the submodule code tonight, so I'll let\nthe list to take it further from here.\n\n-- >8 --\nSubject: [RFC] submodule add: initialize .git/config entry\n\nWhen \"git submodule add $path\" is run to add a subdirectory $path to the\nsuperproject, and $path is already the top of the working tree of the\nsubmodule repository, the command created submodule.$path.url entry in the\nconfiguration file in the superproject. However, when adding a repository\n$URL that is outside the respository of the superproject to $path that\ndoes not exist (yet) with \"git submodule add $URL $path\", the command\nforgot to set it up.\n\nThe user is expressing the interest in the submodule and wants to keep a\ncheckout; the \"submodule add\" command should consistently set up the\nsubmodule.$path.url entry in either case.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n    (A workaround patch that may not be a correct fix for the test breakage)\n\n        diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\n        index 9099e80..cfe81e3 100755\n        --- a/t/t7400-submodule-basic.sh\n        +++ b/t/t7400-submodule-basic.sh\n        @@ -446,16 +446,17 @@ test_expect_success 'add should fail when pat...\n                )\n         '\n\n         test_expect_success 'use superproject as upstream when path is rel...\n                (\n                        cd addtest &&\n                        git submodule add ../repo relative &&\n                        test \"$(git config -f .gitmodules submodule.relativ...\n        +\t\tgit submodule init relative &&\n                        git submodule sync relative &&\n                        test \"$(git config submodule.relative.url)\" = \"$sub...\n                )\n         '\n\n         test_expect_success 'set up for relative path tests' '\n                mkdir reltest &&\n                (\n\n git-submodule.sh |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex b0a8e63..8a16ad1 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -238,7 +238,6 @@ cmd_add()\n \t\t\tdie \"'$path' already exists and is not a valid git repo\"\n \t\tfi\n \n-\t\tgit config submodule.\"$path\".url \"$realrepo\"\n \telse\n \n \t\tmodule_clone \"$path\" \"$realrepo\" \"$reference\" || exit\n@@ -253,6 +252,8 @@ cmd_add()\n \t\t) || die \"Unable to checkout submodule '$path'\"\n \tfi\n \n+\tgit config submodule.\"$path\".url \"$realrepo\"\n+\n \tgit add $force \"$path\" ||\n \tdie \"Failed to add submodule '$path'\"\n \n"},{"id":"170528","messageId":"4E042BFA.3020909@web.de","threadId":"27694","inReplyTo":"4E03C7E5.4050008@gmx.net","subject":"Re: ''git submodule sync'' should not add uninitialized submodules to .git/config","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-06-24T06:17:30Z","receivedAt":"2011-06-24T06:17:30Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 24.06.2011 01:10, schrieb Andreas Köhler:\n> There are two minor issues for me left.\n> (1) The man page of submodule sync talks about \"all submodules\" and not\n> \"registered submodules\". I treated it as \"subsequent init\" and\n> personally do not restrict the list of submodules, simply because my\n> build would not work without them.\n\nWhile investigating this I stumbled across that too, will post a patch.\n\n> (2) It is confusing to have registered submodules in .git/config without\n> a matching gitlink. Say, you switch between branch a1 with a submodule s\n> with url u1, to a branch without s, you git clean -xdff (url u1 is still\n> in .git/config, right?), and then to a branch a2 with s pointing to url\n> u2. This bugged me.\n\nI think that is ok, because that way git remembers that the user cares\nabout submodule s even when he switches to a branch where s doesn't\nexist. And the documentation is pretty clear that you'll have to use\n\"git submodule sync\" to update the url itself from u1 to u2 when needed.\n"},{"id":"170530","messageId":"4E042CA6.6040309@web.de","threadId":"27694","inReplyTo":"7vy60r27et.fsf_-_@alter.siamese.dyndns.org","subject":"Re: Re* ''git submodule sync'' should not add uninitialized submodules to .git/config","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-06-24T06:20:22Z","receivedAt":"2011-06-24T06:20:22Z","isPatch":false,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 24.06.2011 06:13, schrieb Junio C Hamano:\n> I suspect this fix will cascade to breakage elsewhere, but I've run out of\n> energy and inclination to look at the submodule code tonight, so I'll let\n> the list to take it further from here.\n\nI'll won't have much git time today but I'll look into that over the\nweekend.\n"},{"id":"170544","messageId":"4E0647F5.2070802@web.de","threadId":"27694","inReplyTo":"7vboxo2ne9.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] submodule sync: do not auto-vivify uninteresting submodule","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-06-25T20:41:25Z","receivedAt":"2011-06-25T20:41:25Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"From: Junio C Hamano <gitster@pobox.com>\n\nEarlier 33f072f (submodule sync: Update \"submodule.<name>.url\" for empty\ndirectories, 2010-10-08) attempted to fix a bug where \"git submodule sync\"\ncommand does not update the URL if the current superproject does not have\na checkout of the submodule.\n\nHowever, it did so by unconditionally registering submodule.$name.url to\nevery submodule in the project, even the ones that the user has never\nshowed interest in at all by running 'git submodule init' command. This\ncaused subsequent 'git submodule update' to start cloning/updating submodules\nthat are not interesting to the user at all.\n\nUpdate the code so that the URL is updated from the .gitmodules file only\nfor submodules that already have submodule.$name.url entries, i.e. the\nones the user has showed interested in having a checkout.\n\nAcked-by: Jens Lehmann <Jens.Lehmann@web.de>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\nAm 24.06.2011 00:28, schrieb Junio C Hamano:\n> Actually, shouldn't the fix be more like this patch, which is directly on\n> top of 33f072f?  I think this is more in line with what the end user wants\n> to tell the system with \"submodule init\", namely \"I am interested in this\n> submodule\".\n\nYes, I am convinced your patch is the doing Right Thing. I squashed in a\nchange to the git submodule sync documentation (explicitly stating that a\nsync only affects submodules which have an url entry in .git/config) and\ntwo more lines in the test you added to make sure that explicitly giving\na submodule on the command line uses the same logic.\n\nWhat do you think?\n\n(I'll send another patch shortly addressing the failing tests when\njl/submodule-add-relurl-wo-upstream is merged into this one)\n\n\n Documentation/git-submodule.txt |    4 +++-\n git-submodule.sh                |   23 +++++++++++++----------\n t/t7403-submodule-sync.sh       |   15 +++++++++++++--\n 3 files changed, 29 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 5e7a413..2b31d5f 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -167,7 +167,9 @@ commit for each submodule.\n\n sync::\n \tSynchronizes submodules' remote URL configuration setting\n-\tto the value specified in .gitmodules.  This is useful when\n+\tto the value specified in .gitmodules. It will only affect those\n+\tsubmodules which already have an url entry in .git/config (that is the\n+\tcase when they are initialized or freshly added). This is useful when\n \tsubmodule URLs change upstream and you need to update your local\n \trepositories accordingly.\n +\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex d189a24..543f1d0 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -874,17 +874,20 @@ cmd_sync()\n \t\t\t;;\n \t\tesac\n\n-\t\tsay \"Synchronizing submodule url for '$name'\"\n-\t\tgit config submodule.\"$name\".url \"$url\"\n-\n-\t\tif test -e \"$path\"/.git\n+\t\tif git config \"submodule.$name.url\" >/dev/null 2>/dev/null\n \t\tthen\n-\t\t(\n-\t\t\tclear_local_git_env\n-\t\t\tcd \"$path\"\n-\t\t\tremote=$(get_default_remote)\n-\t\t\tgit config remote.\"$remote\".url \"$url\"\n-\t\t)\n+\t\t\tsay \"Synchronizing submodule url for '$name'\"\n+\t\t\tgit config submodule.\"$name\".url \"$url\"\n+\n+\t\t\tif test -e \"$path\"/.git\n+\t\t\tthen\n+\t\t\t(\n+\t\t\t\tclear_local_git_env\n+\t\t\t\tcd \"$path\"\n+\t\t\t\tremote=$(get_default_remote)\n+\t\t\t\tgit config remote.\"$remote\".url \"$url\"\n+\t\t\t)\n+\t\t\tfi\n \t\tfi\n \tdone\n }\ndiff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh\nindex d600583..95ffe34 100755\n--- a/t/t7403-submodule-sync.sh\n+++ b/t/t7403-submodule-sync.sh\n@@ -25,7 +25,8 @@ test_expect_success setup '\n \tgit clone super super-clone &&\n \t(cd super-clone && git submodule update --init) &&\n \tgit clone super empty-clone &&\n-\t(cd empty-clone && git submodule init)\n+\t(cd empty-clone && git submodule init) &&\n+\tgit clone super top-only-clone\n '\n\n test_expect_success 'change submodule' '\n@@ -66,7 +67,7 @@ test_expect_success '\"git submodule sync\" should update submodule URLs' '\n \t)\n '\n\n-test_expect_success '\"git submodule sync\" should update submodule URLs if not yet cloned' '\n+test_expect_success '\"git submodule sync\" should update known submodule URLs' '\n \t(cd empty-clone &&\n \t git pull &&\n \t git submodule sync &&\n@@ -74,4 +75,14 @@ test_expect_success '\"git submodule sync\" should update submodule URLs if not ye\n \t)\n '\n\n+test_expect_success '\"git submodule sync\" should not vivify uninteresting submodule' '\n+\t(cd top-only-clone &&\n+\t git pull &&\n+\t git submodule sync &&\n+\t test -z \"$(git config submodule.submodule.url)\" &&\n+\t git submodule sync submodule &&\n+\t test -z \"$(git config submodule.submodule.url)\"\n+\t)\n+'\n+\n test_done\n-- \n1.7.6.rc3.1.g8fd6\n"},{"id":"170545","messageId":"4E066E8A.7060209@web.de","threadId":"27694","inReplyTo":"7vy60r27et.fsf_-_@alter.siamese.dyndns.org","subject":"[PATCH] submodule add: always initialize .git/config entry","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2011-06-25T23:26:02Z","receivedAt":"2011-06-25T23:26:02Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"When \"git submodule add $path\" is run to add a subdirectory $path to the\nsuperproject, and $path is already the top of the working tree of the\nsubmodule repository, the command created submodule.$path.url entry in the\nconfiguration file in the superproject. However, when adding a repository\n$URL that is outside the respository of the superproject to $path that\ndoes not exist (yet) with \"git submodule add $URL $path\", the command\nforgot to set it up.\n\nThe user is expressing the interest in the submodule and wants to keep a\ncheckout, the \"submodule add\" command should consistently set up the\nsubmodule.$path.url entry in either case.\n\nAs a result \"git submodule init\" can't simply skip the initialization of\nthose submodules for which it finds an url entry in the git./config\nanymore. That lead to problems when adding a submodule (which now sets the\nurl), add the \"update\" setting to .gitmodules and expect init to copy that\ninto .git/config like it is done in t7406. So change init to only then\ncopy the \"url\" and \"update\" entries when they don't exist yet in the\n.git/config and do nothing otherwise.\n\nSigned-off-by: Jens Lehmann <Jens.Lehmann@web.de>\n---\n\nAm 24.06.2011 06:13, schrieb Junio C Hamano:\n> Shouldn't \"submodule add\" add an entry for .git/config even when it cloned\n> from elsewhere?\n\nYes, we should be consistent here.\n\n> I suspect this fix will cascade to breakage elsewhere, but I've run out of\n> energy and inclination to look at the submodule code tonight, so I'll let\n> the list to take it further from here.\n\nOk, t7406 expected \"git submodule init\" to copy the new update setting\ninto .git/config for a newly added submodule, which it didn't do anymore\nbecause it already found the url set. I solved that by teaching init to\nonly then copy the url and update settings if they aren't present yet.\nNow all tests are running fine and your change to the test I added in\njl/submodule-add-relurl-wo-upstream isn't necessary anymore.\n\nWhen I cherry pick that onto cbd0a3c6bc in your current pu branch and\nresolve the conflicts all tests run fine (if you want me to resend this\npatch based on that commit to avoid the conflicts with i18n and the\n\"submodule add: clean up duplicated code\" patch please just say so).\n\n\n git-submodule.sh |   33 +++++++++++++++++----------------\n 1 files changed, 17 insertions(+), 16 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 543f1d0..7e39c97 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -246,7 +246,6 @@ cmd_add()\n \t\t\turl=\"$repo\"\n \t\t\t;;\n \t\tesac\n-\t\tgit config submodule.\"$path\".url \"$url\"\n \telse\n\n \t\tmodule_clone \"$path\" \"$realrepo\" \"$reference\" || exit\n@@ -260,6 +259,7 @@ cmd_add()\n \t\t\tesac\n \t\t) || die \"Unable to checkout submodule '$path'\"\n \tfi\n+\tgit config submodule.\"$path\".url \"$url\"\n\n \tgit add $force \"$path\" ||\n \tdie \"Failed to add submodule '$path'\"\n@@ -355,25 +355,26 @@ cmd_init()\n \tdo\n \t\t# Skip already registered paths\n \t\tname=$(module_name \"$path\") || exit\n-\t\turl=$(git config submodule.\"$name\".url)\n-\t\ttest -z \"$url\" || continue\n-\n-\t\turl=$(git config -f .gitmodules submodule.\"$name\".url)\n-\t\ttest -z \"$url\" &&\n-\t\tdie \"No url found for submodule path '$path' in .gitmodules\"\n-\n-\t\t# Possibly a url relative to parent\n-\t\tcase \"$url\" in\n-\t\t./*|../*)\n-\t\t\turl=$(resolve_relative_url \"$url\") || exit\n-\t\t\t;;\n-\t\tesac\n-\n-\t\tgit config submodule.\"$name\".url \"$url\" ||\n-\t\tdie \"Failed to register url for submodule path '$path'\"\n+\t\tif test -z \"$(git config \"submodule.$name.url\")\"\n+\t\tthen\n+\t\t\turl=$(git config -f .gitmodules submodule.\"$name\".url)\n+\t\t\ttest -z \"$url\" &&\n+\t\t\tdie \"No url found for submodule path '$path' in .gitmodules\"\n+\n+\t\t\t# Possibly a url relative to parent\n+\t\t\tcase \"$url\" in\n+\t\t\t./*|../*)\n+\t\t\t\turl=$(resolve_relative_url \"$url\") || exit\n+\t\t\t\t;;\n+\t\t\tesac\n+\t\t\tgit config submodule.\"$name\".url \"$url\" ||\n+\t\t\tdie \"Failed to register url for submodule path '$path'\"\n+\t\tfi\n\n+\t\t# Copy \"update\" setting when it is not set yet\n \t\tupd=\"$(git config -f .gitmodules submodule.\"$name\".update)\"\n \t\ttest -z \"$upd\" ||\n+\t\ttest -n \"$(git config submodule.\"$name\".update)\" ||\n \t\tgit config submodule.\"$name\".update \"$upd\" ||\n \t\tdie \"Failed to register update mode for submodule path '$path'\"\n\n-- \n1.7.6.rc3.2.gcfa18\n"},{"id":"170689","messageId":"7vwrg4qh61.fsf@alter.siamese.dyndns.org","threadId":"27694","inReplyTo":"4E066E8A.7060209@web.de","subject":"Re: [PATCH] submodule add: always initialize .git/config entry","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-30T00:47:18Z","receivedAt":"2011-06-30T00:47:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann <Jens.Lehmann@web.de> writes:\n\n> Am 24.06.2011 06:13, schrieb Junio C Hamano:\n>> Shouldn't \"submodule add\" add an entry for .git/config even when it cloned\n>> from elsewhere?\n>\n> Yes, we should be consistent here.\n>\n>> I suspect this fix will cascade to breakage elsewhere, but I've run out of\n>> energy and inclination to look at the submodule code tonight, so I'll let\n>> the list to take it further from here.\n>\n> Ok, t7406 expected \"git submodule init\" to copy the new update setting\n> into .git/config for a newly added submodule, which it didn't do anymore\n> because it already found the url set. I solved that by teaching init to\n> only then copy the url and update settings if they aren't present yet.\n> Now all tests are running fine and your change to the test I added in\n> jl/submodule-add-relurl-wo-upstream isn't necessary anymore.\n>\n> When I cherry pick that onto cbd0a3c6bc in your current pu branch and\n> resolve the conflicts all tests run fine (if you want me to resend this\n> patch based on that commit to avoid the conflicts with i18n and the\n> \"submodule add: clean up duplicated code\" patch please just say so).\n\nHmm, now 7610 seems to expect somewhat different behaviour and fails.\n"}]}