[PATCH] meson: src: Fix DESTDIR handling while creating dirs

From: Jan Kiszka <jan.kiszka@siemens.com> If the target path contains a link with an absolute path (e.g. /var/run -> /run), makedirs will target the wrong location. Resolve the path first, then append DESTDIR again if needed. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- scripts/meson-install-dirs.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/meson-install-dirs.py b/scripts/meson-install-dirs.py index 14ec6b93e8..e762e44712 100644 --- a/scripts/meson-install-dirs.py +++ b/scripts/meson-install-dirs.py @@ -6,4 +6,7 @@ import sys destdir = os.environ.get('DESTDIR', os.sep) for dirname in sys.argv[1:]: - os.makedirs(os.path.join(destdir, dirname.strip(os.sep)), exist_ok=True) + realdir = os.path.realpath(os.path.join(destdir, dirname.strip(os.sep))) + if not realdir.startswith(destdir): + realdir = os.path.join(destdir, realdir.strip(os.sep)) + os.makedirs(realdir, exist_ok=True) -- 2.26.2

On Mon, Sep 07, 2020 at 11:25:34PM +0200, Jan Kiszka wrote:
From: Jan Kiszka <jan.kiszka@siemens.com>
If the target path contains a link with an absolute path (e.g. /var/run -> /run), makedirs will target the wrong location. Resolve the path first, then append DESTDIR again if needed.
Can you elaborate on this? os.makedirs should follow the symlinks before actually creating the directory hierarchy, so I'm failing to see the problem this patch is trying to address. Erik

On 09.09.20 16:38, Erik Skultety wrote:
On Mon, Sep 07, 2020 at 11:25:34PM +0200, Jan Kiszka wrote:
From: Jan Kiszka <jan.kiszka@siemens.com>
If the target path contains a link with an absolute path (e.g. /var/run -> /run), makedirs will target the wrong location. Resolve the path first, then append DESTDIR again if needed.
Can you elaborate on this? os.makedirs should follow the symlinks before actually creating the directory hierarchy, so I'm failing to see the problem this patch is trying to address.
mkdir -p /my/destdir/var /my/destdir/run ln -s /run /my/destdir/var/run meson build --prefix=/usr DESTDIR=/my/destdir ninja -C build install -> PermissionError: [Errno 13] Permission denied: '/my/destdir/var/run/libvirt' Jan

On Wed, Sep 09, 2020 at 05:06:19PM +0200, Jan Kiszka wrote:
On 09.09.20 16:38, Erik Skultety wrote:
On Mon, Sep 07, 2020 at 11:25:34PM +0200, Jan Kiszka wrote:
From: Jan Kiszka <jan.kiszka@siemens.com>
If the target path contains a link with an absolute path (e.g. /var/run -> /run), makedirs will target the wrong location. Resolve the path first, then append DESTDIR again if needed.
Can you elaborate on this? os.makedirs should follow the symlinks before actually creating the directory hierarchy, so I'm failing to see the problem this patch is trying to address.
mkdir -p /my/destdir/var /my/destdir/run ln -s /run /my/destdir/var/run meson build --prefix=/usr DESTDIR=/my/destdir ninja -C build install
-> PermissionError: [Errno 13] Permission denied: '/my/destdir/var/run/libvirt'
I would say don't do that as that's not purpose of DESTDIR and running libvirt from the DESTDIR installation will not work as it will not find all the required files. What is you use-case or what are you trying to achieve? Pavel

Am 9. September 2020 17:47:59 MESZ schrieb Pavel Hrdina <phrdina@redhat.com>:
On 09.09.20 16:38, Erik Skultety wrote:
On Mon, Sep 07, 2020 at 11:25:34PM +0200, Jan Kiszka wrote:
From: Jan Kiszka <jan.kiszka@siemens.com>
If the target path contains a link with an absolute path (e.g. /var/run -> /run), makedirs will target the wrong location. Resolve the path first, then append DESTDIR again if needed.
Can you elaborate on this? os.makedirs should follow the symlinks before actually creating the directory hierarchy, so I'm failing to see
On Wed, Sep 09, 2020 at 05:06:19PM +0200, Jan Kiszka wrote: the problem
this patch is trying to address.
mkdir -p /my/destdir/var /my/destdir/run ln -s /run /my/destdir/var/run meson build --prefix=/usr DESTDIR=/my/destdir ninja -C build install
-> PermissionError: [Errno 13] Permission denied: '/my/destdir/var/run/libvirt'
I would say don't do that as that's not purpose of DESTDIR and running libvirt from the DESTDIR installation will not work as it will not find all the required files.
What is you use-case or what are you trying to achieve?
https://www.gnu.org/prep/standards/html_node/DESTDIR.html It's exactly for what I'm using it: installing the package into a rootfs dir that is alien to the builder. It will become the rootfs on the target only. Jan PS: Is it normal with meson that one has to implement such basic stuff at project level? -- Sent from an Android - sorry, just in case...

