[libvirt PATCH 008/351] meson: src/util/virfile: rewrite virFileActivateDirOverrideForProg

[Peter Krempa]

On Thu, Jul 16, 2020 at 11:54:04 +0200, Pavel Hrdina wrote:
> With meson we no longer have .libs directory with the actual binary so
> we have to take a different approach to detect if running from build
> directory.
> 
> This is not as robust as for autotools because if you select --prefix
> in the build directory it will incorrectly enable the override as well
> but nobody should do that.

This wouldn't be that much of a problem as it would end up pointing to
the same files.

More of a problem is if we falsely assume that the override is not
necessary.

Fortunately it's mostly a problem for developers.

> We have to modify some of the tests to not add current build path into
> PATH variable and use the full path for virsh instead. Otherwise it
> would be impossible to figure out that we are running virsh from build
> directory.
> 
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>  src/util/virfile.c    | 34 ++++++++++++++++++-------
>  tests/virsh-optparse  | 58 +++++++++++++++++++------------------------
>  tests/virsh-schedinfo | 12 +++------
>  3 files changed, 54 insertions(+), 50 deletions(-)
> 
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 213acdbcaa2..4542a38278e 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -1781,21 +1781,37 @@ virFileFindResource(const char *filename,
>   * virFileActivateDirOverrideForProg:
>   * @argv0: argv[0] of the calling program
>   *
> - * Look at @argv0 and try to detect if running from
> - * a build directory, by looking for a 'lt-' prefix
> - * on the binary name, or '/.libs/' in the path
> + * Combine $PWD and @argv0, canonicalize it and check if abs_top_builddir
> + * matches as prefix in the path.
>   */
>  void
>  virFileActivateDirOverrideForProg(const char *argv0)
>  {
> -    char *file = strrchr(argv0, '/');
> -    if (!file || file[1] == '\0')
> +    const char *pwd = g_getenv("PWD");

Could you please justify why you chose to use the PWD env variable
instead of e.g. 'getcwd()' or a glib equivalent?

The environment variable requires to be set by the shell, while the
working directory is a property of the process.


> +    g_autofree char *fullPath = NULL;
> +    g_autofree char *canonPath = NULL;
> +    const char *path = NULL;
> +
> +    if (!pwd)
>          return;
> -    file++;
> -    if (STRPREFIX(file, "lt-") ||
> -        strstr(argv0, "/.libs/")) {
> +
> +    if (argv0[0] != '/') {
> +        fullPath = g_strdup_printf("%s/%s", pwd, argv0);
> +        canonPath = virFileCanonicalizePath(fullPath);
> +
> +        if (!canonPath) {
> +            VIR_DEBUG("Failed to get canonicalized path errno=%d", errno);
> +            return;
> +        }
> +
> +        path = canonPath;
> +    } else {
> +        path = argv0;
> +    }
> +
> +    if (STRPREFIX(path, abs_top_builddir)) {

Since this hardcodes the full build directory path, this will not work
in cases when the build-directory is shared to a different host e.g. via
a container or NFS and mounted in a different path.

I think we should create a sentinel file in the build directory and
check whether the directory where the executable resides has that file
which would not be installed afterwards obviously.

>          useDirOverride = true;
> -        VIR_DEBUG("Activating build dir override for %s", argv0);
> +        VIR_DEBUG("Activating build dir override for %s", path);
>      }
>  }