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.

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@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");
+    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)) {
         useDirOverride = true;
-        VIR_DEBUG("Activating build dir override for %s", argv0);
+        VIR_DEBUG("Activating build dir override for %s", path);
     }
 }
 
diff --git a/tests/virsh-optparse b/tests/virsh-optparse
index d9c8f3c731b..fed71a8509e 100755
--- a/tests/virsh-optparse
+++ b/tests/virsh-optparse
@@ -21,17 +21,11 @@
 
 test_expensive
 
-# If $abs_top_builddir/tools is not early in $PATH, put it there,
-# so that we can safely invoke "virsh" simply with its name.
-case $PATH in
-  $abs_top_builddir/tools/src:$abs_top_builddir/tools:*) ;;
-  $abs_top_builddir/tools:*) ;;
-  *) PATH=$abs_top_builddir/tools:$PATH; export PATH ;;
-esac
+VIRSH=$abs_top_builddir/tools/virsh
 
 if test "$VERBOSE" = yes; then
   set -x
-  virsh --version
+  $VIRSH --version
 fi
 
 cat <<\EOF > exp-out || framework_failure
@@ -63,7 +57,7 @@ for args in \
     '--count 2 test' \
     '--count=2 test' \
 ; do
-  virsh -k0 -d0 -c $test_url setvcpus $args >out 2>>err || fail=1
+  $VIRSH -k0 -d0 -c $test_url setvcpus $args >out 2>>err || fail=1
   LC_ALL=C sort out | compare exp-out - || fail=1
 done
 
@@ -81,7 +75,7 @@ cat <<\EOF > exp-out || framework_failure
 </domainsnapshot>
 
 EOF
-virsh -q -c $test_url snapshot-create-as --print-xml test \
+$VIRSH -q -c $test_url snapshot-create-as --print-xml test \
   --diskspec 'vda,file=a&b,,c,snapshot=external' --description '1<2' \
   --diskspec vdb --memspec file=d,,e >out 2>>err || fail=1
 compare exp-out out || fail=1
@@ -96,7 +90,7 @@ cat <<\EOF > exp-out || framework_failure
 </domainsnapshot>
 
 EOF
-virsh -q -c $test_url snapshot-create-as  --print-xml test name vda vdb \
+$VIRSH -q -c $test_url snapshot-create-as  --print-xml test name vda vdb \
   >out 2>>err || fail=1
 compare exp-out out || fail=1
 
@@ -120,7 +114,7 @@ for args in \
     '--description desc --name name --domain test vda vdb' \
     '--description desc --diskspec vda --name name --domain test vdb' \
 ; do
-  virsh -q -c $test_url snapshot-create-as --print-xml $args \
+  $VIRSH -q -c $test_url snapshot-create-as --print-xml $args \
     >out 2>>err || fail=1
   compare exp-out out || fail=1
 done
@@ -131,7 +125,7 @@ test -s err && fail=1
 cat <<\EOF > exp-err || framework_failure
 error: this function is not supported by the connection driver: virDomainQemuMonitorCommand
 EOF
-virsh -q -c $test_url qemu-monitor-command test a >out 2>err && fail=1
+$VIRSH -q -c $test_url qemu-monitor-command test a >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
@@ -141,7 +135,7 @@ compare exp-err err || fail=1
 cat <<\EOF > exp-err || framework_failure
 error: Numeric value 'abc' for <start> option is malformed or out of range
 EOF
-virsh -q -c $test_url cpu-stats test --start abc >out 2>err && fail=1
+$VIRSH -q -c $test_url cpu-stats test --start abc >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
@@ -149,7 +143,7 @@ compare exp-err err || fail=1
 cat <<\EOF > exp-err || framework_failure
 error: Numeric value '42WB' for <start> option is malformed or out of range
 EOF
-virsh -q -c $test_url cpu-stats test --start 42WB >out 2>err && fail=1
+$VIRSH -q -c $test_url cpu-stats test --start 42WB >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
@@ -158,7 +152,7 @@ compare exp-err err || fail=1
 cat <<\EOF > exp-err || framework_failure
 error: Numeric value '42MB' for <start> option is malformed or out of range
 EOF
