All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: What's cooking in git.git (Jan 2022, #03; Thu, 13)
Date: Mon, 17 Jan 2022 08:18:31 +0100	[thread overview]
Message-ID: <YeUYR8TwJf+31wcl@ncase> (raw)
In-Reply-To: <220114.86fspqrxbw.gmgdl@evledraar.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3573 bytes --]

On Fri, Jan 14, 2022 at 07:12:15PM +0100, Ævar Arnfjörð Bjarmason wrote:
> 
> On Thu, Jan 13 2022, Junio C Hamano wrote:
> 
> > Junio C Hamano <gitster@pobox.com> writes:
> >
> >> There are a few "oops, what we merged recently is broken" topics
> >> that still are not in 'master', but otherwise what we have should
> >> be pretty much what we'll have in the final one.
> >>
> >>  - I am reasonably happy with ab/refs-errno-cleanup (just one patch)
> >>    that fixes the incorrect state of the code left by the earlier
> >>    parts of the topic that have already been merged during this
> >>    cycle.
> >>
> >>  - I am also OK with ab/reftable-build-fixes (two patches), one for
> >>    general type correctness fix, the other for helping older sub-C99
> >>    compilers.
> >>
> >> If there are fixes for regressions that we introduced during this
> >> cycle other than these two topics, I certainly am missing them, so
> >> please holler loudly and quickly, hopefully in time for me to tag
> >> the -rc1 tomorrow.
> >
> > Oh, by the way, the tip of 'seen' has consistently failing the
> > leak-check test.  I didn't have chance, time or energy to see if
> > they are failing merely because an existing test script that used to
> > be leak-clean gained a use of command that has been known to be
> > leak-unclean without introducing any new leaks, or our recent change
> > did introduce new leaks to commands that have been leak-clean.
> > Somebody with too much time on their hand should go in and check to
> > help, before CI testing on 'seen' becomes useful again.
> 
> It's a regression in
> ps/avoid-unnecessary-hook-invocation-with-packed-refs, Patrick could you
> look into it? On your current "seen" doing a:
> 
>     git revert -m 1 48b388cbf31
> 
> Will make those 3x failing tests pass:
> https://github.com/git/git/runs/4811683950?check_suite_focus=true
> 
> (That commit being: 48b388cbf31 (Merge branch
> 'ps/avoid-unnecessary-hook-invocation-with-packed-refs' into seen,
> 2022-01-13))
> 
> I didn't have much time to look now, but this mostly untested fix-up
> fixes up the topic under SANITIZE=leak (but may break something else). I
> ran the broken tests with SANITIZE=leak, and the normal tests without
> SANITIZE=leak, but didn't have time for further testing:
> 
> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> index ff96ee482a0..b8012f97009 100644
> --- a/refs/packed-backend.c
> +++ b/refs/packed-backend.c
> @@ -1577,6 +1577,7 @@ int packed_refs_delete_refs(struct ref_store *ref_store,
>                         error(_("could not delete references: %s"), err.buf);
>         }
>  
> +       ref_transaction_free(transaction);
>         strbuf_release(&err);
>         return ret;
>  }
> 
> I.e. the moving around of the ref_transaction_free() is at fault
> somehow, probably...

Thanks for digging! The bug is actually in the files backend, where
`files_delete_refs()` has two different exit paths, but I added the free
of the packed-refs backend only to one of both. So the following patch
fixes it:

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 9a20cb8fa8..844918cbd8 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1280,6 +1280,7 @@ static int files_delete_refs(struct ref_store *ref_store, const char *msg,
 			result |= error(_("could not remove reference %s"), refname);
 	}
 
+	ref_transaction_free(transaction);
 	strbuf_release(&err);
 	return result;
 

I'll send a reroll of my series.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2022-01-17  7:18 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-14  0:48 What's cooking in git.git (Jan 2022, #03; Thu, 13) Junio C Hamano
2022-01-14  4:18 ` Junio C Hamano
2022-01-14 16:27   ` Elijah Newren
2022-01-14 19:47     ` Junio C Hamano
2022-01-14 21:49       ` Elijah Newren
2022-01-14 22:03         ` Junio C Hamano
2022-01-14 22:18           ` Junio C Hamano
2022-01-14 18:12   ` Ævar Arnfjörð Bjarmason
2022-01-17  7:18     ` Patrick Steinhardt [this message]
2022-01-14 15:44 ` Mistakes in the stalled category? (Was: Re: What's cooking in git.git (Jan 2022, #03; Thu, 13)) Elijah Newren
2022-01-14 23:32   ` Mistakes in the stalled category? Junio C Hamano
2022-01-14 23:49   ` Junio C Hamano
2022-01-15 19:38     ` Junio C Hamano
2022-01-18 16:11       ` Derrick Stolee
2022-01-14 15:54 ` en/present-despite-skipped & en/remerge-diff (Was: Re: What's cooking in git.git (Jan 2022, #03; Thu, 13)) Elijah Newren
2022-01-14 19:39 ` tb/midx-bitmap-corruption-fix (was: " Taylor Blau
2022-01-15 16:45 ` What's cooking in git.git (Jan 2022, #03; Thu, 13) David Aguilar
2022-01-15 19:15   ` Junio C Hamano
2022-01-16  2:15     ` David Aguilar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YeUYR8TwJf+31wcl@ncase \
    --to=ps@pks.im \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.