[libvirt PATCH 0/3] ci: Fix paths shown in the website
Compare https://abologna.gitlab.io/-/libvirt/-/jobs/2741293181/artifacts/website/man... with https://libvirt.org/manpages/virtqemud.html#files Andrea Bolognani (3): scripts: Port meson-install-web.py to pathlib scripts: Add $DESTDIR support to meson-install-web.py ci: Fix paths shown in the website .gitlab-ci.yml | 6 +++--- scripts/meson-install-web.py | 22 ++++++++++++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-) -- 2.35.3
This will be useful later. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- scripts/meson-install-web.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/scripts/meson-install-web.py b/scripts/meson-install-web.py index a03f8523cd..fdf407ba33 100755 --- a/scripts/meson-install-web.py +++ b/scripts/meson-install-web.py @@ -4,7 +4,12 @@ import os import shutil import sys +from pathlib import Path + for desc in sys.argv[1:]: inst = desc.split(':') - os.makedirs(inst[1], exist_ok=True) - shutil.copy(inst[0], inst[1]) + src = Path(inst[0]) + dst = Path(inst[1]) + + dst.mkdir(parents=True, exist_ok=True) + shutil.copy(src, dst) -- 2.35.3
meson already supports $DESTDIR natively, but in this case we're using a custom script and so we have to do some extra work ourselves. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- scripts/meson-install-web.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/scripts/meson-install-web.py b/scripts/meson-install-web.py index fdf407ba33..e7456fa750 100755 --- a/scripts/meson-install-web.py +++ b/scripts/meson-install-web.py @@ -6,10 +6,23 @@ import sys from pathlib import Path +destdir = os.getenv('DESTDIR') +if destdir: + destdir = Path(destdir) + if not destdir.is_absolute(): + print('$DESTDIR must be an absolute path') + sys.exit(1) + for desc in sys.argv[1:]: inst = desc.split(':') src = Path(inst[0]) dst = Path(inst[1]) + if destdir: + # Turn dst into a relative path by dropping its first component + # and append it to destdir to obtain the absolute destination + # path that respects the value $DESTDIR found in the environment + dst = Path(destdir, *dst.parts[1:]) + dst.mkdir(parents=True, exist_ok=True) shutil.copy(src, dst) -- 2.35.3
On Tue, Jul 19, 2022 at 04:17:44PM +0200, Andrea Bolognani wrote:
meson already supports $DESTDIR natively, but in this case we're using a custom script and so we have to do some extra work ourselves.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- scripts/meson-install-web.py | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/scripts/meson-install-web.py b/scripts/meson-install-web.py index fdf407ba33..e7456fa750 100755 --- a/scripts/meson-install-web.py +++ b/scripts/meson-install-web.py @@ -6,10 +6,23 @@ import sys
from pathlib import Path
+destdir = os.getenv('DESTDIR') +if destdir: + destdir = Path(destdir) + if not destdir.is_absolute(): + print('$DESTDIR must be an absolute path') + sys.exit(1)
I don't see any reason for this check. Yes, DESTDIR is mostly used with absolute path but the other two scripts where we use DESTDIR don't have this check and meson itself doesn't complaint if the path is not absolute as well. That brings me to the other point that there is no need to use pathlib at all. We can just do the same as scripts/meson-install-dirs.py or scripts/meson-install-symlink.py: destdir = os.environ.get('DESTDIR', os.sep) for desc in sys.argv[1:]: inst = desc.split(':') dst = os.path.join(destdir, inst[1].strip(os.sep)) os.makedirs(dst, exist_ok=True) shutil.copy(src, dst) Pavel
for desc in sys.argv[1:]: inst = desc.split(':') src = Path(inst[0]) dst = Path(inst[1])
+ if destdir: + # Turn dst into a relative path by dropping its first component + # and append it to destdir to obtain the absolute destination + # path that respects the value $DESTDIR found in the environment + dst = Path(destdir, *dst.parts[1:]) + dst.mkdir(parents=True, exist_ok=True) shutil.copy(src, dst) -- 2.35.3
On Tue, Aug 09, 2022 at 05:26:28PM +0200, Pavel Hrdina wrote:
On Tue, Jul 19, 2022 at 04:17:44PM +0200, Andrea Bolognani wrote:
+destdir = os.getenv('DESTDIR') +if destdir: + destdir = Path(destdir) + if not destdir.is_absolute(): + print('$DESTDIR must be an absolute path') + sys.exit(1)
I don't see any reason for this check. Yes, DESTDIR is mostly used with absolute path but the other two scripts where we use DESTDIR don't have this check and meson itself doesn't complaint if the path is not absolute as well.
That brings me to the other point that there is no need to use pathlib at all. We can just do the same as scripts/meson-install-dirs.py or scripts/meson-install-symlink.py:
destdir = os.environ.get('DESTDIR', os.sep)
for desc in sys.argv[1:]: inst = desc.split(':') dst = os.path.join(destdir, inst[1].strip(os.sep)) os.makedirs(dst, exist_ok=True) shutil.copy(src, dst)
You're absolutely right. Somehow I had not realized that we already had custom DESTDIR handling in other scripts that I could rip off ;) v2 here: https://listman.redhat.com/archives/libvir-list/2022-August/233655.html -- Andrea Bolognani / Red Hat / Virtualization
Right now we're setting the prefix to a custom path, which results in paths like /builds/libvirt/libvirt/vroot/etc/libvirt/virtqemud.conf ending up in the generated HTML. In order to avoid that, set the prefix and other installation paths to reasonable default values by passing -Dsystem=true and then take advantage of $DESTDIR support to still be able to write the HTML files without requiring root privileges. Reported-by: Martin Kletzander <mkletzan@redhat.com> Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- .gitlab-ci.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6a8b89729f..d5c431dee0 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -67,9 +67,9 @@ website: before_script: - *script_variables script: - - meson setup build --werror --prefix=$(pwd)/vroot || (cat build/meson-logs/meson-log.txt && exit 1) - - ninja -C build install-web - - mv vroot/share/doc/libvirt/html/ website + - meson setup build --werror -Dsystem=true || (cat build/meson-logs/meson-log.txt && exit 1) + - DESTDIR=$(pwd)/install ninja -C build install-web + - mv install/usr/share/doc/libvirt/html/ website artifacts: expose_as: 'Website' name: 'website' -- 2.35.3
On Tue, Jul 19, 2022 at 04:17:45PM +0200, Andrea Bolognani wrote:
Right now we're setting the prefix to a custom path, which results in paths like
/builds/libvirt/libvirt/vroot/etc/libvirt/virtqemud.conf
ending up in the generated HTML. In order to avoid that, set the prefix and other installation paths to reasonable default values by passing
-Dsystem=true
and then take advantage of $DESTDIR support to still be able to write the HTML files without requiring root privileges.
Reported-by: Martin Kletzander <mkletzan@redhat.com> Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- .gitlab-ci.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (2)
-
Andrea Bolognani -
Pavel Hrdina