-virsh -q -c $test_url cpu-stats test --start 42MB >out 2>err && fail=1
+$VIRSH -q -c $test_url cpu-stats test --start 42MB >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
@@ -166,7 +160,7 @@ compare exp-err err || fail=1
 cat <<\EOF > exp-err || framework_failure
 error: Numeric value '2147483648' for <start> option is malformed or out of range
 EOF
-virsh -q -c $test_url cpu-stats test --start 2147483648 >out 2>err && fail=1
+$VIRSH -q -c $test_url cpu-stats test --start 2147483648 >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
@@ -175,7 +169,7 @@ compare exp-err err || fail=1
 cat <<\EOF > exp-err || framework_failure
 error: Invalid value for start CPU
 EOF
-virsh -q -c $test_url cpu-stats test --start -1 >out 2>err && fail=1
+$VIRSH -q -c $test_url cpu-stats test --start -1 >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
@@ -185,7 +179,7 @@ compare exp-err err || fail=1
 cat <<\EOF > exp-err || framework_failure
 error: Scaled numeric value 'abc' for <size> option is malformed or out of range
 EOF
-virsh -q -c $test_url setmaxmem test abc >out 2>err && fail=1
+$VIRSH -q -c $test_url setmaxmem test abc >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
@@ -194,18 +188,18 @@ cat <<\EOF > exp-err || framework_failure
 error: Scaled numeric value '42WB' for <size> option is malformed or out of range
 error: invalid argument: unknown suffix 'WB'
 EOF
-virsh -q -c $test_url setmaxmem test 42WB >out 2>err && fail=1
+$VIRSH -q -c $test_url setmaxmem test 42WB >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
 # Numeric value with valid suffix
-virsh -q -c $test_url setmaxmem test 42MB --config >out 2>err || fail=1
+$VIRSH -q -c $test_url setmaxmem test 42MB --config >out 2>err || fail=1
 test -s out && fail=1
 test -s err && fail=1
 
 # Numeric value bigger than INT_MAX. No failure here because
 # scaled numeric values are unsigned long long
-virsh -q -c $test_url setmaxmem test 2147483648 --config >out 2>err || fail=1
+$VIRSH -q -c $test_url setmaxmem test 2147483648 --config >out 2>err || fail=1
 test -s out && fail=1
 test -s err && fail=1
 
@@ -213,7 +207,7 @@ test -s err && fail=1
 cat <<\EOF > exp-err || framework_failure
 error: Scaled numeric value '-1' for <size> option is malformed or out of range
 EOF
-virsh -q -c $test_url setmaxmem test -1 >out 2>err && fail=1
+$VIRSH -q -c $test_url setmaxmem test -1 >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
@@ -223,12 +217,12 @@ cat <<\EOF > exp-err || framework_failure
 error: Unable to change MaxMemorySize
 error: memory in virDomainSetMaxMemory must not be zero
 EOF
-virsh -q -c $test_url setmaxmem test 0 >out 2>err && fail=1
+$VIRSH -q -c $test_url setmaxmem test 0 >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
 # Numeric value
-virsh -q -c $test_url setmaxmem test 42 --config >out 2>err || fail=1
+$VIRSH -q -c $test_url setmaxmem test 42 --config >out 2>err || fail=1
 test -s out && fail=1
 test -s err && fail=1
 
@@ -238,7 +232,7 @@ test -s err && fail=1
 cat <<\EOF > exp-err || framework_failure
 error: Numeric value 'abc' for <timeout> option is malformed or out of range
 EOF
-virsh -q -c $test_url event --all --timeout abc >out 2>err && fail=1
+$VIRSH -q -c $test_url event --all --timeout abc >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
@@ -247,7 +241,7 @@ compare exp-err err || fail=1
 cat <<\EOF > exp-err || framework_failure
 error: Numeric value '2147484' for <timeout> option is malformed or out of range
 EOF
-virsh -q -c $test_url event --all --timeout 2147484 >out 2>err && fail=1
+$VIRSH -q -c $test_url event --all --timeout 2147484 >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
@@ -255,7 +249,7 @@ compare exp-err err || fail=1
 cat <<\EOF > exp-err || framework_failure
 error: Numeric value '42WB' for <timeout> option is malformed or out of range
 EOF
-virsh -q -c $test_url event --all --timeout 42WB >out 2>err && fail=1
+$VIRSH -q -c $test_url event --all --timeout 42WB >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
@@ -264,7 +258,7 @@ compare exp-err err || fail=1
 cat <<\EOF > exp-err || framework_failure
 error: Numeric value '42MB' for <timeout> option is malformed or out of range
 EOF
