meson: src: Fix DESTDIR handling while creating dirs

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

Posted by Jan Kiszka 3 years, 7 months ago

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

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

Posted by Erik Skultety 3 years, 7 months ago

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

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

Posted by Jan Kiszka 3 years, 7 months ago

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

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

Posted by Pavel Hrdina 3 years, 7 months ago

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

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

Posted by Jan Kiszka 3 years, 7 months ago


Am 9. September 2020 17:47:59 MESZ schrieb Pavel Hrdina <phrdina@redhat.com>:
>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?

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...

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

Posted by Pavel Hrdina 3 years, 7 months ago

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 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?
> 
> 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

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

Posted by Michal Privoznik 3 years, 7 months ago

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

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

Posted by Pavel Hrdina 3 years, 7 months ago

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

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

Posted by Daniel P. Berrangé 3 years, 7 months ago

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 :|