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

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

[Petr Vorel]

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

Petr Vorel (3):
  Makefile: Add C header with generated LTP version
  lib/C-API: Add option -V to print LTP version
  lib/C-API: Print LTP version at test start

 .gitignore     | 1 +
 lib/Makefile   | 4 ++++
 lib/tst_test.c | 8 ++++++++
 3 files changed, 13 insertions(+)

-- 
2.40.1


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

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

[Petr Vorel]

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(+)

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
-- 
2.40.1


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

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

[Petr Vorel]

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

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

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

[Cyril Hrubis]

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

-- 
Cyril Hrubis
chrubis@suse.cz

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

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

[Li Wang]

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.


-- 
Regards,
Li Wang

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

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

[Petr Vorel]

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

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

[Cyril Hrubis]

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

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

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

[Li Wang]

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
>
>

-- 
Regards,
Li Wang

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

[LTP] [RFC PATCH 2/3] lib/C-API: Add option -V to print LTP version

[Petr Vorel]

It can be useful for troubleshooting reported issues.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 lib/tst_test.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/tst_test.c b/lib/tst_test.c
index 04da456c6..e81a3d036 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -34,6 +34,7 @@
 #include "old_resource.h"
 #include "old_device.h"
 #include "old_tmpdir.h"
+#include "ltp-version.h"
 
 /*
  * Hack to get TCID defined in newlib tests
@@ -509,6 +510,7 @@ static struct option {
 	{"h",  "-h       Prints this help"},
 	{"i:", "-i n     Execute test n times"},
 	{"I:", "-I x     Execute test for n seconds"},
+	{"V",  "-V       Prints LTP version"},
 	{"C:", "-C ARG   Run child process with ARG arguments (used internally)"},
 };
 
@@ -686,6 +688,10 @@ static void parse_opts(int argc, char *argv[])
 			else
 				duration = SAFE_STRTOF(optarg, 0.1, HUGE_VALF);
 		break;
+		case 'V':
+			fprintf(stderr, LTP_VERSION "\n");
+			exit(0);
+		break;
 		case 'C':
 #ifdef UCLINUX
 			child_args = optarg;
-- 
2.40.1


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

Re: [LTP] [RFC PATCH 2/3] lib/C-API: Add option -V to print LTP version

[Cyril Hrubis]

Hi!
> It can be useful for troubleshooting reported issues.
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>  lib/tst_test.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 04da456c6..e81a3d036 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -34,6 +34,7 @@
>  #include "old_resource.h"
>  #include "old_device.h"
>  #include "old_tmpdir.h"
> +#include "ltp-version.h"
>  
>  /*
>   * Hack to get TCID defined in newlib tests
> @@ -509,6 +510,7 @@ static struct option {
>  	{"h",  "-h       Prints this help"},
>  	{"i:", "-i n     Execute test n times"},
>  	{"I:", "-I x     Execute test for n seconds"},
> +	{"V",  "-V       Prints LTP version"},
>  	{"C:", "-C ARG   Run child process with ARG arguments (used internally)"},
>  };
>  
> @@ -686,6 +688,10 @@ static void parse_opts(int argc, char *argv[])
>  			else
>  				duration = SAFE_STRTOF(optarg, 0.1, HUGE_VALF);
>  		break;
> +		case 'V':
> +			fprintf(stderr, LTP_VERSION "\n");
> +			exit(0);
> +		break;

Why don't we print that as a part of help? Do we really need to allocate
a flag for this?

>  		case 'C':
>  #ifdef UCLINUX
>  			child_args = optarg;
> -- 
> 2.40.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [RFC PATCH 2/3] lib/C-API: Add option -V to print LTP version

[Petr Vorel]

Hi Cyril,

...
> > @@ -686,6 +688,10 @@ static void parse_opts(int argc, char *argv[])
> >  			else
> >  				duration = SAFE_STRTOF(optarg, 0.1, HUGE_VALF);
> >  		break;
> > +		case 'V':
> > +			fprintf(stderr, LTP_VERSION "\n");
> > +			exit(0);
> > +		break;

> Why don't we print that as a part of help? Do we really need to allocate
> a flag for this?

Well, people expect -V or --verbose. But because I add printing also on running
test, we could just move it to -h.

BTW I was also thinking about using getopt_long(), at least to allow --help,
I guess you don't think any value in it.

Kind regards,
Petr

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

Re: [LTP] [RFC PATCH 2/3] lib/C-API: Add option -V to print LTP version

[Cyril Hrubis]

Hi!
> > Why don't we print that as a part of help? Do we really need to allocate
> > a flag for this?
> 
> Well, people expect -V or --verbose. But because I add printing also on running
> test, we could just move it to -h.

You mean --version right?

> BTW I was also thinking about using getopt_long(), at least to allow --help,
> I guess you don't think any value in it.

I do not think that it's importan enough to spend time on, but feel free
to send a patch.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [RFC PATCH 2/3] lib/C-API: Add option -V to print LTP version

[Petr Vorel]

> Hi!
> > > Why don't we print that as a part of help? Do we really need to allocate
> > > a flag for this?

> > Well, people expect -V or --verbose. But because I add printing also on running
> > test, we could just move it to -h.

> You mean --version right?

Yes, I'm sorry for a confusion.

> > BTW I was also thinking about using getopt_long(), at least to allow --help,
> > I guess you don't think any value in it.

> I do not think that it's importan enough to spend time on, but feel free
> to send a patch.

Agree. A real reason would be if netstress.c would need to have some more
options (there are still some room left, but not many).

Kind regards,
Petr

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

[LTP] [RFC PATCH 3/3] lib/C-API: Print LTP version at test start

[Petr Vorel]

Although -V option for printing version was added in previous commit,
having a way to always print LTP version at the beginning of the test
(makes debugging of troubleshooting reported issues even easier).

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 lib/tst_test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/tst_test.c b/lib/tst_test.c
index e81a3d036..c93ef6aac 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -1677,6 +1677,8 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
 	SAFE_SIGNAL(SIGALRM, alarm_handler);
 	SAFE_SIGNAL(SIGUSR1, heartbeat_handler);
 
+	tst_res(TINFO, LTP_VERSION);
+
 	if (tst_test->max_runtime)
 		results->max_runtime = multiply_runtime(tst_test->max_runtime);
 
-- 
2.40.1


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

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

[Petr Vorel]

Hi,

> 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

> Petr Vorel (3):
>   Makefile: Add C header with generated LTP version
>   lib/C-API: Add option -V to print LTP version
>   lib/C-API: Print LTP version at test start

>  .gitignore     | 1 +
>  lib/Makefile   | 4 ++++
>  lib/tst_test.c | 8 ++++++++
>  3 files changed, 13 insertions(+)

please ignore this patch. Subject was supposed to be
"[RFC PATCH 0/3] C API: print LTP version"

Kind regards,
Petr

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