-virsh -q -c $test_url event --all --timeout 42MB >out 2>err && fail=1
+$VIRSH -q -c $test_url event --all --timeout 42MB >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
@@ -272,7 +266,7 @@ compare exp-err err || fail=1
 cat <<\EOF > exp-err || framework_failure
 error: Numeric value '-1' for <timeout> option is malformed or out of range
 EOF
-virsh -q -c $test_url event --all --timeout -1 >out 2>err && fail=1
+$VIRSH -q -c $test_url event --all --timeout -1 >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
@@ -281,7 +275,7 @@ compare exp-err err || fail=1
 cat <<\EOF > exp-err || framework_failure
 error: Numeric value '0' for <timeout> option is malformed or out of range
 EOF
-virsh -q -c $test_url event --all --timeout 0 >out 2>err && fail=1
+$VIRSH -q -c $test_url event --all --timeout 0 >out 2>err && fail=1
 test -s out && fail=1
 compare exp-err err || fail=1
 
@@ -291,7 +285,7 @@ cat <<\EOF > exp-out || framework_failure
 event loop timed out
 events received: 0
 EOF
-virsh -q -c $test_url event --all --timeout 1 >out 2>err && fail=1
+$VIRSH -q -c $test_url event --all --timeout 1 >out 2>err && fail=1
 test -s err && fail=1
 compare exp-out out || fail=1
 
diff --git a/tests/virsh-schedinfo b/tests/virsh-schedinfo
index d6d9ac2d59e..e4e2509f762 100755
--- a/tests/virsh-schedinfo
+++ b/tests/virsh-schedinfo
@@ -19,17 +19,11 @@
 
 . "$(dirname $0)/test-lib.sh"
 
-# If $abs_top_builddir/tools is not early in $PATH, put it there,
-# so that we can safely invoke "virsh" simply with its name.
-case $PATH in
-  $abs_top_builddir/tools/src:$abs_top_builddir/tools:*) ;;
-  $abs_top_builddir/tools:*) ;;
-  *) PATH=$abs_top_builddir/tools:$PATH; export PATH ;;
-esac
+VIRSH=$abs_top_builddir/tools/virsh
 
 if test "$VERBOSE" = yes; then
   set -x
-  virsh --version
+  $VIRSH --version
 fi
 
 printf 'Scheduler      : fair\n\n' > exp-out || framework_failure
@@ -39,7 +33,7 @@ fail=0
 
 test_url=test:///default
 
-virsh -c $test_url schedinfo 1 --set j=k >out 2>err && fail=1
+$VIRSH -c $test_url schedinfo 1 --set j=k >out 2>err && fail=1
 compare exp-out out || fail=1
 compare exp-err err || fail=1
 
-- 
2.26.2

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

On a Wednesday in 2020, Peter Krempa wrote:
>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.
>

That's why I'd rather drop this "override-by-binary-path" approach
and use the env variable in the "run" script for this.

You'd only get the override if you asked for it.

>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@redhat.com>
>> ---
>>  src/util/virfile.c    | 34 ++++++++++++++++++-------
>>  tests/virsh-optparse  | 58 +++++++++++++++++++------------------------
>>  tests/virsh-schedinfo | 12 +++------

The test changes can be separated out of this patch.

>>  3 files changed, 54 insertions(+), 50 deletions(-)
>>

Jano

On Wed, Jul 22, 2020 at 10:33:58 +0200, Ján Tomko wrote:
> On a Wednesday in 2020, Peter Krempa wrote:
> > 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.
> > 
> 
> That's why I'd rather drop this "override-by-binary-path" approach
> and use the env variable in the "run" script for this.

That would mandate using ./run even in situations where it was not
required before.

Similarly to what libtool did with the shell script overlay file, meson
links the binaries with local versions of the .so's compiled by the
project. This means that the binary can still be run without using
./run. (It's relinked on install I presume).

Thus I'd prefer if we still keep the possibility to exec binaries
without invoking ./run, but the overrides for dynamicaly loaded files
will still work.

On a Wednesday in 2020, Peter Krempa wrote:
>On Wed, Jul 22, 2020 at 10:33:58 +0200, Ján Tomko wrote:
>> On a Wednesday in 2020, Peter Krempa wrote:
>> > 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.
>> >
>>
>> That's why I'd rather drop this "override-by-binary-path" approach
>> and use the env variable in the "run" script for this.
>
>That would mandate using ./run even in situations where it was not
>required before.
>

Yes, that's what I meant.

Jano

>Similarly to what libtool did with the shell script overlay file, meson
>links the binaries with local versions of the .so's compiled by the
>project. This means that the binary can still be run without using
>./run. (It's relinked on install I presume).
>
>Thus I'd prefer if we still keep the possibility to exec binaries
>without invoking ./run, but the overrides for dynamicaly loaded files
>will still work.

On Wed, Jul 22, 2020 at 11:32:07 +0200, Ján Tomko wrote:
> On a Wednesday in 2020, Peter Krempa wrote:
> > On Wed, Jul 22, 2020 at 10:33:58 +0200, Ján Tomko wrote:
> > > On a Wednesday in 2020, Peter Krempa wrote:
> > > > 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.
> > > >
> > > 
> > > That's why I'd rather drop this "override-by-binary-path" approach
> > > and use the env variable in the "run" script for this.
> > 
> > That would mandate using ./run even in situations where it was not
> > required before.
> > 
> 
> Yes, that's what I meant.

I strongly disagree with doing that. Meson tries hard to make the files
in the build directory work out of the box. This would make it partially
and magically fail in many cases which is imo very bad.

On Wed, Jul 22, 2020 at 11:43:09AM +0200, Peter Krempa wrote:
> On Wed, Jul 22, 2020 at 11:32:07 +0200, Ján Tomko wrote:
> > On a Wednesday in 2020, Peter Krempa wrote:
> > > On Wed, Jul 22, 2020 at 10:33:58 +0200, Ján Tomko wrote:
> > > > On a Wednesday in 2020, Peter Krempa wrote:
> > > > > 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.
> > > > >
> > > > 
> > > > That's why I'd rather drop this "override-by-binary-path" approach
> > > > and use the env variable in the "run" script for this.
> > > 
> > > That would mandate using ./run even in situations where it was not
> > > required before.
> > > 
> > 
> > Yes, that's what I meant.
> 
> I strongly disagree with doing that. Meson tries hard to make the files
> in the build directory work out of the box. This would make it partially
> and magically fail in many cases which is imo very bad.

I agree with Peter here, since meson already does some magic to
correctly point to compiled libraries within the project it would be
nice to keep this functionality.

I'll try the sentinel file approach that Peter suggested to see if it
works and if it's not too ugly.

Thanks

Pavel

On Wed, Jul 22, 2020 at 12:13:18PM +0200, Pavel Hrdina wrote:
> On Wed, Jul 22, 2020 at 11:43:09AM +0200, Peter Krempa wrote:
> > On Wed, Jul 22, 2020 at 11:32:07 +0200, Ján Tomko wrote:
> > > On a Wednesday in 2020, Peter Krempa wrote:
> > > > On Wed, Jul 22, 2020 at 10:33:58 +0200, Ján Tomko wrote:
> > > > > On a Wednesday in 2020, Peter Krempa wrote:
> > > > > > 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.
> > > > > >
> > > > > 
> > > > > That's why I'd rather drop this "override-by-binary-path" approach
> > > > > and use the env variable in the "run" script for this.
> > > > 
> > > > That would mandate using ./run even in situations where it was not
> > > > required before.
> > > > 
> > > 
> > > Yes, that's what I meant.
> > 
> > I strongly disagree with doing that. Meson tries hard to make the files
> > in the build directory work out of the box. This would make it partially
> > and magically fail in many cases which is imo very bad.
> 
> I agree with Peter here, since meson already does some magic to
> correctly point to compiled libraries within the project it would be
> nice to keep this functionality.
> 
> I'll try the sentinel file approach that Peter suggested to see if it
> works and if it's not too ugly.

When I was looking into using sentinel file I realized that it is
pointless to use it.

The argument about NFS or mounting it into container is a good point but
irrelevant in our case. If we detect somehow that we need to override
the path our code has hard-coded abs_top_srcdir or abs_top_builddir as
part of the new path so it would not work in these cases even with the
sentinel file. In addition not only our code hard-codes the absolute
path, meson does the same for libraries.

So I'll keep the detection based on canonicalized path and
abs_top_builddir prefix match including some cleanup and not using PWD
directly.

Pavel