Re: [PATCH v2 10/10] t/unit-tests: adapt lib-reftable{c,h} helper functions to clar

From: Seyi Chamber <kuforiji98@gmail.com>
Date: May 26, 2025, 09:04 UTC
Message-ID: <CAGedMteAkHj0xc2iq7T+CWa=uJWWg0me8hD6x=Nv8PHPtO1P_w@mail.gmail.com>
In-Reply-To: <aBiKUGn7avRUcehV@pks.im>

On Mon, 5 May 2025 at 10:52, Patrick Steinhardt <ps@pks.im> wrote:

Show 39 quoted lines

>
> On Mon, May 05, 2025 at 08:27:16AM +0100, Seyi Chamber wrote:
> > On Fri, 2 May 2025 at 10:58, Patrick Steinhardt <ps@pks.im> wrote:
> > > On Tue, Apr 29, 2025 at 06:53:02PM +0100, Seyi Kuforiji wrote:
> > > > diff --git a/t/unit-tests/lib-reftable.h b/t/unit-tests/lib-reftable.h
> > > > index e4c360fa7e..2958db5dc0 100644
> > > > --- a/t/unit-tests/lib-reftable.h
> > > > +++ b/t/unit-tests/lib-reftable.h
> > > > @@ -6,12 +6,12 @@
> > > >
> > > >  struct reftable_buf;
> > > >
> > > > -void t_reftable_set_hash(uint8_t *p, int i, enum reftable_hash id);
> > > > +void cl_reftable_set_hash(uint8_t *p, int i, enum reftable_hash id);
> > > >
> > > > -struct reftable_writer *t_reftable_strbuf_writer(struct reftable_buf *buf,
> > > > +struct reftable_writer *cl_reftable_strbuf_writer(struct reftable_buf *buf,
> > > >                                                struct reftable_write_options *opts);
> > > >
> > > > -void t_reftable_write_to_buf(struct reftable_buf *buf,
> > > > +void cl_reftable_write_to_buf(struct reftable_buf *buf,
> > > >                            struct reftable_ref_record *refs,
> > > >                            size_t nrecords,
> > > >                            struct reftable_log_record *logs,
> > >
> > > It is quite weird that we declare the replacement functions in
> > > "unit-test.h" in the first commit only to remove them at a later point.
> > > It would make way more sense if we introduced the functions in
> > > "t/unit/lib-reftable.{c,h}" right from the start and then only remove
> > > the unused functions in the last step.
> > >
> > > Patrick
> >
> > If I get it correctly, you're suggesting I have both the original
> > functions and the clar-based variant in `t/unit/lib-reftable.{c,h}`
>
> Yup, exactly.
>
> Patrick

Hi Patrick,

Thanks for the suggestion to move both the original t-helpers and the Clar-based cl_ versions into `t/unit/lib-reftable.{c,h}`.

I’ve tried doing that but ran into some build issues. The Clar-based functions use clar_assert and clar_assert_equal, which aren’t available to non-Clar tests. Since both sets of helpers would live in the same object file, this causes linker errors for binaries that don’t link against Clar.

I also hit Makefile warnings like “target 'lib-reftable.o' given more than once,” due to the object being included in both Clar and non-Clar test builds. And with all declarations in one header, non-Clar tests see prototypes for Clar-only functions they can't link.

Also facing errors like these: `LINK t/unit-tests/bin/t-reftable-basics /usr/bin/ld: t/unit-tests/lib-reftable.o: in function `cl_reftable_strbuf_writer': /home/seyik/git/t/unit-tests/lib-reftable.c:113:(.text+0x520): undefined reference to `clar__assert'`

What would you recommend I do to fix this?

Thanks, Seyi