On Thu, May 05, 2016 at 09:59:15AM -0700, Junio C Hamano wrote: > Sorry for the trouble. > > I queued jk/submodule-c-credential to be mergeable to 'maint', as it > could be argued two ways. We can say that not propagating -c down > is a bug and the series is a bugfix. Or it is merely a known > limitation and the series is a new feature. I was leaning towards > the former, but I was also willing to declare that is a known bug > that will left unfixed in the maintenance track. Ah, OK, that makes perfect sense. > It probably is a good time to merge jk/submodule-config-sanitize-fix > into jk/submodule-c-credential (i.e. a mere fast-forward), remove > that "-fix" branch, and apply this patch directly on top of the > resulting jk/submodule-c-credential. That will make the whole thing > a 13-patch series, consisting of: > > 7 patches up to the current jk/submodule-c-credential > d1f8849 git_config_push_parameter: handle empty GIT_CONFIG_PARAMETERS > 14111fc git: submodule honor -c credential.* from command line > e70986d quote: implement sq_quotef() > 7dad263 submodule: fix segmentation fault in submodule--helper clone > 717416c submodule: fix submodule--helper clone usage > 08e0970 submodule: check argc count for git submodule--helper clone > d10e3b4 submodule: don't pass empty string arguments to submodule--helper clone > > 5 patches up to the current jk/submodule-config-sanitize-fix > c12e865 submodule: use prepare_submodule_repo_env consistently > 4638728 submodule--helper: move config-sanitizing to submodule.c > 860cba6 submodule: export sanitized GIT_CONFIG_PARAMETERS > 455d22c t5550: break submodule config test into multiple sub-tests > 1149ee2 t5550: fix typo in $HTTPD_URL > > 1 patch (this one) > 4e6706a submodule: stop sanitizing config options That sounds reasonable. Note that the later patches drop the only caller of the newly-introduced sq_quotef(). So we could revert e70986d (omitting it from the series doesn't make sense, as it would leave a broken state in the middle). I am also fine with leaving it. It seems like a potentially useful addition. I had originally thought after the final one that we could further clean up by turning prepare_submodule_repo_env() into a static function. But we can't; it gets called in one spot from submodule--helper. However, while looking at it, I did notice that we probably want to squash this into the final patch (since sanitize_submodule_config went away completely): diff --git a/submodule.h b/submodule.h index 48690b1..869d259 100644 --- a/submodule.h +++ b/submodule.h @@ -43,19 +43,10 @@ int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name); void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir); -/* - * This function is intended as a callback for use with - * git_config_from_parameters(). It ignores any config options which - * are not suitable for passing along to a submodule, and accumulates the rest - * in "data", which must be a pointer to a strbuf. The end result can - * be put into $GIT_CONFIG_PARAMETERS for passing to a sub-process. - */ -int sanitize_submodule_config(const char *var, const char *value, void *data); - /* * Prepare the "env_array" parameter of a "struct child_process" for executing * a submodule by clearing any repo-specific envirionment variables, but - * retaining any config approved by sanitize_submodule_config(). + * retaining any config in the environment. */ void prepare_submodule_repo_env(struct argv_array *out); -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html