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@xxxxxxxxxx>
> ---
> 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);
> }
> }
