Re: [libgpiod] [PATCH] install-tests to bindir test

[Anders Roxell <anders.roxell@xxxxxxxxxx>]

On Wed, 27 Feb 2019 at 10:03, Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
>
> wt., 26 lut 2019 o 19:19 Anders Roxell <anders.roxell@xxxxxxxxxx> napisał(a):
> >
> > On Tue, 26 Feb 2019 at 16:57, Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
> > >
> > > wt., 26 lut 2019 o 16:37 Anders Roxell <anders.roxell@xxxxxxxxxx> 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@xxxxxxxxxx>
> > > > ---
> > >
> > > 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