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

[Seyi Chamber <kuforiji98@gmail.com>]

On Mon, 5 May 2025 at 10:52, Patrick Steinhardt <ps@pks.im> wrote:
>
> 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