[1/2] stash: get git_stash_config at the top level

Commit Message

Thomas Gummerer March 2, 2020, 6:18 p.m. UTC

On 02/25, Junio C Hamano wrote:
> Son Luong Ngoc <[e-mail]> writes:
> 
> > I have been trying to build git from source and noticing that some
> > tests have been failing since 2.25 with the flag
> > "GIT_TEST_STASH_USE_BUILTIN=false"
> >
> > I think in 2.25 t3903.103 started to fail (rebase related) and
> > current master t3904 may be failing also.
> >
> > Is "GIT_TEST_STASH_USE_BUILTIN=false" is still being tested with
> > or are we totally deprecating this flag?
> 
> In the longer term, when "git stash" gains new features that did not
> exist in the original scripted version, tests that observe how these
> features work would start failing when using the scripted version.
> 
> I picked some people from "git shortlog --no-merges builtin/stash.c"
> and placed them on the CC line---perhaps they may know more.  It
> happens that Johannes is also familiar with "rebase", which you
> said is involved in the test failure, so I'd imagine he would be the
> best person to ask.

Thanks for the report Son, and sorry for taking so long.  I'm a little
behing on reading my emails right now.

I think it is time to get rid of legacy stash.  Nobody seems to have
noticed this test failure until now, but according to your bisection
it looks like this test has been failing since 2.23, which I confirmed
locally.   In addition, the last bugfix that was related to the
rewrite was in 2.25, though that was really a fix for another fix in
2.24 that we failed to catch earlier.

I think 2.26 should still ship with the option, but after that we can
probably get rid of it.  So here's a couple of patches to do just
that, for merging after 2.26 ships.

--- >8 ---
In the next commit we're adding another config variable to be read
from 'git_stash_config', that is valid for the top level command
instead of just a subset.  Move the 'git_config' invocation for
'git_stash_config' to the top-level to prepare for that.

Signed-off-by: Thomas Gummerer <[e-mail]>
---
 builtin/stash.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Patch

diff --git a/builtin/stash.c b/builtin/stash.c
index 879fc5f368..f371db270c 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -712,7 +712,7 @@  static int git_stash_config(const char *var, const char *value, void *cb)
 		show_patch = git_config_bool(var, value);
 		return 0;
 	}
-	return git_default_config(var, value, cb);
+	return git_diff_basic_config(var, value, cb);
 }
 
 static int show_stash(int argc, const char **argv, const char *prefix)
@@ -749,7 +749,6 @@  static int show_stash(int argc, const char **argv, const char *prefix)
 	 * any options.
 	 */
 	if (revision_args.argc == 1) {
-		git_config(git_stash_config, NULL);
 		if (show_stat)
 			rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT;
 
@@ -1573,7 +1572,7 @@  int cmd_stash(int argc, const char **argv, const char *prefix)
 	trace_repo_setup(prefix);
 	setup_work_tree();
 
-	git_config(git_diff_basic_config, NULL);
+	git_config(git_stash_config, NULL);
 
 	argc = parse_options(argc, argv, prefix, options, git_stash_usage,
 			     PARSE_OPT_KEEP_UNKNOWN | PARSE_OPT_KEEP_DASHDASH);