| Message ID | 20230704091933.496989-2-pvorel@suse.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [RFC,1/3] Makefile: Add C header with generated LTP version | expand |
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/, 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). Kind regards, Petr
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 +fi
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 > What we are doing in those efforts is to have an available macro LTP_VERSION, right? So why not use the script to append that one line in tst_test.h directly? The ltp-version.h looks quite redundant and we could even put this script into build.sh together, IMHO.
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
Hi! > So why not use the script to append that one line in tst_test.h directly? That wouldn't play nice with development, you would have to make sure not to accidentally commit the line, which would happen way to often. > The ltp-version.h looks quite redundant and we could even put this script > into build.sh together, IMHO. The end goal here is to have the LTP version in the test output regardless where it came from (tarball, git) and how it was compiled (build.sh, cd testcases/kernel/ && make, ...). The only way how to do this is to refresh the version on each make and build it from inside the library.
Ah, I see, thank you both. On Fri, Jul 14, 2023 at 2:45 PM Cyril Hrubis <chrubis@suse.cz> wrote: > Hi! > > So why not use the script to append that one line in tst_test.h directly? > > That wouldn't play nice with development, you would have to make sure > not to accidentally commit the line, which would happen way to often. > > > The ltp-version.h looks quite redundant and we could even put this script > > into build.sh together, IMHO. > > The end goal here is to have the LTP version in the test output > regardless where it came from (tarball, git) and how it was compiled > (build.sh, cd testcases/kernel/ && make, ...). The only way how to do > this is to refresh the version on each make and build it from inside the > library. > > -- > Cyril Hrubis > chrubis@suse.cz > >
diff --git a/.gitignore b/.gitignore index 915d22104..49e42bb9e 100644 --- a/.gitignore +++ b/.gitignore @@ -41,6 +41,7 @@ autom4te.cache /include/mk/config-openposix.mk /include/mk/features.mk /m4/ltp-version.m4 +/lib/ltp-version.h /lib/ltp.pc /pan/ltp-bump /pan/ltp-pan diff --git a/lib/Makefile b/lib/Makefile index 9b9906f25..1cac43cde 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -24,5 +24,9 @@ $(pc_file): test -d "$(@D)" || mkdir -p "$(@D)" install -m $(INSTALL_MODE) "$(builddir)/$(@F)" "$@" +.PHONY: ltp-version.h +ltp-version.h: $(top_srcdir)/Version + echo "#define LTP_VERSION \"LTP version: $$(cat $(top_srcdir)/Version)\"" > "$(top_builddir)/lib/$(@F)" + include $(top_srcdir)/include/mk/lib.mk include $(top_srcdir)/include/mk/generic_trunk_target.mk
It will be used for printing LTP version in C API. Signed-off-by: Petr Vorel <pvorel@suse.cz> --- .gitignore | 1 + lib/Makefile | 4 ++++ 2 files changed, 5 insertions(+)