| Message ID | 20190226153630.20803-1-anders.roxell@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [libgpiod] install-tests to bindir test | expand |
wt., 26 lut 2019 o 16:37 Anders Roxell <anders.roxell@linaro.org> napisał(a): > > When building the tests it assumes that the build artifacts is located > in the srcdir. > Rework so we install tests into the bindir, if that is done we look for > the binaries in bin dir and not in the src dir. > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > --- Hi Anders, I guess this is for kernel.ci, right? Just an FYI - once v5.1 is released, the tests will stop working as we're changing the gpio-mockup debugfs interface. I'll try to have a new release ready at that time, but I thought I'd let you know. > configure.ac | 15 +++++++++++++++ > libgpiod.pc.in | 1 + > tests/Makefile.am | 11 ++++++++++- > tests/gpiod-test.c | 10 +++++----- > 4 files changed, 31 insertions(+), 6 deletions(-) > > diff --git a/configure.ac b/configure.ac > index 49bedf4d2410..b93feab6ebcc 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -113,6 +113,21 @@ then > AC_CHECK_HEADERS([sys/signalfd.h], [], [HEADER_NOT_FOUND_TOOLS([sys/signalfd.h])]) > fi > > +AC_ARG_ENABLE([install-tests], > + [AC_HELP_STRING([--enable-install-tests], > + [enable install tests [default=no]])], > + [ > + if test "x$enableval" = xyes > + then > + with_install_tests=true > + with_tests=true > + else > + with_install_tests=false > + fi > + ], Can you put it on a single line as is done in commit 04f566d5bde8a in current master? > + [with_install_tests=false]) > +AM_CONDITIONAL([WITH_INSTALL_TESTS], [test "x$with_install_tests" = xtrue]) > + > AC_ARG_ENABLE([tests], > [AC_HELP_STRING([--enable-tests], > [enable libgpiod tests [default=no]])], > diff --git a/libgpiod.pc.in b/libgpiod.pc.in > index 48ff11392ae4..96d1111324e5 100644 > --- a/libgpiod.pc.in > +++ b/libgpiod.pc.in > @@ -1,5 +1,6 @@ > prefix=@prefix@ > exec_prefix=@exec_prefix@ > +bindir=@bindir@ > libdir=@libdir@ > includedir=@includedir@ > > diff --git a/tests/Makefile.am b/tests/Makefile.am > index a9319a725f0d..e4b14274081f 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -9,10 +9,19 @@ > AM_CFLAGS = -I$(top_srcdir)/include/ -include $(top_builddir)/config.h > AM_CFLAGS += -Wall -Wextra -g $(KMOD_CFLAGS) $(UDEV_CFLAGS) > AM_LDFLAGS = -pthread > -LDADD = ../src/lib/libgpiod.la $(KMOD_LIBS) $(UDEV_LIBS) > +LDADD = $(top_builddir)/src/lib/libgpiod.la $(KMOD_LIBS) $(UDEV_LIBS) > > check_PROGRAMS = gpiod-test > > +if WITH_INSTALL_TESTS > +bin_PROGRAMS = $(check_PROGRAMS) > +TOOLS_PATH=@prefix@/bin Let's stick to the used namespace - please call it GPIOD_TOOLS_PATH. > +else > +noinst_PROGRAMS = $(check_PROGRAMS) > +TOOLS_PATH="./../../src/tools" > +endif > +AM_CFLAGS += -DTOOLS_PATH=$(TOOLS_PATH)/ > + > gpiod_test_SOURCES = gpiod-test.c \ > gpiod-test.h \ > tests-chip.c \ > diff --git a/tests/gpiod-test.c b/tests/gpiod-test.c > index 92d6b7875fef..fda9d9d50fb5 100644 > --- a/tests/gpiod-test.c > +++ b/tests/gpiod-test.c > @@ -30,6 +30,9 @@ > #define NORETURN __attribute__((noreturn)) > #define MALLOC __attribute__((malloc)) > > +#define xstr(s) str(s) > +#define str(s) #s I'm not sure what xstr() is here for and also I actually prefer stringification without macro wrappers - it's more readable. > + > static const unsigned int min_kern_major = 4; > static const unsigned int min_kern_minor = 16; > static const unsigned int min_kern_release = 0; > @@ -449,12 +452,9 @@ static void gpiotool_proc_dup_fds(int in_fd, int out_fd, int err_fd) > > static char *gpiotool_proc_get_path(const char *tool) > { > - char *path, *progpath, *progdir; > + char *path; > > - progpath = xstrdup(program_invocation_name); > - progdir = dirname(progpath); > - path = xappend(NULL, "%s/../../src/tools/%s", progdir, tool); > - free(progpath); > + path = xappend(NULL, "%s%s", xstr(TOOLS_PATH), tool); > > return path; > } > -- > 2.20.1 > Best regards, Bartosz Golaszewski
On Tue, 26 Feb 2019 at 16:57, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > wt., 26 lut 2019 o 16:37 Anders Roxell <anders.roxell@linaro.org> napisał(a): > > > > When building the tests it assumes that the build artifacts is located > > in the srcdir. > > Rework so we install tests into the bindir, if that is done we look for > > the binaries in bin dir and not in the src dir. > > > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > > --- > > Hi Anders, > > I guess this is for kernel.ci, right? Yes. > Just an FYI - once v5.1 is > released, the tests will stop working as we're changing the > gpio-mockup debugfs interface. aha, thanks for the heads up. > I'll try to have a new release ready at > that time, but I thought I'd let you know. Thank you, can we add a switch so it will work with both the new and old interfaces some how? > > > configure.ac | 15 +++++++++++++++ > > libgpiod.pc.in | 1 + > > tests/Makefile.am | 11 ++++++++++- > > tests/gpiod-test.c | 10 +++++----- > > 4 files changed, 31 insertions(+), 6 deletions(-) > > > > diff --git a/configure.ac b/configure.ac > > index 49bedf4d2410..b93feab6ebcc 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -113,6 +113,21 @@ then > > AC_CHECK_HEADERS([sys/signalfd.h], [], [HEADER_NOT_FOUND_TOOLS([sys/signalfd.h])]) > > fi > > > > +AC_ARG_ENABLE([install-tests], > > + [AC_HELP_STRING([--enable-install-tests], > > + [enable install tests [default=no]])], > > + [ > > + if test "x$enableval" = xyes > > + then > > + with_install_tests=true > > + with_tests=true > > + else > > + with_install_tests=false > > + fi > > + ], > > Can you put it on a single line as is done in commit 04f566d5bde8a in > current master? of course I'll fix it. > > > + [with_install_tests=false]) > > +AM_CONDITIONAL([WITH_INSTALL_TESTS], [test "x$with_install_tests" = xtrue]) > > + > > AC_ARG_ENABLE([tests], > > [AC_HELP_STRING([--enable-tests], > > [enable libgpiod tests [default=no]])], > > diff --git a/libgpiod.pc.in b/libgpiod.pc.in > > index 48ff11392ae4..96d1111324e5 100644 > > --- a/libgpiod.pc.in > > +++ b/libgpiod.pc.in > > @@ -1,5 +1,6 @@ > > prefix=@prefix@ > > exec_prefix=@exec_prefix@ > > +bindir=@bindir@ > > libdir=@libdir@ > > includedir=@includedir@ > > > > diff --git a/tests/Makefile.am b/tests/Makefile.am > > index a9319a725f0d..e4b14274081f 100644 > > --- a/tests/Makefile.am > > +++ b/tests/Makefile.am > > @@ -9,10 +9,19 @@ > > AM_CFLAGS = -I$(top_srcdir)/include/ -include $(top_builddir)/config.h > > AM_CFLAGS += -Wall -Wextra -g $(KMOD_CFLAGS) $(UDEV_CFLAGS) > > AM_LDFLAGS = -pthread > > -LDADD = ../src/lib/libgpiod.la $(KMOD_LIBS) $(UDEV_LIBS) > > +LDADD = $(top_builddir)/src/lib/libgpiod.la $(KMOD_LIBS) $(UDEV_LIBS) > > > > check_PROGRAMS = gpiod-test > > > > +if WITH_INSTALL_TESTS > > +bin_PROGRAMS = $(check_PROGRAMS) > > +TOOLS_PATH=@prefix@/bin > > Let's stick to the used namespace - please call it GPIOD_TOOLS_PATH. Yes. > > > +else > > +noinst_PROGRAMS = $(check_PROGRAMS) > > +TOOLS_PATH="./../../src/tools" > > +endif > > +AM_CFLAGS += -DTOOLS_PATH=$(TOOLS_PATH)/ > > + > > gpiod_test_SOURCES = gpiod-test.c \ > > gpiod-test.h \ > > tests-chip.c \ > > diff --git a/tests/gpiod-test.c b/tests/gpiod-test.c > > index 92d6b7875fef..fda9d9d50fb5 100644 > > --- a/tests/gpiod-test.c > > +++ b/tests/gpiod-test.c > > @@ -30,6 +30,9 @@ > > #define NORETURN __attribute__((noreturn)) > > #define MALLOC __attribute__((malloc)) > > > > +#define xstr(s) str(s) > > +#define str(s) #s > > I'm not sure what xstr() is here for and also I actually prefer > stringification without macro wrappers - it's more readable. The macro's are there to redo the GPIOD_TOOLS_PATH to a string, its either that or we have to define it as a string from start via the shell, unsure how to do that, also will that be more readable ? Cheers, Anders > > > + > > static const unsigned int min_kern_major = 4; > > static const unsigned int min_kern_minor = 16; > > static const unsigned int min_kern_release = 0; > > @@ -449,12 +452,9 @@ static void gpiotool_proc_dup_fds(int in_fd, int out_fd, int err_fd) > > > > static char *gpiotool_proc_get_path(const char *tool) > > { > > - char *path, *progpath, *progdir; > > + char *path; > > > > - progpath = xstrdup(program_invocation_name); > > - progdir = dirname(progpath); > > - path = xappend(NULL, "%s/../../src/tools/%s", progdir, tool); > > - free(progpath); > > + path = xappend(NULL, "%s%s", xstr(TOOLS_PATH), tool); > > > > return path; > > } > > -- > > 2.20.1 > > > > Best regards, > Bartosz Golaszewski
wt., 26 lut 2019 o 19:19 Anders Roxell <anders.roxell@linaro.org> napisał(a): > > On Tue, 26 Feb 2019 at 16:57, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > wt., 26 lut 2019 o 16:37 Anders Roxell <anders.roxell@linaro.org> napisał(a): > > > > > > When building the tests it assumes that the build artifacts is located > > > in the srcdir. > > > Rework so we install tests into the bindir, if that is done we look for > > > the binaries in bin dir and not in the src dir. > > > > > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > > > --- > > > > Hi Anders, > > > > I guess this is for kernel.ci, right? > > Yes. > > > Just an FYI - once v5.1 is > > released, the tests will stop working as we're changing the > > gpio-mockup debugfs interface. > > aha, thanks for the heads up. > > > I'll try to have a new release ready at > > that time, but I thought I'd let you know. > > Thank you, can we add a switch so it will work with both the new and > old interfaces some how? > I'd prefer not to keep legacy code in the repo. The new interface completely changes the approach (for instance: we'll be able to verify the results of actions over debugfs, which we now cannot). Updating the kernel in lockstep with libgpiod will work though. Maybe we should wait with adding the tests until v5.1 is out? > > > > > configure.ac | 15 +++++++++++++++ > > > libgpiod.pc.in | 1 + > > > tests/Makefile.am | 11 ++++++++++- > > > tests/gpiod-test.c | 10 +++++----- > > > 4 files changed, 31 insertions(+), 6 deletions(-) > > > > > > diff --git a/configure.ac b/configure.ac > > > index 49bedf4d2410..b93feab6ebcc 100644 > > > --- a/configure.ac > > > +++ b/configure.ac > > > @@ -113,6 +113,21 @@ then > > > AC_CHECK_HEADERS([sys/signalfd.h], [], [HEADER_NOT_FOUND_TOOLS([sys/signalfd.h])]) > > > fi > > > > > > +AC_ARG_ENABLE([install-tests], > > > + [AC_HELP_STRING([--enable-install-tests], > > > + [enable install tests [default=no]])], > > > + [ > > > + if test "x$enableval" = xyes > > > + then > > > + with_install_tests=true > > > + with_tests=true > > > + else > > > + with_install_tests=false > > > + fi > > > + ], > > > > Can you put it on a single line as is done in commit 04f566d5bde8a in > > current master? > > of course I'll fix it. > > > > > > + [with_install_tests=false]) > > > +AM_CONDITIONAL([WITH_INSTALL_TESTS], [test "x$with_install_tests" = xtrue]) > > > + > > > AC_ARG_ENABLE([tests], > > > [AC_HELP_STRING([--enable-tests], > > > [enable libgpiod tests [default=no]])], > > > diff --git a/libgpiod.pc.in b/libgpiod.pc.in > > > index 48ff11392ae4..96d1111324e5 100644 > > > --- a/libgpiod.pc.in > > > +++ b/libgpiod.pc.in > > > @@ -1,5 +1,6 @@ > > > prefix=@prefix@ > > > exec_prefix=@exec_prefix@ > > > +bindir=@bindir@ > > > libdir=@libdir@ > > > includedir=@includedir@ > > > > > > diff --git a/tests/Makefile.am b/tests/Makefile.am > > > index a9319a725f0d..e4b14274081f 100644 > > > --- a/tests/Makefile.am > > > +++ b/tests/Makefile.am > > > @@ -9,10 +9,19 @@ > > > AM_CFLAGS = -I$(top_srcdir)/include/ -include $(top_builddir)/config.h > > > AM_CFLAGS += -Wall -Wextra -g $(KMOD_CFLAGS) $(UDEV_CFLAGS) > > > AM_LDFLAGS = -pthread > > > -LDADD = ../src/lib/libgpiod.la $(KMOD_LIBS) $(UDEV_LIBS) > > > +LDADD = $(top_builddir)/src/lib/libgpiod.la $(KMOD_LIBS) $(UDEV_LIBS) > > > > > > check_PROGRAMS = gpiod-test > > > > > > +if WITH_INSTALL_TESTS > > > +bin_PROGRAMS = $(check_PROGRAMS) > > > +TOOLS_PATH=@prefix@/bin > > > > Let's stick to the used namespace - please call it GPIOD_TOOLS_PATH. > > Yes. > > > > > > +else > > > +noinst_PROGRAMS = $(check_PROGRAMS) > > > +TOOLS_PATH="./../../src/tools" > > > +endif > > > +AM_CFLAGS += -DTOOLS_PATH=$(TOOLS_PATH)/ > > > + > > > gpiod_test_SOURCES = gpiod-test.c \ > > > gpiod-test.h \ > > > tests-chip.c \ > > > diff --git a/tests/gpiod-test.c b/tests/gpiod-test.c > > > index 92d6b7875fef..fda9d9d50fb5 100644 > > > --- a/tests/gpiod-test.c > > > +++ b/tests/gpiod-test.c > > > @@ -30,6 +30,9 @@ > > > #define NORETURN __attribute__((noreturn)) > > > #define MALLOC __attribute__((malloc)) > > > > > > +#define xstr(s) str(s) > > > +#define str(s) #s > > > > I'm not sure what xstr() is here for and also I actually prefer > > stringification without macro wrappers - it's more readable. > > The macro's are there to redo the GPIOD_TOOLS_PATH to a > string, its either that or we have to define it as a string from start > via the shell, unsure how to do that, also will that be more readable ? > Oh I'm seeing why you did this like this: https://gcc.gnu.org/onlinedocs/gcc-3.4.4/cpp/Stringification.html Nevermind my comment, let's leave it like it is. > Cheers, > Anders > > > > > > + > > > static const unsigned int min_kern_major = 4; > > > static const unsigned int min_kern_minor = 16; > > > static const unsigned int min_kern_release = 0; > > > @@ -449,12 +452,9 @@ static void gpiotool_proc_dup_fds(int in_fd, int out_fd, int err_fd) > > > > > > static char *gpiotool_proc_get_path(const char *tool) > > > { > > > - char *path, *progpath, *progdir; > > > + char *path; > > > > > > - progpath = xstrdup(program_invocation_name); > > > - progdir = dirname(progpath); > > > - path = xappend(NULL, "%s/../../src/tools/%s", progdir, tool); > > > - free(progpath); > > > + path = xappend(NULL, "%s%s", xstr(TOOLS_PATH), tool); > > > > > > return path; > > > } > > > -- > > > 2.20.1 > > > > > > > Best regards, > > Bartosz Golaszewski
On Wed, 27 Feb 2019 at 10:03, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > wt., 26 lut 2019 o 19:19 Anders Roxell <anders.roxell@linaro.org> napisał(a): > > > > On Tue, 26 Feb 2019 at 16:57, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > wt., 26 lut 2019 o 16:37 Anders Roxell <anders.roxell@linaro.org> napisał(a): > > > > > > > > When building the tests it assumes that the build artifacts is located > > > > in the srcdir. > > > > Rework so we install tests into the bindir, if that is done we look for > > > > the binaries in bin dir and not in the src dir. > > > > > > > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > > > > --- > > > > > > Hi Anders, > > > > > > I guess this is for kernel.ci, right? > > > > Yes. > > > > > Just an FYI - once v5.1 is > > > released, the tests will stop working as we're changing the > > > gpio-mockup debugfs interface. > > > > aha, thanks for the heads up. > > > > > I'll try to have a new release ready at > > > that time, but I thought I'd let you know. > > > > Thank you, can we add a switch so it will work with both the new and > > old interfaces some how? > > > > I'd prefer not to keep legacy code in the repo. I understand. However, would it be possible to do a tag and possibly a maintenance release on pre-v5.1 libgpiod debugfs ? So we can build that tag and test LTS kernels pre-v5.1. Does that make sense to you? > The new interface > completely changes the approach (for instance: we'll be able to verify > the results of actions over debugfs, which we now cannot). Updating > the kernel in lockstep with libgpiod will work though. Maybe we should > wait with adding the tests until v5.1 is out? I would prefer to add my patch before, for the reason above. > > > > > > > > configure.ac | 15 +++++++++++++++ > > > > libgpiod.pc.in | 1 + > > > > tests/Makefile.am | 11 ++++++++++- > > > > tests/gpiod-test.c | 10 +++++----- > > > > 4 files changed, 31 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/configure.ac b/configure.ac > > > > index 49bedf4d2410..b93feab6ebcc 100644 > > > > --- a/configure.ac > > > > +++ b/configure.ac > > > > @@ -113,6 +113,21 @@ then > > > > AC_CHECK_HEADERS([sys/signalfd.h], [], [HEADER_NOT_FOUND_TOOLS([sys/signalfd.h])]) > > > > fi > > > > > > > > +AC_ARG_ENABLE([install-tests], > > > > + [AC_HELP_STRING([--enable-install-tests], > > > > + [enable install tests [default=no]])], > > > > + [ > > > > + if test "x$enableval" = xyes > > > > + then > > > > + with_install_tests=true > > > > + with_tests=true > > > > + else > > > > + with_install_tests=false > > > > + fi > > > > + ], > > > > > > Can you put it on a single line as is done in commit 04f566d5bde8a in > > > current master? > > > > of course I'll fix it. > > > > > > > > > + [with_install_tests=false]) > > > > +AM_CONDITIONAL([WITH_INSTALL_TESTS], [test "x$with_install_tests" = xtrue]) > > > > + > > > > AC_ARG_ENABLE([tests], > > > > [AC_HELP_STRING([--enable-tests], > > > > [enable libgpiod tests [default=no]])], > > > > diff --git a/libgpiod.pc.in b/libgpiod.pc.in > > > > index 48ff11392ae4..96d1111324e5 100644 > > > > --- a/libgpiod.pc.in > > > > +++ b/libgpiod.pc.in > > > > @@ -1,5 +1,6 @@ > > > > prefix=@prefix@ > > > > exec_prefix=@exec_prefix@ > > > > +bindir=@bindir@ > > > > libdir=@libdir@ > > > > includedir=@includedir@ > > > > > > > > diff --git a/tests/Makefile.am b/tests/Makefile.am > > > > index a9319a725f0d..e4b14274081f 100644 > > > > --- a/tests/Makefile.am > > > > +++ b/tests/Makefile.am > > > > @@ -9,10 +9,19 @@ > > > > AM_CFLAGS = -I$(top_srcdir)/include/ -include $(top_builddir)/config.h > > > > AM_CFLAGS += -Wall -Wextra -g $(KMOD_CFLAGS) $(UDEV_CFLAGS) > > > > AM_LDFLAGS = -pthread > > > > -LDADD = ../src/lib/libgpiod.la $(KMOD_LIBS) $(UDEV_LIBS) > > > > +LDADD = $(top_builddir)/src/lib/libgpiod.la $(KMOD_LIBS) $(UDEV_LIBS) > > > > > > > > check_PROGRAMS = gpiod-test > > > > > > > > +if WITH_INSTALL_TESTS > > > > +bin_PROGRAMS = $(check_PROGRAMS) > > > > +TOOLS_PATH=@prefix@/bin > > > > > > Let's stick to the used namespace - please call it GPIOD_TOOLS_PATH. > > > > Yes. > > > > > > > > > +else > > > > +noinst_PROGRAMS = $(check_PROGRAMS) > > > > +TOOLS_PATH="./../../src/tools" > > > > +endif > > > > +AM_CFLAGS += -DTOOLS_PATH=$(TOOLS_PATH)/ > > > > + > > > > gpiod_test_SOURCES = gpiod-test.c \ > > > > gpiod-test.h \ > > > > tests-chip.c \ > > > > diff --git a/tests/gpiod-test.c b/tests/gpiod-test.c > > > > index 92d6b7875fef..fda9d9d50fb5 100644 > > > > --- a/tests/gpiod-test.c > > > > +++ b/tests/gpiod-test.c > > > > @@ -30,6 +30,9 @@ > > > > #define NORETURN __attribute__((noreturn)) > > > > #define MALLOC __attribute__((malloc)) > > > > > > > > +#define xstr(s) str(s) > > > > +#define str(s) #s > > > > > > I'm not sure what xstr() is here for and also I actually prefer > > > stringification without macro wrappers - it's more readable. > > > > The macro's are there to redo the GPIOD_TOOLS_PATH to a > > string, its either that or we have to define it as a string from start > > via the shell, unsure how to do that, also will that be more readable ? > > > > Oh I'm seeing why you did this like this: > https://gcc.gnu.org/onlinedocs/gcc-3.4.4/cpp/Stringification.html > > Nevermind my comment, let's leave it like it is. OK. thanks for adding the link I forgot that. Cheers, Anders
śr., 27 lut 2019 o 15:30 Anders Roxell <anders.roxell@linaro.org> napisał(a): > > On Wed, 27 Feb 2019 at 10:03, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > wt., 26 lut 2019 o 19:19 Anders Roxell <anders.roxell@linaro.org> napisał(a): > > > > > > On Tue, 26 Feb 2019 at 16:57, Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > > > wt., 26 lut 2019 o 16:37 Anders Roxell <anders.roxell@linaro.org> napisał(a): > > > > > > > > > > When building the tests it assumes that the build artifacts is located > > > > > in the srcdir. > > > > > Rework so we install tests into the bindir, if that is done we look for > > > > > the binaries in bin dir and not in the src dir. > > > > > > > > > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > > > > > --- > > > > > > > > Hi Anders, > > > > > > > > I guess this is for kernel.ci, right? > > > > > > Yes. > > > > > > > Just an FYI - once v5.1 is > > > > released, the tests will stop working as we're changing the > > > > gpio-mockup debugfs interface. > > > > > > aha, thanks for the heads up. > > > > > > > I'll try to have a new release ready at > > > > that time, but I thought I'd let you know. > > > > > > Thank you, can we add a switch so it will work with both the new and > > > old interfaces some how? > > > > > > > I'd prefer not to keep legacy code in the repo. > > I understand. However, would it be possible to do a tag and possibly a > maintenance release on pre-v5.1 libgpiod debugfs ? > So we can build that tag and test LTS kernels pre-v5.1. > > Does that make sense to you? > Yes. I have a couple patches queued already, so I'll release v1.3 in, let's say, two weeks and then I'll simply maintain the v1.3.x branch with previous version of tests that'll cover kernels v4.16..v5.0. > > The new interface > > completely changes the approach (for instance: we'll be able to verify > > the results of actions over debugfs, which we now cannot). Updating > > the kernel in lockstep with libgpiod will work though. Maybe we should > > wait with adding the tests until v5.1 is out? > > I would prefer to add my patch before, for the reason above. > Sure, I'll wait for v2. Bart > > > > > > > > > > > configure.ac | 15 +++++++++++++++ > > > > > libgpiod.pc.in | 1 + > > > > > tests/Makefile.am | 11 ++++++++++- > > > > > tests/gpiod-test.c | 10 +++++----- > > > > > 4 files changed, 31 insertions(+), 6 deletions(-) > > > > > > > > > > diff --git a/configure.ac b/configure.ac > > > > > index 49bedf4d2410..b93feab6ebcc 100644 > > > > > --- a/configure.ac > > > > > +++ b/configure.ac > > > > > @@ -113,6 +113,21 @@ then > > > > > AC_CHECK_HEADERS([sys/signalfd.h], [], [HEADER_NOT_FOUND_TOOLS([sys/signalfd.h])]) > > > > > fi > > > > > > > > > > +AC_ARG_ENABLE([install-tests], > > > > > + [AC_HELP_STRING([--enable-install-tests], > > > > > + [enable install tests [default=no]])], > > > > > + [ > > > > > + if test "x$enableval" = xyes > > > > > + then > > > > > + with_install_tests=true > > > > > + with_tests=true > > > > > + else > > > > > + with_install_tests=false > > > > > + fi > > > > > + ], > > > > > > > > Can you put it on a single line as is done in commit 04f566d5bde8a in > > > > current master? > > > > > > of course I'll fix it. > > > > > > > > > > > > + [with_install_tests=false]) > > > > > +AM_CONDITIONAL([WITH_INSTALL_TESTS], [test "x$with_install_tests" = xtrue]) > > > > > + > > > > > AC_ARG_ENABLE([tests], > > > > > [AC_HELP_STRING([--enable-tests], > > > > > [enable libgpiod tests [default=no]])], > > > > > diff --git a/libgpiod.pc.in b/libgpiod.pc.in > > > > > index 48ff11392ae4..96d1111324e5 100644 > > > > > --- a/libgpiod.pc.in > > > > > +++ b/libgpiod.pc.in > > > > > @@ -1,5 +1,6 @@ > > > > > prefix=@prefix@ > > > > > exec_prefix=@exec_prefix@ > > > > > +bindir=@bindir@ > > > > > libdir=@libdir@ > > > > > includedir=@includedir@ > > > > > > > > > > diff --git a/tests/Makefile.am b/tests/Makefile.am > > > > > index a9319a725f0d..e4b14274081f 100644 > > > > > --- a/tests/Makefile.am > > > > > +++ b/tests/Makefile.am > > > > > @@ -9,10 +9,19 @@ > > > > > AM_CFLAGS = -I$(top_srcdir)/include/ -include $(top_builddir)/config.h > > > > > AM_CFLAGS += -Wall -Wextra -g $(KMOD_CFLAGS) $(UDEV_CFLAGS) > > > > > AM_LDFLAGS = -pthread > > > > > -LDADD = ../src/lib/libgpiod.la $(KMOD_LIBS) $(UDEV_LIBS) > > > > > +LDADD = $(top_builddir)/src/lib/libgpiod.la $(KMOD_LIBS) $(UDEV_LIBS) > > > > > > > > > > check_PROGRAMS = gpiod-test > > > > > > > > > > +if WITH_INSTALL_TESTS > > > > > +bin_PROGRAMS = $(check_PROGRAMS) > > > > > +TOOLS_PATH=@prefix@/bin > > > > > > > > Let's stick to the used namespace - please call it GPIOD_TOOLS_PATH. > > > > > > Yes. > > > > > > > > > > > > +else > > > > > +noinst_PROGRAMS = $(check_PROGRAMS) > > > > > +TOOLS_PATH="./../../src/tools" > > > > > +endif > > > > > +AM_CFLAGS += -DTOOLS_PATH=$(TOOLS_PATH)/ > > > > > + > > > > > gpiod_test_SOURCES = gpiod-test.c \ > > > > > gpiod-test.h \ > > > > > tests-chip.c \ > > > > > diff --git a/tests/gpiod-test.c b/tests/gpiod-test.c > > > > > index 92d6b7875fef..fda9d9d50fb5 100644 > > > > > --- a/tests/gpiod-test.c > > > > > +++ b/tests/gpiod-test.c > > > > > @@ -30,6 +30,9 @@ > > > > > #define NORETURN __attribute__((noreturn)) > > > > > #define MALLOC __attribute__((malloc)) > > > > > > > > > > +#define xstr(s) str(s) > > > > > +#define str(s) #s > > > > > > > > I'm not sure what xstr() is here for and also I actually prefer > > > > stringification without macro wrappers - it's more readable. > > > > > > The macro's are there to redo the GPIOD_TOOLS_PATH to a > > > string, its either that or we have to define it as a string from start > > > via the shell, unsure how to do that, also will that be more readable ? > > > > > > > Oh I'm seeing why you did this like this: > > https://gcc.gnu.org/onlinedocs/gcc-3.4.4/cpp/Stringification.html > > > > Nevermind my comment, let's leave it like it is. > > OK. thanks for adding the link I forgot that. > > Cheers, > Anders
diff --git a/configure.ac b/configure.ac index 49bedf4d2410..b93feab6ebcc 100644 --- a/configure.ac +++ b/configure.ac @@ -113,6 +113,21 @@ then AC_CHECK_HEADERS([sys/signalfd.h], [], [HEADER_NOT_FOUND_TOOLS([sys/signalfd.h])]) fi +AC_ARG_ENABLE([install-tests], + [AC_HELP_STRING([--enable-install-tests], + [enable install tests [default=no]])], + [ + if test "x$enableval" = xyes + then + with_install_tests=true + with_tests=true + else + with_install_tests=false + fi + ], + [with_install_tests=false]) +AM_CONDITIONAL([WITH_INSTALL_TESTS], [test "x$with_install_tests" = xtrue]) + AC_ARG_ENABLE([tests], [AC_HELP_STRING([--enable-tests], [enable libgpiod tests [default=no]])], diff --git a/libgpiod.pc.in b/libgpiod.pc.in index 48ff11392ae4..96d1111324e5 100644 --- a/libgpiod.pc.in +++ b/libgpiod.pc.in @@ -1,5 +1,6 @@ prefix=@prefix@ exec_prefix=@exec_prefix@ +bindir=@bindir@ libdir=@libdir@ includedir=@includedir@ diff --git a/tests/Makefile.am b/tests/Makefile.am index a9319a725f0d..e4b14274081f 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -9,10 +9,19 @@ AM_CFLAGS = -I$(top_srcdir)/include/ -include $(top_builddir)/config.h AM_CFLAGS += -Wall -Wextra -g $(KMOD_CFLAGS) $(UDEV_CFLAGS) AM_LDFLAGS = -pthread -LDADD = ../src/lib/libgpiod.la $(KMOD_LIBS) $(UDEV_LIBS) +LDADD = $(top_builddir)/src/lib/libgpiod.la $(KMOD_LIBS) $(UDEV_LIBS) check_PROGRAMS = gpiod-test +if WITH_INSTALL_TESTS +bin_PROGRAMS = $(check_PROGRAMS) +TOOLS_PATH=@prefix@/bin +else +noinst_PROGRAMS = $(check_PROGRAMS) +TOOLS_PATH="./../../src/tools" +endif +AM_CFLAGS += -DTOOLS_PATH=$(TOOLS_PATH)/ + gpiod_test_SOURCES = gpiod-test.c \ gpiod-test.h \ tests-chip.c \ diff --git a/tests/gpiod-test.c b/tests/gpiod-test.c index 92d6b7875fef..fda9d9d50fb5 100644 --- a/tests/gpiod-test.c +++ b/tests/gpiod-test.c @@ -30,6 +30,9 @@ #define NORETURN __attribute__((noreturn)) #define MALLOC __attribute__((malloc)) +#define xstr(s) str(s) +#define str(s) #s + static const unsigned int min_kern_major = 4; static const unsigned int min_kern_minor = 16; static const unsigned int min_kern_release = 0; @@ -449,12 +452,9 @@ static void gpiotool_proc_dup_fds(int in_fd, int out_fd, int err_fd) static char *gpiotool_proc_get_path(const char *tool) { - char *path, *progpath, *progdir; + char *path; - progpath = xstrdup(program_invocation_name); - progdir = dirname(progpath); - path = xappend(NULL, "%s/../../src/tools/%s", progdir, tool); - free(progpath); + path = xappend(NULL, "%s%s", xstr(TOOLS_PATH), tool); return path; }
When building the tests it assumes that the build artifacts is located in the srcdir. Rework so we install tests into the bindir, if that is done we look for the binaries in bin dir and not in the src dir. Signed-off-by: Anders Roxell <anders.roxell@linaro.org> --- configure.ac | 15 +++++++++++++++ libgpiod.pc.in | 1 + tests/Makefile.am | 11 ++++++++++- tests/gpiod-test.c | 10 +++++----- 4 files changed, 31 insertions(+), 6 deletions(-)