Re: [PATCH v12 18/26] stash: convert push to builtin

From: Thomas Gummerer <t.gummerer@gmail.com>
To: "SZEDER Gábor" <szeder.dev@gmail.com>
Cc: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>,
	git@vger.kernel.org, Johannes.Schindelin@gmx.de
Subject: Re: [PATCH v12 18/26] stash: convert push to builtin
Date: Tue, 12 Feb 2019 23:18:37 +0000	[thread overview]
Message-ID: <20190212231837.GI6085@hank.intra.tgummerer.com> (raw)
In-Reply-To: <20190211011306.GA31807@szeder.dev>

On 02/11, SZEDER Gábor wrote:
> On Sun, Feb 10, 2019 at 10:17:12PM +0000, Thomas Gummerer wrote:
> > On 02/08, SZEDER Gábor wrote:
> > > On Thu, Dec 20, 2018 at 09:44:34PM +0200, Paul-Sebastian Ungureanu wrote:
> > > > Add stash push to the helper.
> > > > 
> > > > Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com>
> > > 
> > > This patch causes rare failures in 't3903-stash.sh', I've seen it
> > > break for the first time today in a Travis CI build:
> > 
> > Thanks for reporting this.  I was going to take a look at it, but
> > unfortunately I can't seem to reproduce the issue even with --stress
> > (I let it run for ~1000 repetitions and then aborted it).
> > 
> > Which platform did you see/test this on, and which compile options did
> > you use?  I went through a failures on
> > https://travis-ci.org/git/git/builds, but couldn't find this
> > particular one.  Could you point me at the failed run?
> 
> It was in a Linux build job in one of my custom CI builds [1]:
> 
>   https://travis-ci.org/szeder/git-cooking-topics-for-travis-ci/jobs/490420713#L3401
> 
> and I tested it locally on Linux as well.  I don't think there are any
> unusual compiler options.  I run it with --stress on Travis CI's macOS
> as well, but couldn't trigger a failure there.

Thanks.  I still didn't manage to reproduce it locally, but I was now
able to test it on Travis CI.

The diff below fixes the issue, but I still need to spend some time to
better understand why it does.  I'll hopefully be in a position to
send a patch with a proper log message why this is the right fix in
the next couple of days.

diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index c77f62c895..3dab488bd6 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -231,6 +231,7 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)
 	struct tree *tree;
 	struct lock_file lock_file = LOCK_INIT;
 
+	discard_cache();
 	read_cache_preload(NULL);
 	if (refresh_cache(REFRESH_QUIET))
 		return -1;


> [1] That build log doesn't include the trash dir of the failed test
>     in the log, most likely because of a faulty merge conflict
>     resolution on my part, but you can find a failed trash dir
>     embedded here:
> 
>       https://travis-ci.org/szeder/git/jobs/491401882#L2309
> 
