Re: [libgpiod][PATCH v2 2/2] tests: look for gpio-tools in more places

[Bartosz Golaszewski <brgl@xxxxxxxx>]

śr., 20 mar 2019 o 02:25 Anders Roxell <anders.roxell@xxxxxxxxxx> napisał(a):
>
> On 2019-03-18 17:10, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
> >
> > Currently we only support running the test cases for gpio-tools from
> > the top-level source directory. Some users want to install the test
> > executable and run the tests from other locations (/bin, /usr/bin or
> > custom path).
> >
> > This patch makes the test suite look in the source tree path first,
> > then check the directory in which the program resides and last iterate
> > over all directories in $PATH.
> >
> > We only do that once at the beginning and then reuse the path later.
> >
> > Cc: Anders Roxell <anders.roxell@xxxxxxxxxx>
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
> > ---
> >  tests/gpiod-test.c | 75 ++++++++++++++++++++++++++++++++++++++--------
> >  1 file changed, 62 insertions(+), 13 deletions(-)
> >
> > diff --git a/tests/gpiod-test.c b/tests/gpiod-test.c
> > index 4c51f4a..9cb9072 100644
> > --- a/tests/gpiod-test.c
> > +++ b/tests/gpiod-test.c
> > @@ -86,6 +86,7 @@ static struct {
> >       pid_t main_pid;
> >       int pipesize;
> >       char *pipebuf;
> > +     char *toolpath;
> >  } globals;
> >
> >  enum {
> > @@ -447,18 +448,6 @@ 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;
> > -
> > -     progpath = xstrdup(program_invocation_name);
> > -     progdir = dirname(progpath);
> > -     path = xappend(NULL, "%s/../../tools/%s", progdir, tool);
> > -     free(progpath);
> > -
> > -     return path;
> > -}
> > -
> >  static NORETURN void gpiotool_proc_exec(const char *path, va_list va)
> >  {
> >       size_t num_args;
> > @@ -545,10 +534,12 @@ void test_tool_run(char *tool, ...)
> >       event_lock();
> >       if (globals.test_ctx.event.running)
> >               die("refusing to fork when the event thread is running");
> > +     if (!globals.toolpath)
> > +             die("asked to run tests for gpio-tools, but the executables were not found");
> >
> >       gpiotool_proc_make_pipes(in_fds, out_fds, err_fds);
> > -     path = gpiotool_proc_get_path(tool);
> >
> > +     path = xappend(NULL, "%s/%s", globals.toolpath, tool);
> >       rv = access(path, R_OK | X_OK);
> >       if (rv)
> >               die_perr("unable to execute '%s'", path);
> > @@ -768,6 +759,63 @@ static void check_gpio_mockup(void)
> >       msg("gpio-mockup ok");
> >  }
> >
> > +static void check_tool_path(void)
> > +{
> > +     /*
> > +      * Let's check gpiodetect only and assume all the other tools are in
> > +      * the same directory.
> > +      */
> > +     static const char *const tool = "gpiodetect";
> > +
> > +     char *progpath, *progdir, *toolpath, *pathenv, *tok;
> > +
> > +     /* First check if we're running the from the top source directory. */
>
> Nit: Remove the first "the".
>
> > +     progpath = xstrdup(program_invocation_name);
> > +     progdir = dirname(progpath);
> > +
> > +     toolpath = xappend(NULL, "%s/../../tools/%s", progdir, tool);
>
> Shouldn't this be "%s/../tools/%s" instead now when the directory
> structure changed ?
>

The actual program is run from ${top_srcdir}/tests/.libs, so it's correct.

> I retested it with these two patches and it still works.
>
> Feel free to add my
> Reviewed-and-Tested-by: Anders Roxell <anders.roxell@xxxxxxxxxx>
>

Thanks, I'll do a new release soon (before reworking the tests for v5.1).

Bart

> Cheers,
> Anders
>
> > +     if (access(toolpath, R_OK | X_OK) == 0) {
> > +             free(progpath);
> > +             goto out;
> > +     }
> > +     free(toolpath);
> > +
> > +     /* Is the tool in the same directory maybe? */
> > +     toolpath = xappend(NULL, "%s/%s", progdir, tool);
> > +     free(progpath);
> > +     if (access(toolpath, R_OK | X_OK) == 0)
> > +             goto out;
> > +     free(toolpath);
> > +
> > +     /* Next iterate over directories in $PATH. */
> > +     pathenv = getenv("PATH");
> > +     if (!pathenv)
> > +             return;
> > +
> > +     progpath = xstrdup(pathenv);
> > +     tok = strtok(progpath, ":");
> > +     while (tok) {
> > +             toolpath = xappend(NULL, "%s/%s", tok, tool);
> > +             if (access(toolpath, R_OK) == 0) {
> > +                     free(progpath);
> > +                     goto out;
> > +             }
> > +
> > +             free(toolpath);
> > +             tok = strtok(NULL, ":");
> > +     }
> > +
> > +     free(progpath);
> > +     toolpath = NULL;
> > +
> > +out:
> > +     if (toolpath) {
> > +             toolpath = dirname(toolpath);
> > +             msg("using gpio-tools from '%s'", toolpath);
> > +             globals.toolpath = toolpath;
> > +     }
> > +}
> > +
> >  static void load_module(struct _test_chip_descr *descr)
> >  {
> >       unsigned int i;
> > @@ -1001,6 +1049,7 @@ int main(int argc TEST_UNUSED, char **argv TEST_UNUSED)
> >
> >       check_kernel();
> >       check_gpio_mockup();
> > +     check_tool_path();
> >
> >       msg("running tests");
> >
> > --
> > 2.20.1
> >