{"thread":{"id":"42135","subject":"[RFC] How to pass Git config command line instructions to Submodule commands?","startedAt":"2016-04-25T10:39:48Z","lastAt":"2016-04-28T16:51:43Z","messageCount":36,"participants":["Lars Schneider","Stefan Beller","Jacob Keller","Jeff King","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"284389","messageId":"60724588-B06E-47E8-9302-8709C4601826@gmail.com","threadId":"42135","inReplyTo":null,"subject":"[RFC] How to pass Git config command line instructions to Submodule commands?","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-04-25T10:39:48Z","receivedAt":"2016-04-25T10:39:48Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"Hi,\n\na few folks from the Git LFS project and I try to make cloning of repositories \nwith a lot of LFS files faster. \n\nThe core problem is that Git LFS uses a Git smudge filter to replace LFS \npointers with the actual file content. Right now, a smudge filter can only \nbe executed on an individual file which makes the operation slow for many \nfiles [1].\n\nWe solved this issue by temporarily disabling the smudge filter for the clone \ncommand via Git config (optimized in 1a8630 [2]):\n\n git -c filter.lfs.smudge= -c filter.lfs.required=false clone \n\nAfterwards Git LFS runs a special command to download and replace all LFS \ncontent in bulk [3]. This works great for LFS repositories.\n\nHowever, I noticed that git config command line instructions such as \n\"-c filter.lfs.smudge=\" are not passed to Git submodule operations. Thus\nthis does not work as expected:\n\n git -c filter.lfs.smudge= -c filter.lfs.required=false clone --recursive \n\nI tried to work around that by copying the relevant pieced from the Git \nSubmodule command [4] and applying the command line Git config\nmanually (look closely at the modified checkout command):\n\n git -c filter.lfs.smudge= -c filter.lfs.required=false clone $@\n if [[ -z $2 ]]; then\n CLONE_PATH=$(basename ${1%.git});\n else\n CLONE_PATH=$2;\n fi\n pushd \"$CLONE_PATH\"\n git submodule init\n wt_prefix=$(git rev-parse --show-prefix)\n git submodule--helper list --prefix \"$wt_prefix\" | {\n while read mode sha1 stage sm_path\n do\n name=$(git submodule--helper name \"$sm_path\") || exit\n url=$(git config submodule.\"$name\".url)\n if ! test -d \"$sm_path\"/.git && ! test -f \"$sm_path\"/.git\n then\n git submodule--helper clone --prefix \"$wt_prefix\" --path \"$sm_path\" --name \"$name\" --url \"$url\"\n pushd \"$sm_path\"\n git -c filter.lfs.smudge= -c filter.lfs.required=false checkout -q $sha1 || exit\n git-lfs pull || exit\n popd\n fi\n done\n }\n popd\n\nDo you see an easier way to pass command line Git config instructions to the \nunderlaying Git Submodule commands? If not, do you think a patch adding this\nwould be worth working on?\n\nI also started a discussion about that on the Git LFS issue page [5].\n\nThanks,\nLars\n\n\n[1] https://github.com/github/git-lfs/issues/931\n[2] https://github.com/git/git/commit/1a8630dc3b1cc6f1361a4e5d94630133c24c97d9\n[3] https://developer.atlassian.com/blog/2016/04/git-lfs-12-clone-faster/\n[4] https://github.com/git/git/blob/6a6636270fbaf74609cd3e1bd207dd2c420d640a/git-submodule.sh#L686-L731\n[5] https://github.com/github/git-lfs/issues/1172\n"},{"id":"284447","messageId":"CAGZ79kYmAr-O6_Jw2KO9eZEfZQ+_WBiERF=nhOYLJCZpUjSSyA@mail.gmail.com","threadId":"42135","inReplyTo":"60724588-B06E-47E8-9302-8709C4601826@gmail.com","subject":"Re: [RFC] How to pass Git config command line instructions to Submodule commands?","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-25T17:02:53Z","receivedAt":"2016-04-25T17:02:53Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Apr 25, 2016 at 3:39 AM, Lars Schneider\n wrote:\n> Hi,\n>\n> a few folks from the Git LFS project and I try to make cloning of repositories\n> with a lot of LFS files faster.\n>\n> The core problem is that Git LFS uses a Git smudge filter to replace LFS\n> pointers with the actual file content. Right now, a smudge filter can only\n> be executed on an individual file which makes the operation slow for many\n> files [1].\n>\n> We solved this issue by temporarily disabling the smudge filter for the clone\n> command via Git config (optimized in 1a8630 [2]):\n>\n> git -c filter.lfs.smudge= -c filter.lfs.required=false clone \n>\n> Afterwards Git LFS runs a special command to download and replace all LFS\n> content in bulk [3]. This works great for LFS repositories.\n>\n> However, I noticed that git config command line instructions such as\n> \"-c filter.lfs.smudge=\" are not passed to Git submodule operations. Thus\n> this does not work as expected:\n>\n> git -c filter.lfs.smudge= -c filter.lfs.required=false clone --recursive \n\nI have cc'd Jacob Keller, who authored origin/jk/submodule-c-credential,\nwhich does work in that area (deciding which config option to pass down\ninto the submodule commands).\n\n>\n> I tried to work around that by copying the relevant pieced from the Git\n> Submodule command [4] and applying the command line Git config\n> manually (look closely at the modified checkout command):\n>\n> git -c filter.lfs.smudge= -c filter.lfs.required=false clone $@\n> if [[ -z $2 ]]; then\n> CLONE_PATH=$(basename ${1%.git});\n> else\n> CLONE_PATH=$2;\n> fi\n> pushd \"$CLONE_PATH\"\n> git submodule init\n> wt_prefix=$(git rev-parse --show-prefix)\n> git submodule--helper list --prefix \"$wt_prefix\" | {\n> while read mode sha1 stage sm_path\n> do\n> name=$(git submodule--helper name \"$sm_path\") || exit\n> url=$(git config submodule.\"$name\".url)\n> if ! test -d \"$sm_path\"/.git && ! test -f \"$sm_path\"/.git\n> then\n> git submodule--helper clone --prefix \"$wt_prefix\" --path \"$sm_path\" --name \"$name\" --url \"$url\"\n\nThe init and then clone should be covered by\n\"git submodule--helper update-clone\", which may be better named as\n\"list-and-clone-if-necessary\", then you get parallel cloning for free\nas well. ;)\n\n> pushd \"$sm_path\"\n> git -c filter.lfs.smudge= -c filter.lfs.required=false checkout -q $sha1 || exit\n> git-lfs pull || exit\n> popd\n> fi\n> done\n> }\n> popd\n>\n> Do you see an easier way to pass command line Git config instructions to the\n> underlaying Git Submodule commands? If not, do you think a patch adding this\n> would be worth working on?\n\nI would build on top of origin/jk/submodule-c-credential at least, and using\n\"git submodule--helper update-clone\" (origin/sb/submodule-parallel-update)\n\n>\n> I also started a discussion about that on the Git LFS issue page [5].\n\nUnrelated to this, but about LFS:\nCurrently it is only possible to store the big blobs at a $VENDOR hosting site.\nHow would you backup you whole repository including all binariers?\n\nI think we should add an option to store one blob in a dedicated ref\n(refs/lfs/$blobsha1 or such). That way you can make a backup of the\nrepository including all large files using \"git clone --mirror\" and\nyou don't have\nto rely on the $VENDOR hosting your files.\n\nThanks,\nStefan\n\n>\n> Thanks,\n> Lars\n>\n>\n> [1] https://github.com/github/git-lfs/issues/931\n> [2] https://github.com/git/git/commit/1a8630dc3b1cc6f1361a4e5d94630133c24c97d9\n> [3] https://developer.atlassian.com/blog/2016/04/git-lfs-12-clone-faster/\n> [4] https://github.com/git/git/blob/6a6636270fbaf74609cd3e1bd207dd2c420d640a/git-submodule.sh#L686-L731\n> [5] https://github.com/github/git-lfs/issues/1172\n>\n"},{"id":"284478","messageId":"CA+P7+xoaqNF+uBHVnD2QR7j-=0Hyvd-scTc_vOdV+etC0VS9jA@mail.gmail.com","threadId":"42135","inReplyTo":"CAGZ79kYmAr-O6_Jw2KO9eZEfZQ+_WBiERF=nhOYLJCZpUjSSyA@mail.gmail.com","subject":"Re: [RFC] How to pass Git config command line instructions to Submodule commands?","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-04-25T20:59:03Z","receivedAt":"2016-04-25T20:59:03Z","isPatch":false,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Mon, Apr 25, 2016 at 10:02 AM, Stefan Beller wrote:\n> On Mon, Apr 25, 2016 at 3:39 AM, Lars Schneider\n> wrote:\n>> Hi,\n>>\n>> a few folks from the Git LFS project and I try to make cloning of repositories\n>> with a lot of LFS files faster.\n>>\n>> The core problem is that Git LFS uses a Git smudge filter to replace LFS\n>> pointers with the actual file content. Right now, a smudge filter can only\n>> be executed on an individual file which makes the operation slow for many\n>> files [1].\n>>\n>> We solved this issue by temporarily disabling the smudge filter for the clone\n>> command via Git config (optimized in 1a8630 [2]):\n>>\n>> git -c filter.lfs.smudge= -c filter.lfs.required=false clone \n>>\n>> Afterwards Git LFS runs a special command to download and replace all LFS\n>> content in bulk [3]. This works great for LFS repositories.\n>>\n>> However, I noticed that git config command line instructions such as\n>> \"-c filter.lfs.smudge=\" are not passed to Git submodule operations. Thus\n>> this does not work as expected:\n>>\n>> git -c filter.lfs.smudge= -c filter.lfs.required=false clone --recursive \n>\n> I have cc'd Jacob Keller, who authored origin/jk/submodule-c-credential,\n> which does work in that area (deciding which config option to pass down\n> into the submodule commands).\n>\n\nThis is a tricky question. The problem is that some configurations are\nobviously not intended to go into the submodules, but determining how\nis somewhat troublesome. There was some discussion on this previous\nthread when we added support for credential options to pass through.\n\nThanks,\nJake\n"},{"id":"284486","messageId":"20160425212449.GA7636@sigill.intra.peff.net","threadId":"42135","inReplyTo":"CA+P7+xoaqNF+uBHVnD2QR7j-=0Hyvd-scTc_vOdV+etC0VS9jA@mail.gmail.com","subject":"Re: [RFC] How to pass Git config command line instructions to Submodule commands?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-25T21:24:50Z","receivedAt":"2016-04-25T21:24:50Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 25, 2016 at 01:59:03PM -0700, Jacob Keller wrote:\n\n> >> However, I noticed that git config command line instructions such as\n> >> \"-c filter.lfs.smudge=\" are not passed to Git submodule operations. Thus\n> >> this does not work as expected:\n> >>\n> >> git -c filter.lfs.smudge= -c filter.lfs.required=false clone --recursive \n> >\n> > I have cc'd Jacob Keller, who authored origin/jk/submodule-c-credential,\n> > which does work in that area (deciding which config option to pass down\n> > into the submodule commands).\n> >\n> \n> This is a tricky question. The problem is that some configurations are\n> obviously not intended to go into the submodules, but determining how\n> is somewhat troublesome. There was some discussion on this previous\n> thread when we added support for credential options to pass through.\n\nRight. I think it may be reasonable to pass through filter.* in the\nwhitelist. They are not activated without a matching .gitattributes\nentry in the repository (and people would generally configure them in\ntheir user-level ~/.gitconfig for that reason).\n\nIt does mean that somebody would be stuck who really wanted to run the\nsmudge filter in their local repo, but for some reason not in the\nsubrepos. I am trying to think of a case in which that might be\nsecurity-relevant if you didn't trust the sub-repos[1]. But I really\ndon't see it. The filter is arbitrary code, but that's specified by the\nuser; we're just feeding it possibly untrusted blobs.\n\n-Peff\n"},{"id":"284488","messageId":"20160425212709.GB7636@sigill.intra.peff.net","threadId":"42135","inReplyTo":"20160425212449.GA7636@sigill.intra.peff.net","subject":"Re: [RFC] How to pass Git config command line instructions to Submodule commands?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-25T21:27:09Z","receivedAt":"2016-04-25T21:27:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 25, 2016 at 05:24:50PM -0400, Jeff King wrote:\n\n> It does mean that somebody would be stuck who really wanted to run the\n> smudge filter in their local repo, but for some reason not in the\n> subrepos. I am trying to think of a case in which that might be\n> security-relevant if you didn't trust the sub-repos[1]. But I really\n> don't see it. The filter is arbitrary code, but that's specified by the\n> user; we're just feeding it possibly untrusted blobs.\n\nI forgot my [1], which was going to be: I wonder if there are any\ninteresting things you can do by feeding git-lfs untrusted content\n(e.g., convincing it to hit arbitrary URLs). But I don't think so. The\nURL is derived from the remote, and the LFS pointer files just contain\nhashes.\n\nThat's all orthogonal to this thread anyway, though. People using LFS\ngenerally have the config in ~/.gitconfig, so they run it for all repos,\ntrusted and untrusted.\n\n-Peff\n"},{"id":"284785","messageId":"017AA7DB-5224-49C3-A4A6-6C93005BF006@gmail.com","threadId":"42135","inReplyTo":"20160425212449.GA7636@sigill.intra.peff.net","subject":"Re: [RFC] How to pass Git config command line instructions to Submodule commands?","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-04-28T11:06:45Z","receivedAt":"2016-04-28T11:06:45Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 25 Apr 2016, at 23:24, Jeff King wrote:\n> \n> On Mon, Apr 25, 2016 at 01:59:03PM -0700, Jacob Keller wrote:\n> \n>>>> However, I noticed that git config command line instructions such as\n>>>> \"-c filter.lfs.smudge=\" are not passed to Git submodule operations. Thus\n>>>> this does not work as expected:\n>>>> \n>>>> git -c filter.lfs.smudge= -c filter.lfs.required=false clone --recursive \n>>> \n>>> I have cc'd Jacob Keller, who authored origin/jk/submodule-c-credential,\n>>> which does work in that area (deciding which config option to pass down\n>>> into the submodule commands).\n>>> \n>> \n>> This is a tricky question. The problem is that some configurations are\n>> obviously not intended to go into the submodules, but determining how\n>> is somewhat troublesome. There was some discussion on this previous\n>> thread when we added support for credential options to pass through.\n> \n> Right. I think it may be reasonable to pass through filter.* in the\n> whitelist. They are not activated without a matching .gitattributes\n> entry in the repository (and people would generally configure them in\n> their user-level ~/.gitconfig for that reason).\n> \n> It does mean that somebody would be stuck who really wanted to run the\n> smudge filter in their local repo, but for some reason not in the\n> subrepos. I am trying to think of a case in which that might be\n> security-relevant if you didn't trust the sub-repos[1]. But I really\n> don't see it. The filter is arbitrary code, but that's specified by the\n> user; we're just feeding it possibly untrusted blobs.\n\nThis looks like a very promising solution and I can't think of a\nsecurity problem either (I checked the older thread [1] you \nreferenced, too)!\n\nI got my Git-LFS use case working with the patch below. \nFor me it was necessary to export GIT_CONFIG_PARAMETERS\nto make it available to the Git process if the process is \ninvoked as follows [2]: \n\n(sanitize_submodule_env; cd \"$sm_path\" && git \")\n\n\nExporting the variable would not be necessary in this case:\n\n(cd \"$sm_path\" && sanitize_submodule_env git \")\n\nUnfortunately that does not work and I think the reason is that \nclear_local_git_env (invoked by sanitize_submodule_env) clears \nthe $GIT_DIR, too.\n\n\nIf there is a reason against exporting GIT_CONFIG_PARAMETERS,\nthen this would work, too:\n\n(sanitize_submodule_env; cd \"$sm_path\" && GIT_CONFIG_PARAMETERS=$GIT_CONFIG_PARAMETERS git )\n\n\nWould the patch below be an acceptable solution?\n\n\nThanks,\nLars\n\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/264840\n[2] https://github.com/git/git/blob/3ad15fd5e17bbb73fb1161ff4e9c3ed254d5b243/git-submodule.sh#L811\n\n\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 3bd6883..9231089 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -129,6 +129,8 @@ static int submodule_config_ok(const char *var)\n {\n \tif (starts_with(var, \"credential.\"))\n \t\treturn 1;\n+\tif (starts_with(var, \"filter.\"))\n+\t\treturn 1;\n \treturn 0;\n }\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 2a84d7e..b02f5b9 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -199,7 +199,7 @@ sanitize_submodule_env()\n {\n \tsanitized_config=$(git submodule--helper sanitize-config)\n \tclear_local_git_env\n-\tGIT_CONFIG_PARAMETERS=$sanitized_config\n+\texport GIT_CONFIG_PARAMETERS=$sanitized_config\n }\n\n #\n"},{"id":"284786","messageId":"20160428112511.GA11522@sigill.intra.peff.net","threadId":"42135","inReplyTo":"017AA7DB-5224-49C3-A4A6-6C93005BF006@gmail.com","subject":"Re: [RFC] How to pass Git config command line instructions to Submodule commands?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-28T11:25:11Z","receivedAt":"2016-04-28T11:25:11Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 28, 2016 at 01:06:45PM +0200, Lars Schneider wrote:\n\n> I got my Git-LFS use case working with the patch below. \n> For me it was necessary to export GIT_CONFIG_PARAMETERS\n> to make it available to the Git process if the process is \n> invoked as follows [2]: \n> \n> (sanitize_submodule_env; cd \"$sm_path\" && git \")\n\nHrm. I'm not sure why you need to export. Or perhaps, I am not sure why\nit ever works in the first place in git-submodule.sh. In this code:\n\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 2a84d7e..b02f5b9 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -199,7 +199,7 @@ sanitize_submodule_env()\n> {\n> \tsanitized_config=$(git submodule--helper sanitize-config)\n> \tclear_local_git_env\n> -\tGIT_CONFIG_PARAMETERS=$sanitized_config\n> +\texport GIT_CONFIG_PARAMETERS=$sanitized_config\n> }\n\nIf you already have $GIT_CONFIG_PARAMETERS exported when we enter the\nfunction, then we should not need to re-export it when changing the\nvalue in the final line (the export bit is retained by the shell). But\nif you don't have it set already, then $sanitized_config must by\ndefinition be empty.\n\nSo it should do the right thing without the export.\n\nAt the same time, clear_local_git_env() will call \"unset\" on\nGIT_CONFIG_PARAMETERS. Which would clear the export bit, meaning the\nfinal line doesn't ever have any impact on sub-programs, and the whole\nthing is totally broken. But then, why does the test in t5550 pass?\n\nConfused...\n\n-Peff\n"},{"id":"284791","messageId":"20160428120504.GA22399@sigill.intra.peff.net","threadId":"42135","inReplyTo":"20160428112511.GA11522@sigill.intra.peff.net","subject":"Re: [RFC] How to pass Git config command line instructions to Submodule commands?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-28T12:05:04Z","receivedAt":"2016-04-28T12:05:04Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 28, 2016 at 07:25:11AM -0400, Jeff King wrote:\n\n> > diff --git a/git-submodule.sh b/git-submodule.sh\n> > index 2a84d7e..b02f5b9 100755\n> > --- a/git-submodule.sh\n> > +++ b/git-submodule.sh\n> > @@ -199,7 +199,7 @@ sanitize_submodule_env()\n> > {\n> > \tsanitized_config=$(git submodule--helper sanitize-config)\n> > \tclear_local_git_env\n> > -\tGIT_CONFIG_PARAMETERS=$sanitized_config\n> > +\texport GIT_CONFIG_PARAMETERS=$sanitized_config\n> > }\n> \n> If you already have $GIT_CONFIG_PARAMETERS exported when we enter the\n> function, then we should not need to re-export it when changing the\n> value in the final line (the export bit is retained by the shell). But\n> if you don't have it set already, then $sanitized_config must by\n> definition be empty.\n> \n> So it should do the right thing without the export.\n> \n> At the same time, clear_local_git_env() will call \"unset\" on\n> GIT_CONFIG_PARAMETERS. Which would clear the export bit, meaning the\n> final line doesn't ever have any impact on sub-programs, and the whole\n> thing is totally broken. But then, why does the test in t5550 pass?\n> \n> Confused...\n\nAh. t5550 passes because it does not exercise this code path at all. We\ntry a recursive clone, which calls \"git submodule update --init\", which\ndoes not seem to clear the config at all. So it works even without\n14111fc49.\n\nI tried to improve the test by adding git-fetch (note that I also fixed\na bug where we use $HTTP_URL instead of $HTTPD_URL, and added some\nwhitespace to make the result more readable):\n\ndiff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\nindex 69ef388..6ec3ba3 100755\n--- a/t/t5550-http-fetch-dumb.sh\n+++ b/t/t5550-http-fetch-dumb.sh\n@@ -103,12 +103,23 @@ test_expect_success 'cmdline credential config passes into submodules' '\n \t\tgit submodule add \"$HTTPD_URL/auth/dumb/repo.git\" sub &&\n \t\tgit commit -m \"add submodule\"\n \t) &&\n+\n \tset_askpass wrong pass@host &&\n \ttest_must_fail git clone --recursive super super-clone &&\n \trm -rf super-clone &&\n+\n \tset_askpass wrong pass@host &&\n-\tgit -c \"credential.$HTTP_URL.username=user@host\" \\\n+\tgit -c \"credential.$HTTPD_URL.username=user@host\" \\\n \t\tclone --recursive super super-clone &&\n+\texpect_askpass pass user@host &&\n+\n+\tset_askpass wrong pass@host &&\n+\ttest_must_fail git -C super-clone fetch --recurse-submodules &&\n+\n+\tset_askpass wrong pass@host &&\n+\tgit -C super-clone \\\n+\t -c \"credential.$HTTPD_URL.username=user@host\" \\\n+\t fetch --recurse-submodules &&\n \texpect_askpass pass user@host\n '\n \n\nbut that doesn't pass, even with the export fix! That's because fetch\ndoesn't go through git-submodule at all; it calls \"git fetch\" itself,\nand uses local_repo_env, which clears the config. It needs to learn to\nuse the same mechanism that sanitize_submodule_env() does.\n\nSo AFAICT 14111fc49 is totally broken. It doesn't actually work for\ngit-submodule (because of the missing export), nor for git-fetch\n(because that skips the shell script), and the one case we are testing\nalready worked without it (but probably _should_ be sanitizing the\nconfig, so is buggy, too).\n\n-Peff\n"},{"id":"284792","messageId":"FF931AB2-2381-4DF9-AE28-CE30EEC5C352@gmail.com","threadId":"42135","inReplyTo":"20160428112511.GA11522@sigill.intra.peff.net","subject":"Re: [RFC] How to pass Git config command line instructions to Submodule commands?","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-04-28T12:05:20Z","receivedAt":"2016-04-28T12:05:20Z","isPatch":false,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 28 Apr 2016, at 13:25, Jeff King wrote:\n> \n> On Thu, Apr 28, 2016 at 01:06:45PM +0200, Lars Schneider wrote:\n> \n>> I got my Git-LFS use case working with the patch below. \n>> For me it was necessary to export GIT_CONFIG_PARAMETERS\n>> to make it available to the Git process if the process is \n>> invoked as follows [2]: \n>> \n>> (sanitize_submodule_env; cd \"$sm_path\" && git \")\n> \n> Hrm. I'm not sure why you need to export. Or perhaps, I am not sure why\n> it ever works in the first place in git-submodule.sh. In this code:\n> \n>> diff --git a/git-submodule.sh b/git-submodule.sh\n>> index 2a84d7e..b02f5b9 100755\n>> --- a/git-submodule.sh\n>> +++ b/git-submodule.sh\n>> @@ -199,7 +199,7 @@ sanitize_submodule_env()\n>> {\n>> \tsanitized_config=$(git submodule--helper sanitize-config)\n>> \tclear_local_git_env\n>> -\tGIT_CONFIG_PARAMETERS=$sanitized_config\n>> +\texport GIT_CONFIG_PARAMETERS=$sanitized_config\n>> }\n> \n> If you already have $GIT_CONFIG_PARAMETERS exported when we enter the\n> function, then we should not need to re-export it when changing the\n> value in the final line (the export bit is retained by the shell). But\n> if you don't have it set already, then $sanitized_config must by\n> definition be empty.\n> \n> So it should do the right thing without the export.\n> \n> At the same time, clear_local_git_env() will call \"unset\" on\n> GIT_CONFIG_PARAMETERS. Which would clear the export bit, meaning the\n> final line doesn't ever have any impact on sub-programs, and the whole\n> thing is totally broken. But then, why does the test in t5550 pass?\n> \n> Confused...\n\nI am no expert in the Submodule code but I think the cloning of\nthe submodules is not yet guarded with sanitize_submodule_env [3].\nThat means the submodule is cloned with the GIT_CONFIG_PARAMETERS\nof the super project. That might explain why t5550 passes as the \ncredential config is only used in that area.\n\nThe submodule checkout is guarded with sanitize_submodule_env\nand therefore my Git-LFS filter use case is affect.\n\nDoes this sound reasonable?\n\nThanks,\nLars\n\n[3] https://github.com/git/git/blob/3ad15fd5e17bbb73fb1161ff4e9c3ed254d5b243/git-submodule.sh#L704-L711\n[4] https://github.com/git/git/blob/3ad15fd5e17bbb73fb1161ff4e9c3ed254d5b243/git-submodule.sh#L811\n"},{"id":"284793","messageId":"20160428121753.GA5023@sigill.intra.peff.net","threadId":"42135","inReplyTo":"20160428120504.GA22399@sigill.intra.peff.net","subject":"Re: [RFC] How to pass Git config command line instructions to Submodule commands?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-28T12:17:53Z","receivedAt":"2016-04-28T12:17:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 28, 2016 at 08:05:04AM -0400, Jeff King wrote:\n\n> So AFAICT 14111fc49 is totally broken. It doesn't actually work for\n> git-submodule (because of the missing export), nor for git-fetch\n> (because that skips the shell script), and the one case we are testing\n> already worked without it (but probably _should_ be sanitizing the\n> config, so is buggy, too).\n\nThis last bit is not quite accurate. The test in t5550 doesn't pass\nwithout 14111fc49. But it _does_ pass if we make\nsanitize_submodule_env() in the shell script a noop. That's because it\nis going through clone_submodule() in the C code, which uses the C-only\nprepare_submodule_repo_env().\n\nSo that case _is_ correct right now. It's just that t5550 isn't testing\nthe shell script part, which is broken. Probably running \"git submodule\nupdate\" in the resulting clone would cover that.\n\nAnd for the fetch case, we probably just need to be calling\nprepare_submodule_repo_env() there, too.\n\n-Peff\n"},{"id":"284797","messageId":"20160428133534.GA19056@sigill.intra.peff.net","threadId":"42135","inReplyTo":"20160428121753.GA5023@sigill.intra.peff.net","subject":"[PATCH 0/5] fixes for sanitized submodule config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-28T13:35:34Z","receivedAt":"2016-04-28T13:35:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 28, 2016 at 08:17:53AM -0400, Jeff King wrote:\n\n> So that case _is_ correct right now. It's just that t5550 isn't testing\n> the shell script part, which is broken. Probably running \"git submodule\n> update\" in the resulting clone would cover that.\n> \n> And for the fetch case, we probably just need to be calling\n> prepare_submodule_repo_env() there, too.\n\nSo here's a series which fixes sanitizing in the \"git-submodule\" shell\nscript, along with \"git fetch\". And cleans up a few things along the\nway.\n\n [1/5]: t5550: fix typo in $HTTPD_URL\n [2/5]: t5550: break submodule config test into multiple sub-tests\n [3/5]: submodule: export sanitize GIT_CONFIG_PARAMETERS\n [4/5]: submodule--helper: move config-sanitizing to submodule.c\n [5/5]: submodule: use prepare_submodule_repo_env consistently\n\n-Peff\n"},{"id":"284798","messageId":"20160428133636.GA25319@sigill.intra.peff.net","threadId":"42135","inReplyTo":"20160428133534.GA19056@sigill.intra.peff.net","subject":"[PATCH 1/5] t5550: fix typo in $HTTPD_URL","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-28T13:36:37Z","receivedAt":"2016-04-28T13:36:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Commit 14111fc (git: submodule honor -c credential.* from\ncommand line, 2016-02-29) accidentally wrote $HTTP_URL. It\nhappened to work because we ended up with \"credential..helper\",\nwhich we treat the same as \"credential.helper\", applying it\nto all URLs.\n\nSigned-off-by: Jeff King \n---\n t/t5550-http-fetch-dumb.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\nindex 48e2ab6..81cc57f 100755\n--- a/t/t5550-http-fetch-dumb.sh\n+++ b/t/t5550-http-fetch-dumb.sh\n@@ -103,7 +103,7 @@ test_expect_success 'cmdline credential config passes into submodules' '\n \ttest_must_fail git clone --recursive super super-clone &&\n \trm -rf super-clone &&\n \tset_askpass wrong pass@host &&\n-\tgit -c \"credential.$HTTP_URL.username=user@host\" \\\n+\tgit -c \"credential.$HTTPD_URL.username=user@host\" \\\n \t\tclone --recursive super super-clone &&\n \texpect_askpass pass user@host\n '\n-- \n2.8.1.617.gbdccc2d\n"},{"id":"284799","messageId":"20160428133704.GB25319@sigill.intra.peff.net","threadId":"42135","inReplyTo":"20160428133534.GA19056@sigill.intra.peff.net","subject":"[PATCH 2/5] t5550: break submodule config test into multiple sub-tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-28T13:37:04Z","receivedAt":"2016-04-28T13:37:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Right now we test only the cloning case, but there are other\ninteresting cases (e.g., fetching). Let's pull the setup\nbits into their own test, which will make things flow more\nlogically once we start adding more tests which use the\nsetup.\n\nLet's also introduce some whitespace to the clone-test to\nsplit the two parts: making sure it fails without our\ncmdline config, and that it succeeds with it.\n\nSigned-off-by: Jeff King \n---\n t/t5550-http-fetch-dumb.sh | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\nindex 81cc57f..e8e91bb 100755\n--- a/t/t5550-http-fetch-dumb.sh\n+++ b/t/t5550-http-fetch-dumb.sh\n@@ -91,17 +91,21 @@ test_expect_success 'configured username does not override URL' '\n \texpect_askpass pass user@host\n '\n \n-test_expect_success 'cmdline credential config passes into submodules' '\n+test_expect_success 'set up repo with http submodules' '\n \tgit init super &&\n \tset_askpass user@host pass@host &&\n \t(\n \t\tcd super &&\n \t\tgit submodule add \"$HTTPD_URL/auth/dumb/repo.git\" sub &&\n \t\tgit commit -m \"add submodule\"\n-\t) &&\n+\t)\n+'\n+\n+test_expect_success 'cmdline credential config passes to submodule via clone' '\n \tset_askpass wrong pass@host &&\n \ttest_must_fail git clone --recursive super super-clone &&\n \trm -rf super-clone &&\n+\n \tset_askpass wrong pass@host &&\n \tgit -c \"credential.$HTTPD_URL.username=user@host\" \\\n \t\tclone --recursive super super-clone &&\n-- \n2.8.1.617.gbdccc2d\n"},{"id":"284800","messageId":"20160428133744.GC25319@sigill.intra.peff.net","threadId":"42135","inReplyTo":"20160428133534.GA19056@sigill.intra.peff.net","subject":"[PATCH 3/5] submodule: export sanitized GIT_CONFIG_PARAMETERS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-28T13:37:44Z","receivedAt":"2016-04-28T13:37:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Commit 14111fc (git: submodule honor -c credential.* from\ncommand line, 2016-02-29) taught git-submodule.sh to save\nthe sanitized value of $GIT_CONFIG_PARAMETERS when clearing\nthe environment for a submodule. However, it failed to\nexport the result, meaning that it had no effect for any\nsub-programs.\n\nWe didn't catch this in our initial tests because we checked\nonly the \"clone\" case, which does not go through the shell\nscript at all. Provoking \"git submodule update\" to do a\nfetch demonstrates the bug.\n\nNoticed-by: Lars Schneider \nSigned-off-by: Jeff King \n---\n git-submodule.sh | 1 +\n t/t5550-http-fetch-dumb.sh | 17 +++++++++++++++++\n 2 files changed, 18 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 2a84d7e..3a40d4b 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -200,6 +200,7 @@ sanitize_submodule_env()\n \tsanitized_config=$(git submodule--helper sanitize-config)\n \tclear_local_git_env\n \tGIT_CONFIG_PARAMETERS=$sanitized_config\n+\texport GIT_CONFIG_PARAMETERS\n }\n \n #\ndiff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\nindex e8e91bb..13ac788 100755\n--- a/t/t5550-http-fetch-dumb.sh\n+++ b/t/t5550-http-fetch-dumb.sh\n@@ -112,6 +112,23 @@ test_expect_success 'cmdline credential config passes to submodule via clone' '\n \texpect_askpass pass user@host\n '\n \n+test_expect_success 'cmdline credential config passes submodule update' '\n+\t# advance the submodule HEAD so that a fetch is required\n+\tgit commit --allow-empty -m foo &&\n+\tgit push \"$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/repo.git\" HEAD &&\n+\tsha1=$(git rev-parse HEAD) &&\n+\tgit -C super-clone update-index --cacheinfo 160000,$sha1,sub &&\n+\n+\tset_askpass wrong pass@host &&\n+\ttest_must_fail git -C super-clone submodule update &&\n+\n+\tset_askpass wrong pass@host &&\n+\tgit -C super-clone \\\n+\t -c \"credential.$HTTPD_URL.username=user@host\" \\\n+\t submodule update &&\n+\texpect_askpass pass user@host\n+'\n+\n test_expect_success 'fetch changes via http' '\n \techo content >>file &&\n \tgit commit -a -m two &&\n-- \n2.8.1.617.gbdccc2d\n"},{"id":"284801","messageId":"20160428133819.GD25319@sigill.intra.peff.net","threadId":"42135","inReplyTo":"20160428133534.GA19056@sigill.intra.peff.net","subject":"[PATCH 4/5] submodule--helper: move config-sanitizing to submodule.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-28T13:38:20Z","receivedAt":"2016-04-28T13:38:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"These functions should be used by any code which spawns a\nsubmodule process, which may happen in submodule.c (e.g.,\nfor spawning fetch). Let's move them there and make them\npublic so that submodule--helper can continue to use them.\n\nSine they're now public, let's also provide a basic overview\nof their intended use.\n\nSigned-off-by: Jeff King \n---\n builtin/submodule--helper.c | 48 --------------------------------------------\n submodule.c | 49 +++++++++++++++++++++++++++++++++++++++++++++\n submodule.h | 16 +++++++++++++++\n 3 files changed, 65 insertions(+), 48 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 3bd6883..de3ad5b 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -119,54 +119,6 @@ static int module_name(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n-/*\n- * Rules to sanitize configuration variables that are Ok to be passed into\n- * submodule operations from the parent project using \"-c\". Should only\n- * include keys which are both (a) safe and (b) necessary for proper\n- * operation.\n- */\n-static int submodule_config_ok(const char *var)\n-{\n-\tif (starts_with(var, \"credential.\"))\n-\t\treturn 1;\n-\treturn 0;\n-}\n-\n-static int sanitize_submodule_config(const char *var, const char *value, void *data)\n-{\n-\tstruct strbuf *out = data;\n-\n-\tif (submodule_config_ok(var)) {\n-\t\tif (out->len)\n-\t\t\tstrbuf_addch(out, ' ');\n-\n-\t\tif (value)\n-\t\t\tsq_quotef(out, \"%s=%s\", var, value);\n-\t\telse\n-\t\t\tsq_quote_buf(out, var);\n-\t}\n-\n-\treturn 0;\n-}\n-\n-static void prepare_submodule_repo_env(struct argv_array *out)\n-{\n-\tconst char * const *var;\n-\n-\tfor (var = local_repo_env; *var; var++) {\n-\t\tif (!strcmp(*var, CONFIG_DATA_ENVIRONMENT)) {\n-\t\t\tstruct strbuf sanitized_config = STRBUF_INIT;\n-\t\t\tgit_config_from_parameters(sanitize_submodule_config,\n-\t\t\t\t\t\t &sanitized_config);\n-\t\t\targv_array_pushf(out, \"%s=%s\", *var, sanitized_config.buf);\n-\t\t\tstrbuf_release(&sanitized_config);\n-\t\t} else {\n-\t\t\targv_array_push(out, *var);\n-\t\t}\n-\t}\n-\n-}\n-\n static int clone_submodule(const char *path, const char *gitdir, const char *url,\n \t\t\t const char *depth, const char *reference, int quiet)\n {\ndiff --git a/submodule.c b/submodule.c\nindex 90825e1..02eaf0e 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -13,6 +13,7 @@\n #include \"argv-array.h\"\n #include \"blob.h\"\n #include \"thread-utils.h\"\n+#include \"quote.h\"\n \n static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n static int parallel_jobs = 1;\n@@ -1129,3 +1130,51 @@ int parallel_submodules(void)\n {\n \treturn parallel_jobs;\n }\n+\n+/*\n+ * Rules to sanitize configuration variables that are Ok to be passed into\n+ * submodule operations from the parent project using \"-c\". Should only\n+ * include keys which are both (a) safe and (b) necessary for proper\n+ * operation.\n+ */\n+static int submodule_config_ok(const char *var)\n+{\n+\tif (starts_with(var, \"credential.\"))\n+\t\treturn 1;\n+\treturn 0;\n+}\n+\n+int sanitize_submodule_config(const char *var, const char *value, void *data)\n+{\n+\tstruct strbuf *out = data;\n+\n+\tif (submodule_config_ok(var)) {\n+\t\tif (out->len)\n+\t\t\tstrbuf_addch(out, ' ');\n+\n+\t\tif (value)\n+\t\t\tsq_quotef(out, \"%s=%s\", var, value);\n+\t\telse\n+\t\t\tsq_quote_buf(out, var);\n+\t}\n+\n+\treturn 0;\n+}\n+\n+void prepare_submodule_repo_env(struct argv_array *out)\n+{\n+\tconst char * const *var;\n+\n+\tfor (var = local_repo_env; *var; var++) {\n+\t\tif (!strcmp(*var, CONFIG_DATA_ENVIRONMENT)) {\n+\t\t\tstruct strbuf sanitized_config = STRBUF_INIT;\n+\t\t\tgit_config_from_parameters(sanitize_submodule_config,\n+\t\t\t\t\t\t &sanitized_config);\n+\t\t\targv_array_pushf(out, \"%s=%s\", *var, sanitized_config.buf);\n+\t\t\tstrbuf_release(&sanitized_config);\n+\t\t} else {\n+\t\t\targv_array_push(out, *var);\n+\t\t}\n+\t}\n+\n+}\ndiff --git a/submodule.h b/submodule.h\nindex 7ef3775..7577b3b 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -61,4 +61,20 @@ int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam\n void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n int parallel_submodules(void);\n \n+/*\n+ * This function is intended as a callback for use with\n+ * git_config_from_parameters(). It ignores any config options which\n+ * are not suitable for passing along to a submodule, and accumulates the rest\n+ * in \"data\", which must be a pointer to a strbuf. The end result can\n+ * be put into $GIT_CONFIG_PARAMETERS for passing to a sub-process.\n+ */\n+int sanitize_submodule_config(const char *var, const char *value, void *data);\n+\n+/*\n+ * Prepare the \"env_array\" parameter of a \"struct child_process\" for executing\n+ * a submodule by clearing any repo-specific envirionment variables, but\n+ * retaining any config approved by sanitize_submodule_config().\n+ */\n+void prepare_submodule_repo_env(struct argv_array *out);\n+\n #endif\n-- \n2.8.1.617.gbdccc2d\n"},{"id":"284802","messageId":"20160428133914.GE25319@sigill.intra.peff.net","threadId":"42135","inReplyTo":"20160428133534.GA19056@sigill.intra.peff.net","subject":"[PATCH 5/5] submodule: use prepare_submodule_repo_env consistently","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-28T13:39:15Z","receivedAt":"2016-04-28T13:39:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Before 14111fc (git: submodule honor -c credential.* from\ncommand line, 2016-02-29), it was sufficient for code which\nspawned a process in a submodule to just set the child\nprocess's \"env\" field to \"local_repo_env\" to clear the\nenvironment of any repo-specific variables.\n\nThat commit introduced a more complicated procedure, in\nwhich we clear most variables but allow through sanitized\nconfig. For C code, we used that procedure only for cloning,\nbut not for any of the programs spawned by submodule.c. As a\nresult, things like \"git fetch --recurse-submodules\" behave\ndifferently than \"git clone --recursive\"; the former will\nnot pass through the sanitized config.\n\nWe can fix this by using prepare_submodule_repo_env()\neverywhere in submodule.c.\n\nSigned-off-by: Jeff King \n---\n submodule.c | 14 +++++++-------\n t/t5550-http-fetch-dumb.sh | 11 +++++++++++\n 2 files changed, 18 insertions(+), 7 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 02eaf0e..4e76b98 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -394,7 +394,7 @@ static int submodule_needs_pushing(const char *path, const unsigned char sha1[20\n \n \t\targv[1] = sha1_to_hex(sha1);\n \t\tcp.argv = argv;\n-\t\tcp.env = local_repo_env;\n+\t\tprepare_submodule_repo_env(&cp.env_array);\n \t\tcp.git_cmd = 1;\n \t\tcp.no_stdin = 1;\n \t\tcp.out = -1;\n@@ -481,7 +481,7 @@ static int push_submodule(const char *path)\n \t\tconst char *argv[] = {\"push\", NULL};\n \n \t\tcp.argv = argv;\n-\t\tcp.env = local_repo_env;\n+\t\tprepare_submodule_repo_env(&cp.env_array);\n \t\tcp.git_cmd = 1;\n \t\tcp.no_stdin = 1;\n \t\tcp.dir = path;\n@@ -527,7 +527,7 @@ static int is_submodule_commit_present(const char *path, unsigned char sha1[20])\n \n \t\targv[3] = sha1_to_hex(sha1);\n \t\tcp.argv = argv;\n-\t\tcp.env = local_repo_env;\n+\t\tprepare_submodule_repo_env(&cp.env_array);\n \t\tcp.git_cmd = 1;\n \t\tcp.no_stdin = 1;\n \t\tcp.dir = path;\n@@ -710,7 +710,7 @@ static int get_next_submodule(struct child_process *cp,\n \t\tif (is_directory(git_dir)) {\n \t\t\tchild_process_init(cp);\n \t\t\tcp->dir = strbuf_detach(&submodule_path, NULL);\n-\t\t\tcp->env = local_repo_env;\n+\t\t\tprepare_submodule_repo_env(&cp->env_array);\n \t\t\tcp->git_cmd = 1;\n \t\t\tif (!spf->quiet)\n \t\t\t\tstrbuf_addf(err, \"Fetching submodule %s%s\\n\",\n@@ -825,7 +825,7 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)\n \t\targv[2] = \"-uno\";\n \n \tcp.argv = argv;\n-\tcp.env = local_repo_env;\n+\tprepare_submodule_repo_env(&cp.env_array);\n \tcp.git_cmd = 1;\n \tcp.no_stdin = 1;\n \tcp.out = -1;\n@@ -886,7 +886,7 @@ int submodule_uses_gitfile(const char *path)\n \n \t/* Now test that all nested submodules use a gitfile too */\n \tcp.argv = argv;\n-\tcp.env = local_repo_env;\n+\tprepare_submodule_repo_env(&cp.env_array);\n \tcp.git_cmd = 1;\n \tcp.no_stdin = 1;\n \tcp.no_stderr = 1;\n@@ -919,7 +919,7 @@ int ok_to_remove_submodule(const char *path)\n \t\treturn 0;\n \n \tcp.argv = argv;\n-\tcp.env = local_repo_env;\n+\tprepare_submodule_repo_env(&cp.env_array);\n \tcp.git_cmd = 1;\n \tcp.no_stdin = 1;\n \tcp.out = -1;\ndiff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\nindex 13ac788..3484b6f 100755\n--- a/t/t5550-http-fetch-dumb.sh\n+++ b/t/t5550-http-fetch-dumb.sh\n@@ -112,6 +112,17 @@ test_expect_success 'cmdline credential config passes to submodule via clone' '\n \texpect_askpass pass user@host\n '\n \n+test_expect_success 'cmdline credential config passes submodule via fetch' '\n+\tset_askpass wrong pass@host &&\n+\ttest_must_fail git -C super-clone fetch --recurse-submodules &&\n+\n+\tset_askpass wrong pass@host &&\n+\tgit -C super-clone \\\n+\t -c \"credential.$HTTPD_URL.username=user@host\" \\\n+\t fetch --recurse-submodules &&\n+\texpect_askpass pass user@host\n+'\n+\n test_expect_success 'cmdline credential config passes submodule update' '\n \t# advance the submodule HEAD so that a fetch is required\n \tgit commit --allow-empty -m foo &&\n-- \n2.8.1.617.gbdccc2d\n"},{"id":"284803","messageId":"20160428134047.GA25364@sigill.intra.peff.net","threadId":"42135","inReplyTo":"FF931AB2-2381-4DF9-AE28-CE30EEC5C352@gmail.com","subject":"Re: [RFC] How to pass Git config command line instructions to Submodule commands?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-28T13:40:47Z","receivedAt":"2016-04-28T13:40:47Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 28, 2016 at 02:05:20PM +0200, Lars Schneider wrote:\n\n> I am no expert in the Submodule code but I think the cloning of\n> the submodules is not yet guarded with sanitize_submodule_env [3].\n> That means the submodule is cloned with the GIT_CONFIG_PARAMETERS\n> of the super project. That might explain why t5550 passes as the \n> credential config is only used in that area.\n> \n> The submodule checkout is guarded with sanitize_submodule_env\n> and therefore my Git-LFS filter use case is affect.\n> \n> Does this sound reasonable?\n\nYes, that's exactly what's going on. git-submodule.sh's code _is_\nbroken, which is what you're seeing. I've just posted a series to fix\nthis. I think it would be reasonable to add filter.* to the whitelist on\ntop of that series.\n\n-Peff\n"},{"id":"284807","messageId":"alpine.DEB.2.20.1604281600020.9313@virtualbox","threadId":"42135","inReplyTo":"20160428133534.GA19056@sigill.intra.peff.net","subject":"Re: [PATCH 0/5] fixes for sanitized submodule config","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-04-28T14:02:07Z","receivedAt":"2016-04-28T14:02:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Thu, 28 Apr 2016, Jeff King wrote:\n\n> On Thu, Apr 28, 2016 at 08:17:53AM -0400, Jeff King wrote:\n> \n> > So that case _is_ correct right now. It's just that t5550 isn't testing\n> > the shell script part, which is broken. Probably running \"git submodule\n> > update\" in the resulting clone would cover that.\n> > \n> > And for the fetch case, we probably just need to be calling\n> > prepare_submodule_repo_env() there, too.\n> \n> So here's a series which fixes sanitizing in the \"git-submodule\" shell\n> script, along with \"git fetch\". And cleans up a few things along the\n> way.\n\nNice!\n\nI reviewed those changes and they all look sensible to me (did not apply\nthem locally for lack of time, though).\n\nCiao,\nDscho\n"},{"id":"284815","messageId":"CAGZ79kacgJ_=C-Y=QkSDmvEvCrmEbcvdMkg0x76zkj-Y86m4=g@mail.gmail.com","threadId":"42135","inReplyTo":"20160428133704.GB25319@sigill.intra.peff.net","subject":"Re: [PATCH 2/5] t5550: break submodule config test into multiple sub-tests","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-28T15:21:21Z","receivedAt":"2016-04-28T15:21:21Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Apr 28, 2016 at 6:37 AM, Jeff King wrote:\n> Right now we test only the cloning case, but there are other\n> interesting cases (e.g., fetching). Let's pull the setup\n> bits into their own test, which will make things flow more\n> logically once we start adding more tests which use the\n> setup.\n>\n> Let's also introduce some whitespace to the clone-test to\n> split the two parts: making sure it fails without our\n> cmdline config, and that it succeeds with it.\n>\n> Signed-off-by: Jeff King \n> ---\n> t/t5550-http-fetch-dumb.sh | 8 ++++++--\n> 1 file changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\n> index 81cc57f..e8e91bb 100755\n> --- a/t/t5550-http-fetch-dumb.sh\n> +++ b/t/t5550-http-fetch-dumb.sh\n> @@ -91,17 +91,21 @@ test_expect_success 'configured username does not override URL' '\n> expect_askpass pass user@host\n> '\n>\n> -test_expect_success 'cmdline credential config passes into submodules' '\n> +test_expect_success 'set up repo with http submodules' '\n\nset up or setup?\n\n$ grep -r \"set up\" |wc -l\n69\n$ grep -r \"setup\" |wc -l\n1162\n\nApart from that nit, this patch looks good to me.\n\n> git init super &&\n> set_askpass user@host pass@host &&\n> (\n> cd super &&\n> git submodule add \"$HTTPD_URL/auth/dumb/repo.git\" sub &&\n> git commit -m \"add submodule\"\n> - ) &&\n> + )\n> +'\n> +\n> +test_expect_success 'cmdline credential config passes to submodule via clone' '\n> set_askpass wrong pass@host &&\n> test_must_fail git clone --recursive super super-clone &&\n> rm -rf super-clone &&\n> +\n> set_askpass wrong pass@host &&\n> git -c \"credential.$HTTPD_URL.username=user@host\" \\\n> clone --recursive super super-clone &&\n> --\n> 2.8.1.617.gbdccc2d\n>\n"},{"id":"284816","messageId":"CA+P7+xqSYA5++pq0_0FYFg8TXhc7f0=X_4uNWtFRpebEDzqtHQ@mail.gmail.com","threadId":"42135","inReplyTo":"20160428133636.GA25319@sigill.intra.peff.net","subject":"Re: [PATCH 1/5] t5550: fix typo in $HTTPD_URL","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-04-28T15:24:05Z","receivedAt":"2016-04-28T15:24:05Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Apr 28, 2016 at 6:36 AM, Jeff King wrote:\n> Commit 14111fc (git: submodule honor -c credential.* from\n> command line, 2016-02-29) accidentally wrote $HTTP_URL. It\n> happened to work because we ended up with \"credential..helper\",\n> which we treat the same as \"credential.helper\", applying it\n> to all URLs.\n\nYou say \"credential.helper\" twice here? I think that's confusing. The\npatch looks perfectly fine but I am having trouble parsing this\ndescription. 'We end up with X which we treat the same as X\"?\n\n>\n> Signed-off-by: Jeff King \n> ---\n> t/t5550-http-fetch-dumb.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\n> index 48e2ab6..81cc57f 100755\n> --- a/t/t5550-http-fetch-dumb.sh\n> +++ b/t/t5550-http-fetch-dumb.sh\n> @@ -103,7 +103,7 @@ test_expect_success 'cmdline credential config passes into submodules' '\n> test_must_fail git clone --recursive super super-clone &&\n> rm -rf super-clone &&\n> set_askpass wrong pass@host &&\n> - git -c \"credential.$HTTP_URL.username=user@host\" \\\n> + git -c \"credential.$HTTPD_URL.username=user@host\" \\\n> clone --recursive super super-clone &&\n> expect_askpass pass user@host\n> '\n> --\n> 2.8.1.617.gbdccc2d\n>\n"},{"id":"284818","messageId":"20160428152503.GA31063@sigill.intra.peff.net","threadId":"42135","inReplyTo":"CAGZ79kacgJ_=C-Y=QkSDmvEvCrmEbcvdMkg0x76zkj-Y86m4=g@mail.gmail.com","subject":"Re: [PATCH 2/5] t5550: break submodule config test into multiple sub-tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-28T15:25:03Z","receivedAt":"2016-04-28T15:25:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 28, 2016 at 08:21:21AM -0700, Stefan Beller wrote:\n\n> > -test_expect_success 'cmdline credential config passes into submodules' '\n> > +test_expect_success 'set up repo with http submodules' '\n> \n> set up or setup?\n> \n> $ grep -r \"set up\" |wc -l\n> 69\n> $ grep -r \"setup\" |wc -l\n> 1162\n> \n> Apart from that nit, this patch looks good to me.\n\n\"set up\" is a verb phrase. \"setup\" is a noun or an adjective.\n\nI think we quite often use the latter as a verb. Whether that is\ngrammatically depends on your philosophy, I think.\n\n-Peff\n"},{"id":"284819","messageId":"CAGZ79karNW3+xiZQuoh5v-nRabs+h-5pyHDfjHS4vTVBkAyv_Q@mail.gmail.com","threadId":"42135","inReplyTo":"20160428133744.GC25319@sigill.intra.peff.net","subject":"Re: [PATCH 3/5] submodule: export sanitized GIT_CONFIG_PARAMETERS","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-28T15:25:29Z","receivedAt":"2016-04-28T15:25:29Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Apr 28, 2016 at 6:37 AM, Jeff King wrote:\n> Commit 14111fc (git: submodule honor -c credential.* from\n> command line, 2016-02-29) taught git-submodule.sh to save\n> the sanitized value of $GIT_CONFIG_PARAMETERS when clearing\n> the environment for a submodule. However, it failed to\n> export the result, meaning that it had no effect for any\n> sub-programs.\n>\n> We didn't catch this in our initial tests because we checked\n> only the \"clone\" case, which does not go through the shell\n> script at all. Provoking \"git submodule update\" to do a\n> fetch demonstrates the bug.\n>\n> Noticed-by: Lars Schneider \n> Signed-off-by: Jeff King \n> ---\n> git-submodule.sh | 1 +\n> t/t5550-http-fetch-dumb.sh | 17 +++++++++++++++++\n> 2 files changed, 18 insertions(+)\n>\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 2a84d7e..3a40d4b 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -200,6 +200,7 @@ sanitize_submodule_env()\n> sanitized_config=$(git submodule--helper sanitize-config)\n> clear_local_git_env\n> GIT_CONFIG_PARAMETERS=$sanitized_config\n> + export GIT_CONFIG_PARAMETERS\n> }\n>\n> #\n> diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\n> index e8e91bb..13ac788 100755\n> --- a/t/t5550-http-fetch-dumb.sh\n> +++ b/t/t5550-http-fetch-dumb.sh\n> @@ -112,6 +112,23 @@ test_expect_success 'cmdline credential config passes to submodule via clone' '\n> expect_askpass pass user@host\n> '\n>\n> +test_expect_success 'cmdline credential config passes submodule update' '\n> + # advance the submodule HEAD so that a fetch is required\n> + git commit --allow-empty -m foo &&\n> + git push \"$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/repo.git\" HEAD &&\n> + sha1=$(git rev-parse HEAD) &&\n> + git -C super-clone update-index --cacheinfo 160000,$sha1,sub &&\n\nThe use of update-index seems elegant to me, though different than\nany submodule test I wrote so far. :)\n\n> +\n> + set_askpass wrong pass@host &&\n> + test_must_fail git -C super-clone submodule update &&\n> +\n> + set_askpass wrong pass@host &&\n> + git -C super-clone \\\n> + -c \"credential.$HTTPD_URL.username=user@host\" \\\n> + submodule update &&\n> + expect_askpass pass user@host\n> +'\n> +\n> test_expect_success 'fetch changes via http' '\n> echo content >>file &&\n> git commit -a -m two &&\n> --\n> 2.8.1.617.gbdccc2d\n>\n"},{"id":"284821","messageId":"CA+P7+xqpntuL==uW75r29TxJsdjJ5WDdfBEorqd5-zHKZt5mQg@mail.gmail.com","threadId":"42135","inReplyTo":"CAGZ79kacgJ_=C-Y=QkSDmvEvCrmEbcvdMkg0x76zkj-Y86m4=g@mail.gmail.com","subject":"Re: [PATCH 2/5] t5550: break submodule config test into multiple sub-tests","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-04-28T15:25:41Z","receivedAt":"2016-04-28T15:25:41Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Apr 28, 2016 at 8:21 AM, Stefan Beller wrote:\n> On Thu, Apr 28, 2016 at 6:37 AM, Jeff King wrote:\n>> Right now we test only the cloning case, but there are other\n>> interesting cases (e.g., fetching). Let's pull the setup\n>> bits into their own test, which will make things flow more\n>> logically once we start adding more tests which use the\n>> setup.\n>>\n>> Let's also introduce some whitespace to the clone-test to\n>> split the two parts: making sure it fails without our\n>> cmdline config, and that it succeeds with it.\n>>\n>> Signed-off-by: Jeff King \n>> ---\n>> t/t5550-http-fetch-dumb.sh | 8 ++++++--\n>> 1 file changed, 6 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh\n>> index 81cc57f..e8e91bb 100755\n>> --- a/t/t5550-http-fetch-dumb.sh\n>> +++ b/t/t5550-http-fetch-dumb.sh\n>> @@ -91,17 +91,21 @@ test_expect_success 'configured username does not override URL' '\n>> expect_askpass pass user@host\n>> '\n>>\n>> -test_expect_success 'cmdline credential config passes into submodules' '\n>> +test_expect_success 'set up repo with http submodules' '\n>\n> set up or setup?\n>\n> $ grep -r \"set up\" |wc -l\n> 69\n> $ grep -r \"setup\" |wc -l\n> 1162\n>\n> Apart from that nit, this patch looks good to me.\n>\n\nYes this looks quite a bit more readable.\n\nThanks,\nJake\n"},{"id":"284820","messageId":"20160428152549.GB31063@sigill.intra.peff.net","threadId":"42135","inReplyTo":"CA+P7+xqSYA5++pq0_0FYFg8TXhc7f0=X_4uNWtFRpebEDzqtHQ@mail.gmail.com","subject":"Re: [PATCH 1/5] t5550: fix typo in $HTTPD_URL","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-28T15:25:50Z","receivedAt":"2016-04-28T15:25:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 28, 2016 at 08:24:05AM -0700, Jacob Keller wrote:\n\n> On Thu, Apr 28, 2016 at 6:36 AM, Jeff King wrote:\n> > Commit 14111fc (git: submodule honor -c credential.* from\n> > command line, 2016-02-29) accidentally wrote $HTTP_URL. It\n> > happened to work because we ended up with \"credential..helper\",\n> > which we treat the same as \"credential.helper\", applying it\n> > to all URLs.\n> \n> You say \"credential.helper\" twice here? I think that's confusing. The\n> patch looks perfectly fine but I am having trouble parsing this\n> description. 'We end up with X which we treat the same as X\"?\n\nNote the two dots in the first one. There is no variable $HTTP_URL, so:\n\n credential.$HTTP_URL.helper\n\nbecomes:\n\n credential..helper\n\n-Peff\n"},{"id":"284822","messageId":"CA+P7+xpR7wWso2yiaschPYbf7eDDvZ+RuS2aWdNPS2FeeGV1qg@mail.gmail.com","threadId":"42135","inReplyTo":"20160428152549.GB31063@sigill.intra.peff.net","subject":"Re: [PATCH 1/5] t5550: fix typo in $HTTPD_URL","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-04-28T15:26:46Z","receivedAt":"2016-04-28T15:26:46Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Apr 28, 2016 at 8:25 AM, Jeff King wrote:\n> On Thu, Apr 28, 2016 at 08:24:05AM -0700, Jacob Keller wrote:\n>\n>> On Thu, Apr 28, 2016 at 6:36 AM, Jeff King wrote:\n>> > Commit 14111fc (git: submodule honor -c credential.* from\n>> > command line, 2016-02-29) accidentally wrote $HTTP_URL. It\n>> > happened to work because we ended up with \"credential..helper\",\n>> > which we treat the same as \"credential.helper\", applying it\n>> > to all URLs.\n>>\n>> You say \"credential.helper\" twice here? I think that's confusing. The\n>> patch looks perfectly fine but I am having trouble parsing this\n>> description. 'We end up with X which we treat the same as X\"?\n>\n> Note the two dots in the first one. There is no variable $HTTP_URL, so:\n>\n> credential.$HTTP_URL.helper\n>\n> becomes:\n>\n> credential..helper\n>\n> -Peff\n\nAh yes, very tiny text and tired morning eyes you are right. Makes\nmore sense now.\n\nThanks,\nJake\n"},{"id":"284823","messageId":"20160428152811.GC31063@sigill.intra.peff.net","threadId":"42135","inReplyTo":"CAGZ79karNW3+xiZQuoh5v-nRabs+h-5pyHDfjHS4vTVBkAyv_Q@mail.gmail.com","subject":"Re: [PATCH 3/5] submodule: export sanitized GIT_CONFIG_PARAMETERS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-28T15:28:11Z","receivedAt":"2016-04-28T15:28:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 28, 2016 at 08:25:29AM -0700, Stefan Beller wrote:\n\n> > +test_expect_success 'cmdline credential config passes submodule update' '\n> > + # advance the submodule HEAD so that a fetch is required\n> > + git commit --allow-empty -m foo &&\n> > + git push \"$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/repo.git\" HEAD &&\n> > + sha1=$(git rev-parse HEAD) &&\n> > + git -C super-clone update-index --cacheinfo 160000,$sha1,sub &&\n> \n> The use of update-index seems elegant to me, though different than\n> any submodule test I wrote so far. :)\n\nYeah, I actually wrestled with finding the shortest recipe to convince\ngit-submodule to actually call git-fetch. Suggestions welcome if there's\nsomething more canonical.\n\nBut I think we have to advance the submodule pointer in some way to\nconvince it to want to fetch (I also tried deleting the refs in the\ncloned module, but that seemed hacky).\n\nI guess the way it would happen in real life is that the \"origin\" remote\n(\"super\" here, not \"super-clone\") would make the change and commit the\nsubmodule, and then \"super-clone\" would pull it.\n\nThat seemed even more convoluted to me.\n\n-Peff\n"},{"id":"284824","messageId":"CA+P7+xoJrU884n7xG_kof46TSg_4NwS=5+nZza8PM7D=YtsXng@mail.gmail.com","threadId":"42135","inReplyTo":"20160428133744.GC25319@sigill.intra.peff.net","subject":"Re: [PATCH 3/5] submodule: export sanitized GIT_CONFIG_PARAMETERS","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-04-28T15:28:29Z","receivedAt":"2016-04-28T15:28:29Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Apr 28, 2016 at 6:37 AM, Jeff King wrote:\n> Commit 14111fc (git: submodule honor -c credential.* from\n> command line, 2016-02-29) taught git-submodule.sh to save\n> the sanitized value of $GIT_CONFIG_PARAMETERS when clearing\n> the environment for a submodule. However, it failed to\n> export the result, meaning that it had no effect for any\n> sub-programs.\n>\n> We didn't catch this in our initial tests because we checked\n> only the \"clone\" case, which does not go through the shell\n> script at all. Provoking \"git submodule update\" to do a\n> fetch demonstrates the bug.\n>\n> Noticed-by: Lars Schneider \n> Signed-off-by: Jeff King \n> ---\n> git-submodule.sh | 1 +\n> t/t5550-http-fetch-dumb.sh | 17 +++++++++++++++++\n> 2 files changed, 18 insertions(+)\n>\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 2a84d7e..3a40d4b 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -200,6 +200,7 @@ sanitize_submodule_env()\n> sanitized_config=$(git submodule--helper sanitize-config)\n> clear_local_git_env\n> GIT_CONFIG_PARAMETERS=$sanitized_config\n> + export GIT_CONFIG_PARAMETERS\n\nwhy not\n\nexport GIT_CONFIG_PARAMETERS=$santized_config\n\n?\n\nThanks,\nJake\n"},{"id":"284825","messageId":"CAGZ79kbJvgseY6aBV6vh0aEFNmHDtSvQuM8xHvp7_pYZSXCq+A@mail.gmail.com","threadId":"42135","inReplyTo":"20160428133819.GD25319@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] submodule--helper: move config-sanitizing to submodule.c","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-28T15:30:45Z","receivedAt":"2016-04-28T15:30:45Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Apr 28, 2016 at 6:38 AM, Jeff King wrote:\n> These functions should be used by any code which spawns a\n> submodule process, which may happen in submodule.c (e.g.,\n> for spawning fetch). Let's move them there and make them\n> public so that submodule--helper can continue to use them.\n>\n> Sine they're now public, let's also provide a basic overview\n> of their intended use.\n>\n> Signed-off-by: Jeff King \n> ---\n> builtin/submodule--helper.c | 48 --------------------------------------------\n> submodule.c | 49 +++++++++++++++++++++++++++++++++++++++++++++\n> submodule.h | 16 +++++++++++++++\n> 3 files changed, 65 insertions(+), 48 deletions(-)\n>\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index 3bd6883..de3ad5b 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -119,54 +119,6 @@ static int module_name(int argc, const char **argv, const char *prefix)\n> return 0;\n> }\n>\n> -/*\n> - * Rules to sanitize configuration variables that are Ok to be passed into\n> - * submodule operations from the parent project using \"-c\". Should only\n> - * include keys which are both (a) safe and (b) necessary for proper\n> - * operation.\n> - */\n> -static int submodule_config_ok(const char *var)\n> -{\n> - if (starts_with(var, \"credential.\"))\n> - return 1;\n> - return 0;\n> -}\n> -\n> -static int sanitize_submodule_config(const char *var, const char *value, void *data)\n> -{\n> - struct strbuf *out = data;\n> -\n> - if (submodule_config_ok(var)) {\n> - if (out->len)\n> - strbuf_addch(out, ' ');\n> -\n> - if (value)\n> - sq_quotef(out, \"%s=%s\", var, value);\n> - else\n> - sq_quote_buf(out, var);\n> - }\n> -\n> - return 0;\n> -}\n> -\n> -static void prepare_submodule_repo_env(struct argv_array *out)\n> -{\n> - const char * const *var;\n> -\n> - for (var = local_repo_env; *var; var++) {\n> - if (!strcmp(*var, CONFIG_DATA_ENVIRONMENT)) {\n> - struct strbuf sanitized_config = STRBUF_INIT;\n> - git_config_from_parameters(sanitize_submodule_config,\n> - &sanitized_config);\n> - argv_array_pushf(out, \"%s=%s\", *var, sanitized_config.buf);\n> - strbuf_release(&sanitized_config);\n> - } else {\n> - argv_array_push(out, *var);\n> - }\n> - }\n> -\n> -}\n> -\n> static int clone_submodule(const char *path, const char *gitdir, const char *url,\n> const char *depth, const char *reference, int quiet)\n> {\n> diff --git a/submodule.c b/submodule.c\n> index 90825e1..02eaf0e 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -13,6 +13,7 @@\n> #include \"argv-array.h\"\n> #include \"blob.h\"\n> #include \"thread-utils.h\"\n> +#include \"quote.h\"\n>\n> static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n> static int parallel_jobs = 1;\n> @@ -1129,3 +1130,51 @@ int parallel_submodules(void)\n> {\n> return parallel_jobs;\n> }\n> +\n> +/*\n> + * Rules to sanitize configuration variables that are Ok to be passed into\n> + * submodule operations from the parent project using \"-c\". Should only\n> + * include keys which are both (a) safe and (b) necessary for proper\n> + * operation.\n> + */\n> +static int submodule_config_ok(const char *var)\n> +{\n> + if (starts_with(var, \"credential.\"))\n> + return 1;\n> + return 0;\n> +}\n> +\n> +int sanitize_submodule_config(const char *var, const char *value, void *data)\n> +{\n> + struct strbuf *out = data;\n> +\n> + if (submodule_config_ok(var)) {\n> + if (out->len)\n> + strbuf_addch(out, ' ');\n> +\n> + if (value)\n> + sq_quotef(out, \"%s=%s\", var, value);\n> + else\n> + sq_quote_buf(out, var);\n> + }\n> +\n> + return 0;\n> +}\n> +\n> +void prepare_submodule_repo_env(struct argv_array *out)\n> +{\n> + const char * const *var;\n> +\n> + for (var = local_repo_env; *var; var++) {\n> + if (!strcmp(*var, CONFIG_DATA_ENVIRONMENT)) {\n> + struct strbuf sanitized_config = STRBUF_INIT;\n> + git_config_from_parameters(sanitize_submodule_config,\n> + &sanitized_config);\n> + argv_array_pushf(out, \"%s=%s\", *var, sanitized_config.buf);\n> + strbuf_release(&sanitized_config);\n> + } else {\n> + argv_array_push(out, *var);\n> + }\n> + }\n> +\n> +}\n> diff --git a/submodule.h b/submodule.h\n> index 7ef3775..7577b3b 100644\n> --- a/submodule.h\n> +++ b/submodule.h\n> @@ -61,4 +61,20 @@ int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam\n> void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n> int parallel_submodules(void);\n>\n> +/*\n> + * This function is intended as a callback for use with\n> + * git_config_from_parameters(). It ignores any config options which\n> + * are not suitable for passing along to a submodule, and accumulates the rest\n> + * in \"data\", which must be a pointer to a strbuf.\n\nSo why is it a void* then? You could make it a strbuf* here, so you\nwould not have to document it?\nOh right, because of git_config_from_parameters(sanitize_submodule_config, ...\n\n\n\n> The end result can\n> + * be put into $GIT_CONFIG_PARAMETERS for passing to a sub-process.\n\ns/sub-process/process operating on submodules/, maybe ?\n\nWhile it is technically a sub-process, I started to have an aversion\nagainst \"sub\"-things\nunless strictly required. :)\n\n> + */\n> +int sanitize_submodule_config(const char *var, const char *value, void *data);\n> +\n> +/*\n> + * Prepare the \"env_array\" parameter of a \"struct child_process\" for executing\n> + * a submodule by clearing any repo-specific envirionment variables, but\n> + * retaining any config approved by sanitize_submodule_config().\n> + */\n> +void prepare_submodule_repo_env(struct argv_array *out);\n> +\n> #endif\n> --\n> 2.8.1.617.gbdccc2d\n>\n"},{"id":"284826","messageId":"CAGZ79kY+d2GV5L_aEunWMFzPURCLkqWqtQU4ibh3-7=5MEJmEQ@mail.gmail.com","threadId":"42135","inReplyTo":"20160428152811.GC31063@sigill.intra.peff.net","subject":"Re: [PATCH 3/5] submodule: export sanitized GIT_CONFIG_PARAMETERS","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-28T15:35:21Z","receivedAt":"2016-04-28T15:35:21Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Apr 28, 2016 at 8:28 AM, Jeff King wrote:\n> On Thu, Apr 28, 2016 at 08:25:29AM -0700, Stefan Beller wrote:\n>\n>> > +test_expect_success 'cmdline credential config passes submodule update' '\n>> > + # advance the submodule HEAD so that a fetch is required\n>> > + git commit --allow-empty -m foo &&\n>> > + git push \"$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/repo.git\" HEAD &&\n>> > + sha1=$(git rev-parse HEAD) &&\n>> > + git -C super-clone update-index --cacheinfo 160000,$sha1,sub &&\n>>\n>> The use of update-index seems elegant to me, though different than\n>> any submodule test I wrote so far. :)\n>\n> Yeah, I actually wrestled with finding the shortest recipe to convince\n> git-submodule to actually call git-fetch. Suggestions welcome if there's\n> something more canonical.\n>\n> But I think we have to advance the submodule pointer in some way to\n> convince it to want to fetch (I also tried deleting the refs in the\n> cloned module, but that seemed hacky).\n>\n> I guess the way it would happen in real life is that the \"origin\" remote\n> (\"super\" here, not \"super-clone\") would make the change and commit the\n> submodule, and then \"super-clone\" would pull it.\n>\n> That seemed even more convoluted to me.\n\nThat's what I write in the other submodule tests, because I think we should\ntest closest to reality.\n\nInstead of deleting the refs code, another hacky way would be\n\n echo $newsha1 > .git/module/remote/origin/refs/heads/master\n\nbut the update-index is less hacky, so let's keep that.\n\nI did not want to point out an issue with it, just that I was pleasantly\nsurprised to see such a short test.\n\nThanks,\nStefan\n\n>\n> -Peff\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"284827","messageId":"20160428153601.GD31063@sigill.intra.peff.net","threadId":"42135","inReplyTo":"CA+P7+xoJrU884n7xG_kof46TSg_4NwS=5+nZza8PM7D=YtsXng@mail.gmail.com","subject":"Re: [PATCH 3/5] submodule: export sanitized GIT_CONFIG_PARAMETERS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-28T15:36:02Z","receivedAt":"2016-04-28T15:36:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 28, 2016 at 08:28:29AM -0700, Jacob Keller wrote:\n\n> > diff --git a/git-submodule.sh b/git-submodule.sh\n> > index 2a84d7e..3a40d4b 100755\n> > --- a/git-submodule.sh\n> > +++ b/git-submodule.sh\n> > @@ -200,6 +200,7 @@ sanitize_submodule_env()\n> > sanitized_config=$(git submodule--helper sanitize-config)\n> > clear_local_git_env\n> > GIT_CONFIG_PARAMETERS=$sanitized_config\n> > + export GIT_CONFIG_PARAMETERS\n> \n> why not\n> \n> export GIT_CONFIG_PARAMETERS=$santized_config\n\nPortability. Try:\n\n $ dash -c 'one=\"foo bar\"; export two=$one; echo $two'\n foo\n\n $ bash -c 'one=\"foo bar\"; export two=$one; echo $two'\n foo bar\n\nI think:\n\n export GIT_CONFIG_PARAMETERS=\"$sanitized_config\"\n\nsolves that. Some antique shells do not like \"export x=y\" at all, but I\ndon't know if any of them are still relevant.\n\n-Peff\n"},{"id":"284829","messageId":"20160428153751.GE31063@sigill.intra.peff.net","threadId":"42135","inReplyTo":"CAGZ79kbJvgseY6aBV6vh0aEFNmHDtSvQuM8xHvp7_pYZSXCq+A@mail.gmail.com","subject":"Re: [PATCH 4/5] submodule--helper: move config-sanitizing to submodule.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-04-28T15:37:52Z","receivedAt":"2016-04-28T15:37:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 28, 2016 at 08:30:45AM -0700, Stefan Beller wrote:\n\n> > +/*\n> > + * This function is intended as a callback for use with\n> > + * git_config_from_parameters(). It ignores any config options which\n> > + * are not suitable for passing along to a submodule, and accumulates the rest\n> > + * in \"data\", which must be a pointer to a strbuf.\n> \n> So why is it a void* then? You could make it a strbuf* here, so you\n> would not have to document it?\n> Oh right, because of git_config_from_parameters(sanitize_submodule_config, ...\n\nExactly (and why it is all the more important do document it!).\n\n> > The end result can\n> > + * be put into $GIT_CONFIG_PARAMETERS for passing to a sub-process.\n> \n> s/sub-process/process operating on submodules/, maybe ?\n> \n> While it is technically a sub-process, I started to have an aversion\n> against \"sub\"-things\n> unless strictly required. :)\n\nTechnically it can be any process which then spawns a process operating\non a submodule. Maybe \"another process\" would be enough?\n\n-Peff\n"},{"id":"284831","messageId":"CA+P7+xoiyEdKQhcb7QAQOqr2ynhmyNtg3_+Jdi3s6vsfVHMo3w@mail.gmail.com","threadId":"42135","inReplyTo":"20160428153601.GD31063@sigill.intra.peff.net","subject":"Re: [PATCH 3/5] submodule: export sanitized GIT_CONFIG_PARAMETERS","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-04-28T15:40:22Z","receivedAt":"2016-04-28T15:40:22Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Apr 28, 2016 at 8:36 AM, Jeff King wrote:\n> On Thu, Apr 28, 2016 at 08:28:29AM -0700, Jacob Keller wrote:\n>\n>> > diff --git a/git-submodule.sh b/git-submodule.sh\n>> > index 2a84d7e..3a40d4b 100755\n>> > --- a/git-submodule.sh\n>> > +++ b/git-submodule.sh\n>> > @@ -200,6 +200,7 @@ sanitize_submodule_env()\n>> > sanitized_config=$(git submodule--helper sanitize-config)\n>> > clear_local_git_env\n>> > GIT_CONFIG_PARAMETERS=$sanitized_config\n>> > + export GIT_CONFIG_PARAMETERS\n>>\n>> why not\n>>\n>> export GIT_CONFIG_PARAMETERS=$santized_config\n>\n> Portability. Try:\n>\n> $ dash -c 'one=\"foo bar\"; export two=$one; echo $two'\n> foo\n>\n> $ bash -c 'one=\"foo bar\"; export two=$one; echo $two'\n> foo bar\n>\n> I think:\n>\n> export GIT_CONFIG_PARAMETERS=\"$sanitized_config\"\n>\n> solves that. Some antique shells do not like \"export x=y\" at all, but I\n> don't know if any of them are still relevant.\n>\n> -Peff\n\nFair enough. quotes would most likely work and I know I am in the\nhabbit of using quotes a bit more liberally but this will work\ncorrectly so I wouldn't change it.\n\nThanks,\nJake\n"},{"id":"284832","messageId":"CAGZ79kY-QMfK_u7V5sXcpC8ScjyrpugLgxMaRkM2q_+y3F_j1g@mail.gmail.com","threadId":"42135","inReplyTo":"alpine.DEB.2.20.1604281600020.9313@virtualbox","subject":"Re: [PATCH 0/5] fixes for sanitized submodule config","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-04-28T15:56:04Z","receivedAt":"2016-04-28T15:56:04Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Apr 28, 2016 at 7:02 AM, Johannes Schindelin\n wrote:\n> Hi Peff,\n>\n> On Thu, 28 Apr 2016, Jeff King wrote:\n>\n>> On Thu, Apr 28, 2016 at 08:17:53AM -0400, Jeff King wrote:\n>>\n>> > So that case _is_ correct right now. It's just that t5550 isn't testing\n>> > the shell script part, which is broken. Probably running \"git submodule\n>> > update\" in the resulting clone would cover that.\n>> >\n>> > And for the fetch case, we probably just need to be calling\n>> > prepare_submodule_repo_env() there, too.\n>>\n>> So here's a series which fixes sanitizing in the \"git-submodule\" shell\n>> script, along with \"git fetch\". And cleans up a few things along the\n>> way.\n>\n> Nice!\n>\n> I reviewed those changes and they all look sensible to me (did not apply\n> them locally for lack of time, though).\n\nSame here.\n\n>\n> Ciao,\n> Dscho\n"},{"id":"284833","messageId":"CA+P7+xqU8k2bwZC3-z6+gx+fr7Tbn09Mpq1DEOsr7z-usyv6Pw@mail.gmail.com","threadId":"42135","inReplyTo":"CAGZ79kY-QMfK_u7V5sXcpC8ScjyrpugLgxMaRkM2q_+y3F_j1g@mail.gmail.com","subject":"Re: [PATCH 0/5] fixes for sanitized submodule config","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-04-28T16:03:48Z","receivedAt":"2016-04-28T16:03:48Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Apr 28, 2016 at 8:56 AM, Stefan Beller wrote:\n> On Thu, Apr 28, 2016 at 7:02 AM, Johannes Schindelin\n> wrote:\n>> Hi Peff,\n>>\n>> On Thu, 28 Apr 2016, Jeff King wrote:\n>>\n>>> On Thu, Apr 28, 2016 at 08:17:53AM -0400, Jeff King wrote:\n>>>\n>>> > So that case _is_ correct right now. It's just that t5550 isn't testing\n>>> > the shell script part, which is broken. Probably running \"git submodule\n>>> > update\" in the resulting clone would cover that.\n>>> >\n>>> > And for the fetch case, we probably just need to be calling\n>>> > prepare_submodule_repo_env() there, too.\n>>>\n>>> So here's a series which fixes sanitizing in the \"git-submodule\" shell\n>>> script, along with \"git fetch\". And cleans up a few things along the\n>>> way.\n>>\n>> Nice!\n>>\n>> I reviewed those changes and they all look sensible to me (did not apply\n>> them locally for lack of time, though).\n>\n> Same here.\n>\n\nSame as well. Looks good, and good catch finding the bug!\n\nThanks,\nJake\n\n>>\n>> Ciao,\n>> Dscho\n"},{"id":"284836","messageId":"6EAB03CC-F52B-463C-98EC-C1C91B3B00EB@gmail.com","threadId":"42135","inReplyTo":"20160428133819.GD25319@sigill.intra.peff.net","subject":"Re: [PATCH 4/5] submodule--helper: move config-sanitizing to submodule.c","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-04-28T16:28:24Z","receivedAt":"2016-04-28T16:28:24Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n\n> On 28 Apr 2016, at 15:38, Jeff King wrote:\n> \n> These functions should be used by any code which spawns a\n> submodule process, which may happen in submodule.c (e.g.,\n> for spawning fetch). Let's move them there and make them\n> public so that submodule--helper can continue to use them.\n> \n> Sine they're now public, let's also\ns/Sine/Since\n\nThanks for fixing this entire issue, Peff!\n\n- Lars\n\n> provide a basic overview\n> of their intended use.\n> \n> Signed-off-by: Jeff King \n> ---\n> builtin/submodule--helper.c | 48 --------------------------------------------\n> submodule.c | 49 +++++++++++++++++++++++++++++++++++++++++++++\n> submodule.h | 16 +++++++++++++++\n> 3 files changed, 65 insertions(+), 48 deletions(-)\n> \n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index 3bd6883..de3ad5b 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -119,54 +119,6 @@ static int module_name(int argc, const char **argv, const char *prefix)\n> return 0;\n> }\n> \n> -/*\n> - * Rules to sanitize configuration variables that are Ok to be passed into\n> - * submodule operations from the parent project using \"-c\". Should only\n> - * include keys which are both (a) safe and (b) necessary for proper\n> - * operation.\n> - */\n> -static int submodule_config_ok(const char *var)\n> -{\n> - if (starts_with(var, \"credential.\"))\n> - return 1;\n> - return 0;\n> -}\n> -\n> -static int sanitize_submodule_config(const char *var, const char *value, void *data)\n> -{\n> - struct strbuf *out = data;\n> -\n> - if (submodule_config_ok(var)) {\n> - if (out->len)\n> - strbuf_addch(out, ' ');\n> -\n> - if (value)\n> - sq_quotef(out, \"%s=%s\", var, value);\n> - else\n> - sq_quote_buf(out, var);\n> - }\n> -\n> - return 0;\n> -}\n> -\n> -static void prepare_submodule_repo_env(struct argv_array *out)\n> -{\n> - const char * const *var;\n> -\n> - for (var = local_repo_env; *var; var++) {\n> - if (!strcmp(*var, CONFIG_DATA_ENVIRONMENT)) {\n> - struct strbuf sanitized_config = STRBUF_INIT;\n> - git_config_from_parameters(sanitize_submodule_config,\n> - &sanitized_config);\n> - argv_array_pushf(out, \"%s=%s\", *var, sanitized_config.buf);\n> - strbuf_release(&sanitized_config);\n> - } else {\n> - argv_array_push(out, *var);\n> - }\n> - }\n> -\n> -}\n> -\n> static int clone_submodule(const char *path, const char *gitdir, const char *url,\n> const char *depth, const char *reference, int quiet)\n> {\n> diff --git a/submodule.c b/submodule.c\n> index 90825e1..02eaf0e 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -13,6 +13,7 @@\n> #include \"argv-array.h\"\n> #include \"blob.h\"\n> #include \"thread-utils.h\"\n> +#include \"quote.h\"\n> \n> static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n> static int parallel_jobs = 1;\n> @@ -1129,3 +1130,51 @@ int parallel_submodules(void)\n> {\n> return parallel_jobs;\n> }\n> +\n> +/*\n> + * Rules to sanitize configuration variables that are Ok to be passed into\n> + * submodule operations from the parent project using \"-c\". Should only\n> + * include keys which are both (a) safe and (b) necessary for proper\n> + * operation.\n> + */\n> +static int submodule_config_ok(const char *var)\n> +{\n> + if (starts_with(var, \"credential.\"))\n> + return 1;\n> + return 0;\n> +}\n> +\n> +int sanitize_submodule_config(const char *var, const char *value, void *data)\n> +{\n> + struct strbuf *out = data;\n> +\n> + if (submodule_config_ok(var)) {\n> + if (out->len)\n> + strbuf_addch(out, ' ');\n> +\n> + if (value)\n> + sq_quotef(out, \"%s=%s\", var, value);\n> + else\n> + sq_quote_buf(out, var);\n> + }\n> +\n> + return 0;\n> +}\n> +\n> +void prepare_submodule_repo_env(struct argv_array *out)\n> +{\n> + const char * const *var;\n> +\n> + for (var = local_repo_env; *var; var++) {\n> + if (!strcmp(*var, CONFIG_DATA_ENVIRONMENT)) {\n> + struct strbuf sanitized_config = STRBUF_INIT;\n> + git_config_from_parameters(sanitize_submodule_config,\n> + &sanitized_config);\n> + argv_array_pushf(out, \"%s=%s\", *var, sanitized_config.buf);\n> + strbuf_release(&sanitized_config);\n> + } else {\n> + argv_array_push(out, *var);\n> + }\n> + }\n> +\n> +}\n> diff --git a/submodule.h b/submodule.h\n> index 7ef3775..7577b3b 100644\n> --- a/submodule.h\n> +++ b/submodule.h\n> @@ -61,4 +61,20 @@ int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam\n> void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n> int parallel_submodules(void);\n> \n> +/*\n> + * This function is intended as a callback for use with\n> + * git_config_from_parameters(). It ignores any config options which\n> + * are not suitable for passing along to a submodule, and accumulates the rest\n> + * in \"data\", which must be a pointer to a strbuf. The end result can\n> + * be put into $GIT_CONFIG_PARAMETERS for passing to a sub-process.\n> + */\n> +int sanitize_submodule_config(const char *var, const char *value, void *data);\n> +\n> +/*\n> + * Prepare the \"env_array\" parameter of a \"struct child_process\" for executing\n> + * a submodule by clearing any repo-specific envirionment variables, but\n> + * retaining any config approved by sanitize_submodule_config().\n> + */\n> +void prepare_submodule_repo_env(struct argv_array *out);\n> +\n> #endif\n> -- \n> 2.8.1.617.gbdccc2d\n> \n"},{"id":"284842","messageId":"alpine.DEB.2.20.1604281840350.9313@virtualbox","threadId":"42135","inReplyTo":"20160428152811.GC31063@sigill.intra.peff.net","subject":"Re: [PATCH 3/5] submodule: export sanitized GIT_CONFIG_PARAMETERS","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-04-28T16:51:43Z","receivedAt":"2016-04-28T16:51:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 28 Apr 2016, Jeff King wrote:\n\n> On Thu, Apr 28, 2016 at 08:25:29AM -0700, Stefan Beller wrote:\n> \n> > > +test_expect_success 'cmdline credential config passes submodule update' '\n> > > + # advance the submodule HEAD so that a fetch is required\n> > > + git commit --allow-empty -m foo &&\n> > > + git push \"$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/repo.git\" HEAD &&\n> > > + sha1=$(git rev-parse HEAD) &&\n> > > + git -C super-clone update-index --cacheinfo 160000,$sha1,sub &&\n> > \n> > The use of update-index seems elegant to me, though different than\n> > any submodule test I wrote so far. :)\n> \n> Yeah, I actually wrestled with finding the shortest recipe to convince\n> git-submodule to actually call git-fetch. Suggestions welcome if there's\n> something more canonical.\n\nFWIW that's exactly how I did things in\nhttps://github.com/dscho/git/commit/89d0024450b0e6e9997ad9e3d681248bde1bafc0\n\n:-)\n\nCiao,\nDscho\n"}]}