Re: git submodule should honor "-c credential.helper" command line argument
On Fri, Feb 19, 2016 at 9:33 AM, Junio C Hamanowrote: > Jeff King writes: > >> That being said, I am not sure this is the right solution. In the thread >> I linked earlier[1], Jens indicated he would prefer not to blindly share >> config with the submodules, and I think I agree. Or are you proposing to >> pick and choose the keys in GIT_CONFIG_PARAMETERS, and whitelist >> credential.*? > > Yes, I think it is sensible not to propagate by default, and pass > ones that are absolutely safe, sane and necessary by whitelisting. Yes I agree. I'm trying to think of a reasonable way to implement this as part of a submodule--helper.. something like whitelist-config or something, which would just output the value of GIT_CONFIG_PARAMETERS after whitelisting. It would be up to the shell code to then use this inside a subshell. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule should honor "-c credential.helper" command line argument
Jeff Kingwrites: > That being said, I am not sure this is the right solution. In the thread > I linked earlier[1], Jens indicated he would prefer not to blindly share > config with the submodules, and I think I agree. Or are you proposing to > pick and choose the keys in GIT_CONFIG_PARAMETERS, and whitelist > credential.*? Yes, I think it is sensible not to propagate by default, and pass ones that are absolutely safe, sane and necessary by whitelisting. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule should honor "-c credential.helper" command line argument
On Thu, Feb 18, 2016 at 11:46 PM, Jeff Kingwrote: > To trigger a credential fetch in actual use, you have to clone over > http. See the credential tests in t5550, for example. > I'll look at these. >> As for how to whitelist config to share with the submodule I am really >> not 100% sure, since we just clear GIT_CONFIG_PARAMETERS, and I think >> we'd need a specialized variant of clear_local_git_env_vars specific >> to submodule then. > > Yeah, you'll have to parse, which is pretty painful. In C, you'd do > something like: > > but right now git-submodule.sh is all in shell. You'd probably need a > special helper from git-submodule--helper, though it might simply make > sense to put this off until the submodule code is fully ported to C. > I think the best approach is probably to put this off since I am not 100% sure how we'd handle environment variables from swapping into submoduler--helper and out. That seems really finicky and likely to go away once the full port occurs so I am not sure it's worth it. > -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule should honor "-c credential.helper" command line argument
On Thu, Feb 18, 2016 at 11:29:09PM -0800, Jacob Keller wrote: > I would prefer to either.. blacklist stuff like core.worktree, or > whitelist a bunch of stuff that makes sense. In this case though, I > would prefer to have an explicit test of credential.helper, but I > don't know if any of our tests actually have a solid test case for > "credential.helper was used in a clone. There may not be test > infrastructure for this though, so your test might work well enough. To trigger a credential fetch in actual use, you have to clone over http. See the credential tests in t5550, for example. > As for how to whitelist config to share with the submodule I am really > not 100% sure, since we just clear GIT_CONFIG_PARAMETERS, and I think > we'd need a specialized variant of clear_local_git_env_vars specific > to submodule then. Yeah, you'll have to parse, which is pretty painful. In C, you'd do something like: int submodule_config_ok(const char *var) { if (starts_with(var, "credential.")) return 1; return 0; } int filter_submodule_config(const char *var, const char *value, void *data) { struct strbuf *out = data; if (submodule_config_ok(var)) { if (out->len) strbuf_addch(out, ' '); /* these actually probably need quoted all as * one string */ sq_quote_buf(out, var); sq_quote_buf(out, "="); sq_quote_buf(out, value); } return 0; } and then call it like: struct strbuf filtered_config = STRBUF_INIT; git_config_from_parameters(filter_submodule_config, _config); argv_array_pushf(_process.env, "%s=%s", CONFIG_DATA_ENVIRONMENT, filtered_config.buf); but right now git-submodule.sh is all in shell. You'd probably need a special helper from git-submodule--helper, though it might simply make sense to put this off until the submodule code is fully ported to C. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule should honor "-c credential.helper" command line argument
On Thu, Feb 18, 2016 at 8:30 PM, Jeff Kingwrote: > On Thu, Feb 18, 2016 at 05:15:54PM -0800, Jacob Keller wrote: > >> I am looking at this more and I am stuck as to how best to provide a >> test case. >> >> I think the problem as stated above is pretty straight forward, we >> just want to stop clearing GIT_CONFIG_PARAMETERS but I can't find an >> easy way to test that we've done the right thing. There are no current >> tests for using a credential helper with submodule update right now. > > If you just want to test that GIT_CONFIG_PARAMETERS is left untouched in > the submodule, you can tweak any config setting that would impact the > newly-cloned repo. E.g. this: > > unset GIT_COMMITTER_NAME > git config --global user.name='Global Name' > git -c user.name='Command-Line Name' clone repo-with-module foo > head -1 foo/.git/logs/HEAD > head -1 foo/.git/modules/sub/logs/HEAD > > shows that the parent-level clone uses the "-c" name, but the submodule > does not. > > That being said, I am not sure this is the right solution. In the thread > I linked earlier[1], Jens indicated he would prefer not to blindly share > config with the submodules, and I think I agree. Or are you proposing to > pick and choose the keys in GIT_CONFIG_PARAMETERS, and whitelist > credential.*? > > In that case, obviously my test example would not work, though I think > that it might be fine to put "user.name" onto the whitelist (the things > we really would worry about is stuff like "core.worktree" that clearly > does not make sense to carry over into the submodule). > > -Peff > > [1] http://thread.gmane.org/gmane.comp.version-control.git/264840 > I would prefer to either.. blacklist stuff like core.worktree, or whitelist a bunch of stuff that makes sense. In this case though, I would prefer to have an explicit test of credential.helper, but I don't know if any of our tests actually have a solid test case for "credential.helper was used in a clone. There may not be test infrastructure for this though, so your test might work well enough. As for how to whitelist config to share with the submodule I am really not 100% sure, since we just clear GIT_CONFIG_PARAMETERS, and I think we'd need a specialized variant of clear_local_git_env_vars specific to submodule then. Thanks, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule should honor "-c credential.helper" command line argument
On Thu, Feb 18, 2016 at 05:15:54PM -0800, Jacob Keller wrote: > I am looking at this more and I am stuck as to how best to provide a > test case. > > I think the problem as stated above is pretty straight forward, we > just want to stop clearing GIT_CONFIG_PARAMETERS but I can't find an > easy way to test that we've done the right thing. There are no current > tests for using a credential helper with submodule update right now. If you just want to test that GIT_CONFIG_PARAMETERS is left untouched in the submodule, you can tweak any config setting that would impact the newly-cloned repo. E.g. this: unset GIT_COMMITTER_NAME git config --global user.name='Global Name' git -c user.name='Command-Line Name' clone repo-with-module foo head -1 foo/.git/logs/HEAD head -1 foo/.git/modules/sub/logs/HEAD shows that the parent-level clone uses the "-c" name, but the submodule does not. That being said, I am not sure this is the right solution. In the thread I linked earlier[1], Jens indicated he would prefer not to blindly share config with the submodules, and I think I agree. Or are you proposing to pick and choose the keys in GIT_CONFIG_PARAMETERS, and whitelist credential.*? In that case, obviously my test example would not work, though I think that it might be fine to put "user.name" onto the whitelist (the things we really would worry about is stuff like "core.worktree" that clearly does not make sense to carry over into the submodule). -Peff [1] http://thread.gmane.org/gmane.comp.version-control.git/264840 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule should honor "-c credential.helper" command line argument
On Sun, Feb 7, 2016 at 7:44 PM, Jacob Kellerwrote: > On Sun, Feb 7, 2016 at 5:48 AM, Marc Strapetz > wrote: >> On 07.02.2016 05:41, Jacob Keller wrote: >>> >>> On Wed, Feb 3, 2016 at 3:44 PM, Jacob Keller >>> wrote: Ok so I am not sure we even really need to use "-c" option in git-clone considering that we can just use the same flow we do for setting core.worktree values. I'll propose a patch with you two Cc'ed, which I think fixes the issue. There may actually be a set of configuration we want to include though, and the main issue I see is that it won't get updated correctly whenever the parent configuration changes. Thanks, Jake >>> >>> >>> I tried adding the config as part of module_clone in >>> submodule--helper.c but it didn't pass the test I added. I haven't had >>> time to look at this in the last few days, but I am stuck as to why >>> submodule--helper.c appeared to not use module_clone as I thought. >> >> >> I've tried to just comment out clearing of environment variables in >> git-sh-setup.sh, clear_local_git_env(). I've noticed that "-c >> credentials-helper ..." is stored in $GIT_CONFIG_PARAMETERS and with >> existing code is reset there. If not clearing the environment variables, at >> least "git submodule init" is working properly. I didn't try with other >> commands nor to run tests. >> >> -Marc >> >> > > I'll have to dig more into this next week. > > Regards, > Jake I am looking at this more and I am stuck as to how best to provide a test case. I think the problem as stated above is pretty straight forward, we just want to stop clearing GIT_CONFIG_PARAMETERS but I can't find an easy way to test that we've done the right thing. There are no current tests for using a credential helper with submodule update right now. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule should honor "-c credential.helper" command line argument
On 07.02.2016 05:41, Jacob Keller wrote: On Wed, Feb 3, 2016 at 3:44 PM, Jacob Kellerwrote: Ok so I am not sure we even really need to use "-c" option in git-clone considering that we can just use the same flow we do for setting core.worktree values. I'll propose a patch with you two Cc'ed, which I think fixes the issue. There may actually be a set of configuration we want to include though, and the main issue I see is that it won't get updated correctly whenever the parent configuration changes. Thanks, Jake I tried adding the config as part of module_clone in submodule--helper.c but it didn't pass the test I added. I haven't had time to look at this in the last few days, but I am stuck as to why submodule--helper.c appeared to not use module_clone as I thought. I've tried to just comment out clearing of environment variables in git-sh-setup.sh, clear_local_git_env(). I've noticed that "-c credentials-helper ..." is stored in $GIT_CONFIG_PARAMETERS and with existing code is reset there. If not clearing the environment variables, at least "git submodule init" is working properly. I didn't try with other commands nor to run tests. -Marc -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule should honor "-c credential.helper" command line argument
On Sun, Feb 7, 2016 at 5:48 AM, Marc Strapetzwrote: > On 07.02.2016 05:41, Jacob Keller wrote: >> >> On Wed, Feb 3, 2016 at 3:44 PM, Jacob Keller >> wrote: >>> >>> Ok so I am not sure we even really need to use "-c" option in >>> git-clone considering that we can just use the same flow we do for >>> setting core.worktree values. I'll propose a patch with you two Cc'ed, >>> which I think fixes the issue. There may actually be a set of >>> configuration we want to include though, and the main issue I see is >>> that it won't get updated correctly whenever the parent configuration >>> changes. >>> >>> Thanks, >>> Jake >> >> >> I tried adding the config as part of module_clone in >> submodule--helper.c but it didn't pass the test I added. I haven't had >> time to look at this in the last few days, but I am stuck as to why >> submodule--helper.c appeared to not use module_clone as I thought. > > > I've tried to just comment out clearing of environment variables in > git-sh-setup.sh, clear_local_git_env(). I've noticed that "-c > credentials-helper ..." is stored in $GIT_CONFIG_PARAMETERS and with > existing code is reset there. If not clearing the environment variables, at > least "git submodule init" is working properly. I didn't try with other > commands nor to run tests. > > -Marc > > I'll have to dig more into this next week. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule should honor "-c credential.helper" command line argument
On Wed, Feb 3, 2016 at 3:44 PM, Jacob Kellerwrote: > Ok so I am not sure we even really need to use "-c" option in > git-clone considering that we can just use the same flow we do for > setting core.worktree values. I'll propose a patch with you two Cc'ed, > which I think fixes the issue. There may actually be a set of > configuration we want to include though, and the main issue I see is > that it won't get updated correctly whenever the parent configuration > changes. > > Thanks, > Jake I tried adding the config as part of module_clone in submodule--helper.c but it didn't pass the test I added. I haven't had time to look at this in the last few days, but I am stuck as to why submodule--helper.c appeared to not use module_clone as I thought. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule should honor "-c credential.helper" command line argument
On 03.02.2016 08:35, Jacob Keller wrote: On Tue, Feb 2, 2016 at 8:25 PM, Jeff Kingwrote: I think the problem is that when git "switches" to working in the submodule repository, it clears the environment, which includes any "-c" command switches. This makes sense for some situations, but not for others. This thread shows a similar problem: http://thread.gmane.org/gmane.comp.version-control.git/264840 Jens suggested there adding an option to tell clone to pass specific variables to the submodule, which I think makes sense. AFAIK, nobody has done any work yet on that approach. This is something that I am also interested in, haven't had a chance to look at it just yet though. I may have some time soon to take a stab at this. To have this issue addressed would be great. It's currently preventing me to switch our authentication-related code to credential helpers. Jake, I'm happy to help, if I can (I don't have any experience with Git code though, except of browsing it a couple of times ... so patches from my side would need a lot of review I guess :). Just let me know. -Marc -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule should honor "-c credential.helper" command line argument
On Tue, Feb 2, 2016 at 8:25 PM, Jeff Kingwrote: > On Tue, Feb 02, 2016 at 06:13:14PM +0100, Marc Strapetz wrote: > >> git -c credential.helper=helper submodule update --init submodule >> >> does not invoke "helper", but falls back to the default strategies. >> When configuring in ~/.gitconfig: >> >> [credential] >> helper=helper >> >> git submodule update --init submodule >> >> works fine. This behavior is somewhat unexpected -- is this a bug or by >> intention? In case intention, what's the recommended way to "inject" >> credentials helpers to work on submodules without modifying Git's config >> files? >> >> Tested with Git 2.5.0 (Windows). > > I think the problem is that when git "switches" to working in the > submodule repository, it clears the environment, which includes any "-c" > command switches. This makes sense for some situations, but not for > others. This thread shows a similar problem: > > http://thread.gmane.org/gmane.comp.version-control.git/264840 > > Jens suggested there adding an option to tell clone to pass specific > variables to the submodule, which I think makes sense. AFAIK, nobody has > done any work yet on that approach. > > -Peff > -- Ok so I am not sure we even really need to use "-c" option in git-clone considering that we can just use the same flow we do for setting core.worktree values. I'll propose a patch with you two Cc'ed, which I think fixes the issue. There may actually be a set of configuration we want to include though, and the main issue I see is that it won't get updated correctly whenever the parent configuration changes. Thanks, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule should honor "-c credential.helper" command line argument
On Tue, Feb 02, 2016 at 06:13:14PM +0100, Marc Strapetz wrote: > git -c credential.helper=helper submodule update --init submodule > > does not invoke "helper", but falls back to the default strategies. > When configuring in ~/.gitconfig: > > [credential] > helper=helper > > git submodule update --init submodule > > works fine. This behavior is somewhat unexpected -- is this a bug or by > intention? In case intention, what's the recommended way to "inject" > credentials helpers to work on submodules without modifying Git's config > files? > > Tested with Git 2.5.0 (Windows). I think the problem is that when git "switches" to working in the submodule repository, it clears the environment, which includes any "-c" command switches. This makes sense for some situations, but not for others. This thread shows a similar problem: http://thread.gmane.org/gmane.comp.version-control.git/264840 Jens suggested there adding an option to tell clone to pass specific variables to the submodule, which I think makes sense. AFAIK, nobody has done any work yet on that approach. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git submodule should honor "-c credential.helper" command line argument
git -c credential.helper=helper submodule update --init submodule does not invoke "helper", but falls back to the default strategies. When configuring in ~/.gitconfig: [credential] helper=helper git submodule update --init submodule works fine. This behavior is somewhat unexpected -- is this a bug or by intention? In case intention, what's the recommended way to "inject" credentials helpers to work on submodules without modifying Git's config files? Tested with Git 2.5.0 (Windows). -Marc -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule should honor "-c credential.helper" command line argument
On Tue, Feb 2, 2016 at 8:25 PM, Jeff Kingwrote: > I think the problem is that when git "switches" to working in the > submodule repository, it clears the environment, which includes any "-c" > command switches. This makes sense for some situations, but not for > others. This thread shows a similar problem: > > http://thread.gmane.org/gmane.comp.version-control.git/264840 > > Jens suggested there adding an option to tell clone to pass specific > variables to the submodule, which I think makes sense. AFAIK, nobody has > done any work yet on that approach. > This is something that I am also interested in, haven't had a chance to look at it just yet though. I may have some time soon to take a stab at this. Regards, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html