On Wed, Sep 09, 2020 at 05:53:06PM +0200, Jan Kiszka wrote:
Am 9. September 2020 17:47:59 MESZ schrieb Pavel Hrdina <phrdina@redhat.com>:
On 09.09.20 16:38, Erik Skultety wrote:
On Mon, Sep 07, 2020 at 11:25:34PM +0200, Jan Kiszka wrote:
From: Jan Kiszka <jan.kiszka@siemens.com>
If the target path contains a link with an absolute path (e.g. /var/run -> /run), makedirs will target the wrong location. Resolve the path first, then append DESTDIR again if needed.
Can you elaborate on this? os.makedirs should follow the symlinks before actually creating the directory hierarchy, so I'm failing to see
On Wed, Sep 09, 2020 at 05:06:19PM +0200, Jan Kiszka wrote: the problem
this patch is trying to address.
mkdir -p /my/destdir/var /my/destdir/run ln -s /run /my/destdir/var/run meson build --prefix=/usr DESTDIR=/my/destdir ninja -C build install
-> PermissionError: [Errno 13] Permission denied: '/my/destdir/var/run/libvirt'
I would say don't do that as that's not purpose of DESTDIR and running libvirt from the DESTDIR installation will not work as it will not find all the required files.
What is you use-case or what are you trying to achieve?
https://www.gnu.org/prep/standards/html_node/DESTDIR.html
It's exactly for what I'm using it: installing the package into a rootfs dir that is alien to the builder. It will become the rootfs on the target only.
So if I understand it correctly you set DESTDIR to the path to that rootfs in order to install it there. I'm not sure if that is the desired usage of DESTDIR. My understanding is that it should be empty directory where package systems will install the project in order to create a package out of it. Or for users to inspect what is installed by the project. Looking at our old autoconf build system it would fail with the same permission denied error as we were using this in Makefile: $(MKDIR_P) "$(DESTDIR)$(runstatedir)/libvirt/libxl" Based on that I'm not sure if we want to fix it.
PS: Is it normal with meson that one has to implement such basic stuff at project level?
I would not say it's normal but in the current state of Meson it is probably expected to have some features missing as it's a fairly young project and they are still introducing a lot of features. Pavel

On 9/9/20 5:47 PM, Pavel Hrdina wrote:
On Wed, Sep 09, 2020 at 05:06:19PM +0200, Jan Kiszka wrote:
On 09.09.20 16:38, Erik Skultety wrote:
On Mon, Sep 07, 2020 at 11:25:34PM +0200, Jan Kiszka wrote:
From: Jan Kiszka <jan.kiszka@siemens.com>
If the target path contains a link with an absolute path (e.g. /var/run -> /run), makedirs will target the wrong location. Resolve the path first, then append DESTDIR again if needed.
Can you elaborate on this? os.makedirs should follow the symlinks before actually creating the directory hierarchy, so I'm failing to see the problem this patch is trying to address.
mkdir -p /my/destdir/var /my/destdir/run ln -s /run /my/destdir/var/run meson build --prefix=/usr DESTDIR=/my/destdir ninja -C build install
-> PermissionError: [Errno 13] Permission denied: '/my/destdir/var/run/libvirt'
I would say don't do that as that's not purpose of DESTDIR and running libvirt from the DESTDIR installation will not work as it will not find all the required files.
This is exactly what gentoo portage does, btw. And I bet some other package tools too. Michal

On Wed, Sep 09, 2020 at 06:06:19PM +0200, Michal Privoznik wrote:
On 9/9/20 5:47 PM, Pavel Hrdina wrote:
On Wed, Sep 09, 2020 at 05:06:19PM +0200, Jan Kiszka wrote:
On 09.09.20 16:38, Erik Skultety wrote:
On Mon, Sep 07, 2020 at 11:25:34PM +0200, Jan Kiszka wrote:
From: Jan Kiszka <jan.kiszka@siemens.com>
If the target path contains a link with an absolute path (e.g. /var/run -> /run), makedirs will target the wrong location. Resolve the path first, then append DESTDIR again if needed.
Can you elaborate on this? os.makedirs should follow the symlinks before actually creating the directory hierarchy, so I'm failing to see the problem this patch is trying to address.
mkdir -p /my/destdir/var /my/destdir/run ln -s /run /my/destdir/var/run meson build --prefix=/usr DESTDIR=/my/destdir ninja -C build install
-> PermissionError: [Errno 13] Permission denied: '/my/destdir/var/run/libvirt'
I would say don't do that as that's not purpose of DESTDIR and running libvirt from the DESTDIR installation will not work as it will not find all the required files.
This is exactly what gentoo portage does, btw. And I bet some other package tools too.
RPM is doing the same and everything works correctly, that's why I wrote the statement above. It breaks only if the DESTDIR is not empty directory where there are some links outside of DESTDIR. This use-case would not work with our previous autotools build system as well. So I don't understand what you meant by your reply. Pavel

On Wed, Sep 09, 2020 at 05:06:19PM +0200, Jan Kiszka wrote:
On 09.09.20 16:38, Erik Skultety wrote:
On Mon, Sep 07, 2020 at 11:25:34PM +0200, Jan Kiszka wrote:
From: Jan Kiszka <jan.kiszka@siemens.com>
If the target path contains a link with an absolute path (e.g. /var/run -> /run), makedirs will target the wrong location. Resolve the path first, then append DESTDIR again if needed.
Can you elaborate on this? os.makedirs should follow the symlinks before actually creating the directory hierarchy, so I'm failing to see the problem this patch is trying to address.
mkdir -p /my/destdir/var /my/destdir/run ln -s /run /my/destdir/var/run
This looks dubious to me as you're trying to install into a private root dir, but the root dir is pointing to directories outside of it. Instead it should be ln -s /my/destdir/run /my/destdir/var/run to let installation work normally, then when creating the package the destdir needs to be stripped from the symlink. Or alternatively don't use the symlink at all, and instead tell libvirt to install straight to /my/destdir/run
meson build --prefix=/usr
ie pass -Drunstatedir=/run
DESTDIR=/my/destdir ninja -C build install
-> PermissionError: [Errno 13] Permission denied: '/my/destdir/var/run/libvirt'
Jan
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (5)
-
Daniel P. Berrangé
-
Erik Skultety
-
Jan Kiszka
-
Michal Privoznik
-
Pavel Hrdina