Re: [LTP] [RFC PATCH 1/3] Makefile: Add C header with generated LTP version

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril, Li,

> On Thu, Jul 13, 2023 at 7:57 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> > Hi!
> > > obviously this is wrong, because $(top_srcdir)/Version (ltp-version.h
> > > dependency) is not specified in the top level Makefile (only Version is
> > > there). But I'm not sure if it should be changed to
> > > $(top_srcdir)/Version.

> > > I suppose ltp-version.h should be in include/

> > Not reall, as long as it's used only in the library it can stay in the
> > lib/

> > > , but here I'm completely lost with dependencies under lib/. My goal
> > > is to type make in lib/ and make sure the header is generated
> > > (dependencies correctly resolved).

> > There is another problem as well, currently the Version file is
> > generated at the end of the LTP build, that means if you do a git pull
> > and make it's not updated until the build has finished.

> > Also we will have to rebuild tst_test.c each time Version file has been
> > rewritten, which actually happens each time make is build in the top
> > level directory, which would cause useless rebuilds.

> > The best I could came up with:

> > ---
> >  lib/.gitignore     |  2 ++
> >  lib/Makefile       | 13 +++++++++++++
> >  lib/gen_version.sh | 16 ++++++++++++++++
> >  3 files changed, 31 insertions(+)
> >  create mode 100644 lib/.gitignore
> >  create mode 100755 lib/gen_version.sh

> > diff --git a/lib/.gitignore b/lib/.gitignore
> > new file mode 100644
> > index 000000000..178867a94
> > --- /dev/null
> > +++ b/lib/.gitignore
> > @@ -0,0 +1,2 @@
> > +ltp-version.h
> > +cached-version
> > diff --git a/lib/Makefile b/lib/Makefile
> > index 9b9906f25..371119ede 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -20,6 +20,19 @@ pc_file                      :=
> > $(DESTDIR)/$(datarootdir)/pkgconfig/ltp.pc

> >  INSTALL_TARGETS                := $(pc_file)

> > +tst_test.o: ltp-version.h
> > +
> > +ltp-version.h: gen_version
> > +
> > +MAKE_TARGETS+=gen_version
> > +
> > +.PHONY: gen_version
> > +gen_version:
> > +       @echo GEN ltp-version.h
> > +       @./gen_version.sh
> > +
> > +CLEAN_TARGETS+=ltp-version.h cached-version
> > +
> >  $(pc_file):
> >         test -d "$(@D)" || mkdir -p "$(@D)"
> >         install -m $(INSTALL_MODE) "$(builddir)/$(@F)" "$@"
> > diff --git a/lib/gen_version.sh b/lib/gen_version.sh
> > new file mode 100755
> > index 000000000..7ecfb9077
> > --- /dev/null
> > +++ b/lib/gen_version.sh
> > @@ -0,0 +1,16 @@
> > +#!/bin/sh
> > +
> > +touch cached-version;
> > +
> > +if git describe >/dev/null 2>&1; then
> > +       VERSION=`git describe`
> > +else
> > +       VERSION=`cat $(top_srcdir)/VERSION`
> > +fi
> > +
> > +CACHED_VERSION=`cat cached-version`
> > +
> > +if [ "$CACHED_VERSION" != "$VERSION" ]; then
> > +       echo "$VERSION" > cached-version
> > +       echo "#define LTP_VERSION \"$VERSION\"" > ltp-version.h

Cyril, thank you for your time! LGTM, I'll test it soon.

> What we are doing in those efforts is to have an available macro
> LTP_VERSION, right?
Yes.

> So why not use the script to append that one line in tst_test.h directly?
We'd have to have tst_test.h.in which would be be kind of skeleton for
tst_test.h. Otherwise tst_test.h would be constantly "modified" by this line.

> The ltp-version.h looks quite redundant and we could even put this script
> into build.sh together, IMHO.

It must be somehow make based, because not everybody uses build.sh.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp