"install_subdir" breaks when DESTDIR is a relative path

[RafaGago]

This is present on 0.5. It's very easy to reproduce:

cd $(mktemp -d)
mkdir -p include/myproject
touch include/myproject/myfile.h
echo "project('myproject')" > meson.build
echo "install_subdir('include/myproject', install_dir: get_option('includedir'))" >> meson.build
meson builddir
DESTDIR=reldir ninja -C builddir install

ninja: Entering directory `builddir'
[0/1] Installing files.
Installing subdir /tmp/mesonerase/include/myproject to reldir/usr/local/include/myproject
Traceback (most recent call last):
File "/usr/local/lib/python3.6/dist-packages/mesonbuild/mesonmain.py", line 122, in run
return options.run_func(options)
File "/usr/local/lib/python3.6/dist-packages/mesonbuild/minstall.py", line 514, in run
installer.do_install(datafilename)
File "/usr/local/lib/python3.6/dist-packages/mesonbuild/minstall.py", line 342, in do_install
self.install_subdirs(d) # Must be first, because it needs to delete the old subtree.
File "/usr/local/lib/python3.6/dist-packages/mesonbuild/minstall.py", line 366, in install_subdirs
self.do_copydir(d, src_dir, full_dst_dir, exclude, mode)
File "/usr/local/lib/python3.6/dist-packages/mesonbuild/minstall.py", line 289, in do_copydir
raise ValueError('dst_dir must be absolute, got %s' % dst_dir)
ValueError: dst_dir must be absolute, got reldir/usr/local/include/myproject

The workaround is pretty trivial: "DESTDIR=$PWD/reldir ninja -C builddir install"

[Akaricchi]

I don't think this ever really worked. The same error occurs at least as far as Meson 0.45. I'm not sure it's even possible to make it work relative to $PWD, because Ninja would cd into the build dir before meson would even begin to process $DESTDIR. It's easy to make it work relative to the build dir, but that is not really correct, and I'm not entirely sure it doesn't break anything else:

diff --git a/mesonbuild/scripts/__init__.py b/mesonbuild/scripts/__init__.py
index af6bedc1..7c8e1613 100644
--- a/mesonbuild/scripts/__init__.py
+++ b/mesonbuild/scripts/__init__.py
@@ -12,9 +12,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import os.path
+
 def destdir_join(d1, d2):
     # c:\destdir + c:\prefix must produce c:\destdir\prefix
     if len(d1) > 1 and d1[1] == ':' \
             and len(d2) > 1 and d2[1] == ':':
         return d1 + d2[2:]
-    return d1 + d2
+    return os.path.abspath(d1 + d2)

This seems to match the (perhaps unintentional) behavior of older Meson versions e.g. 0.40.0.

[RafaGago]

Well, I guess that the $PWD environment variable is still available in ninja somehow, so it could be sent back to meson.

It's not the most important bug IMO anyways, as it has an easy workaround and package maintainers will mostly install to abspaths. I only use this when I'm developing and I want to test that the tree is correctly generated.

Still on autotools/cmake DESTDIR can be relative, so the error comes as unexpected. The python stack trace makes it look unintentional (bug).

[Akaricchi]

Well, I guess that the $PWD environment variable is still available in ninja somehow, so it could be sent back to meson.

It isn't, not in its original form. The problem is that using the -C argument causes ninja to change working directory very early on, before it even begins to interpret the build rules. So there really is no way for it to pass the previous working directory back to meson.

For what it's worth, I'm not really opposed to DESTDIR being interpreted relative to the build dir (perhaps with a warning?), and I definitely agree that if relative DESTDIR is not supported, then meson should exit gracefully rather than let an exception slip up.

Still on autotools/cmake DESTDIR can be relative

Do you happen to know how CMake's ninja backend handles this issue?

[RafaGago]

Well, if ninja does that so early then a solution could be to do platform specific stuff with the parent PID to retrieve the path. I'm almost sure that it's not worth the added complexity.

I was using CMake with regular make. I tried with ninja an then it gets the behavior you described. It places things relative to the build directory. This is totally unexpected from the user point of view, you pass one path and get things on another. It is undesirable IMO.

The most sane and pragmatic thing to do is to ban relative paths on DESTDIR with a clear message (and maybe a very short rationale). This wouldn't break anyone's files, as relative paths on DESTDIR aren't working anyways. The workaround is so simple that no one can get irritated.

[nirbheek]

The most sane and pragmatic thing to do is to ban relative paths on DESTDIR with a clear message (and maybe a very short rationale).

Sounds good to me!

[orbea]

The most sane and pragmatic thing to do is to ban relative paths on DESTDIR with a clear message (and maybe a very short rationale). This wouldn't break anyone's files, as relative paths on DESTDIR aren't working anyways. The workaround is so simple that no one can get irritated.

I have used relative paths with DESTDIR where it worked as expected. My reasoning is install target needed additional cleanup (Its a special case) and I had to install files to a final directory manually before creating the package. Having it relative to the build directory was not a problem as it was just one less temporary directory to remove.

I suppose I could change this to a full path, but that would be additional work and would break my currently working build script...

[eli-schwartz]

Since #9060 if DESTDIR is not an absolute path, meson will convert it to an absolute path.

If that's not the path you expected it to be because ninja -C builddir install chdir'ed out from under you then, well, you should have used an absolute path to begin with I suppose.

I wouldn't cry if it had been banned rather than being worked around, truthfully, but... it doesn't error out anymore.