Web lists-archives.com

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

On Wed, 14 Nov 2018 at 16:26, Gaël Lhez via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
> However, the `.lock` file was still open and on Windows that means
> that it could not be deleted properly. This patch fixes that issue.

Hmmm, doesn't the tempfile machinery remove the lock file when we die?

>         ref_count = write_bundle_refs(bundle_fd, &revs);
> -       if (!ref_count)
> -               die(_("Refusing to create empty bundle."));
> -       else if (ref_count < 0)
> +       if (ref_count <= 0)  {
> +               if (!ref_count)
> +                       error(_("Refusing to create empty bundle."));
>                 goto err;
> +       }

One less `die()` in libgit -- nice.

> +test_expect_success 'try to create a bundle with empty ref count' '
> +       test_expect_code 1 git bundle create foobar.bundle master..master
> +'

This tries to make sure that we don't just die, but that we exit in a
"controlled" way with exit code 1. After reading the log message, I had
perhaps expected something like

  test_must_fail git bundle [...] &&
  test_path_is_missing foobar.bundle.lock

That relies on magically knowing the ".lock" suffix, but my point is
that for me (on Linux), this alternative test passes both before and
after the patch. Before, because tempfile.c cleans up; after, because
bundle.c does so. Doesn't this happen on Windows too? What am I missing?