Re: [PATCH v2 1/1] bundle: cleanup lock files on error

[Jeff King <peff@peff.net>]

On Thu, Nov 15, 2018 at 05:32:15PM +0100, Johannes Schindelin wrote:

> I cannot claim that I wrapped my head around your explanation or your diff (I am
> busy trying to prepare Git for Windows' `master` for rebasing to v2.20.0-rc0),
> but it does fix the problem. Thank you so much!
> 
> The line `test_expect_code 1 ...` needs to be adjusted to `test_expect_code
> 128`, of course, and to discern from the fixed problem (which also exits with
> code 128), the error output should be verified, like so:
> 
> -- snip --
> test_expect_success 'try to create a bundle with empty ref count' '
> 	test_must_fail git bundle create foobar.bundle master..master 2>err &&
> 	test_i18ngrep "Refusing to create empty bundle" err
> '
> -- snap --

It seems like we should be checking that the stale lockfile isn't left,
which is the real problem (the warning is annoying, but is a symptom of
the same thing). I.e., something like:

  test_must_fail git bundle create foobar.bundle master..master &&
  test_path_is_missing foobar.bundle.lock

That would already pass on non-Windows platforms, but that's OK. It will
never give a false failure.

If you don't mind, can you confirm that the test above fails without
either of the two patches under discussion?

> Do you want to integrate this test into your patch and run with it, or do you
> want me to shepherd your patch?

I'll wrap it up with a commit message and a test.

-Peff