diff mbox series

[libgpiod] install-tests to bindir test

Message ID 20190226153630.20803-1-anders.roxell@linaro.org
State Superseded
Headers show
Series [libgpiod] install-tests to bindir test | expand

Commit Message

Anders Roxell Feb. 26, 2019, 3:36 p.m. UTC
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(-)

-- 
2.20.1

Comments

Bartosz Golaszewski Feb. 26, 2019, 3:57 p.m. UTC | #1
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
Anders Roxell Feb. 26, 2019, 6:18 p.m. UTC | #2
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
Bartosz Golaszewski Feb. 27, 2019, 9:03 a.m. UTC | #3
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
Anders Roxell Feb. 27, 2019, 2:29 p.m. UTC | #4
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
Bartosz Golaszewski Feb. 27, 2019, 3:32 p.m. UTC | #5
ś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 mbox series

Patch

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