GNU bug report logs -
#42547
[PATCH] build-system/qt: Don't include useless inputs in wrapped variables.
Previous Next
Reported by: Jakub Kądziołka <kuba <at> kadziolka.net>
Date: Sun, 26 Jul 2020 12:23:02 UTC
Severity: normal
Tags: patch
Done: Hartmut Goebel <h.goebel <at> goebel-consult.de>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 42547 in the body.
You can then email your comments to 42547 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
guix-patches <at> gnu.org
:
bug#42547
; Package
guix-patches
.
(Sun, 26 Jul 2020 12:23:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Jakub Kądziołka <kuba <at> kadziolka.net>
:
New bug report received and forwarded. Copy sent to
guix-patches <at> gnu.org
.
(Sun, 26 Jul 2020 12:23:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
* guix/build-system/qt.scm (qt-build)[qt-wrap-excluded-inputs]: New argument.
* guix/build/qt-build-system.scm (variables-for-wrapping): Take the
output directory as an argument for special handling. Check for
subdirectories of /share used by Qt before including inputs in
XDG_DATA_DIRS.
(wrap-all-programs): Pass the output directory to variables-for-wrapping.
(wrap-all-programs)[qt-wrap-excluded-inputs]: New argument.
---
guix/build-system/qt.scm | 3 +++
guix/build/qt-build-system.scm | 45 ++++++++++++++++++++++++----------
2 files changed, 35 insertions(+), 13 deletions(-)
diff --git a/guix/build-system/qt.scm b/guix/build-system/qt.scm
index 118022ec45..3da73e2b58 100644
--- a/guix/build-system/qt.scm
+++ b/guix/build-system/qt.scm
@@ -3,6 +3,7 @@
;;; Copyright © 2013 Cyril Roelandt <tipecaml <at> gmail.com>
;;; Copyright © 2017 Ricardo Wurmus <rekado <at> elephly.net>
;;; Copyright © 2019 Hartmut Goebel <h.goebel <at> crazy-compilers.com>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -123,6 +124,7 @@
"bin" "sbin"))
(phases '(@ (guix build qt-build-system)
%standard-phases))
+ (qt-wrap-excluded-inputs ''("qttools"))
(qt-wrap-excluded-outputs ''())
(system (%current-system))
(imported-modules %qt-build-system-modules)
@@ -146,6 +148,7 @@ provides a 'CMakeLists.txt' file as its build system."
#:search-paths ',(map search-path-specification->sexp
search-paths)
#:phases ,phases
+ #:qt-wrap-excluded-inputs ,qt-wrap-excluded-inputs
#:qt-wrap-excluded-outputs ,qt-wrap-excluded-outputs
#:configure-flags ,configure-flags
#:make-flags ,make-flags
diff --git a/guix/build/qt-build-system.scm b/guix/build/qt-build-system.scm
index 005157b0a4..1239c9f5fb 100644
--- a/guix/build/qt-build-system.scm
+++ b/guix/build/qt-build-system.scm
@@ -3,6 +3,7 @@
;;; Copyright © 2014, 2015 Ludovic Courtès <ludo <at> gnu.org>
;;; Copyright © 2018 Mark H Weaver <mhw <at> netris.org>
;;; Copyright © 2019, 2020 Hartmut Goebel <h.goebel <at> crazy-compilers.com>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -47,29 +48,44 @@
(setenv "CTEST_OUTPUT_ON_FAILURE" "1")
#t)
-(define (variables-for-wrapping base-directories)
+;; NOTE: Apart from standard subdirectories of /share, Qt also provides facilities
+;; for per-application data directories, such as /share/quassel. Thus, we include
+;; the output directory even if it doesn't contain any of the standard subdirectories.
+(define (variables-for-wrapping base-directories output-directory)
- (define (collect-sub-dirs base-directories subdirectory)
+ (define (collect-sub-dirs base-directories subdir-spec)
(filter-map
(lambda (dir)
- (let ((directory (string-append dir subdirectory)))
- (if (directory-exists? directory) directory #f)))
+ (and
+ (match subdir-spec
+ ((subdir) (directory-exists? (string-append dir subdir)))
+ ((subdir children)
+ (or
+ (or-map
+ (lambda (child)
+ (directory-exists? (string-append dir subdir child)))
+ children)
+ (and (eq? dir output-directory)
+ (directory-exists? (string-append dir subdir))))))
+ (string-append dir (car subdir-spec))))
base-directories))
(filter
(lambda (var-to-wrap) (not (null? (last var-to-wrap))))
(map
- (lambda (var-spec)
- `(,(first var-spec) = ,(collect-sub-dirs base-directories (last var-spec))))
+ (match-lambda
+ ((var . subdir-spec)
+ `(,var = ,(collect-sub-dirs base-directories subdir-spec))))
(list
;; these shall match the search-path-specification for Qt and KDE
;; libraries
- '("XDG_DATA_DIRS" "/share")
+ '("XDG_DATA_DIRS" "/share" ("/applications" "/fonts" "/icons" "/mime"))
'("XDG_CONFIG_DIRS" "/etc/xdg")
'("QT_PLUGIN_PATH" "/lib/qt5/plugins")
'("QML2_IMPORT_PATH" "/lib/qt5/qml")))))
(define* (wrap-all-programs #:key inputs outputs
+ (qt-wrap-excluded-inputs '("qttools"))
(qt-wrap-excluded-outputs '())
#:allow-other-keys)
"Implement phase \"qt-wrap\": look for GSettings schemas and
@@ -90,10 +106,13 @@ add a dependency of that output on Qt."
(string-append directory "/lib/libexec"))))
(define input-directories
- ;; FIXME: Filter out unwanted inputs, e.g. cmake
- (match inputs
- (((_ . dir) ...)
- dir)))
+ (filter-map
+ (match-lambda
+ ((name . dir)
+ (if (member name qt-wrap-excluded-inputs)
+ #f
+ dir)))
+ inputs))
(define handle-output
(match-lambda
@@ -101,8 +120,8 @@ add a dependency of that output on Qt."
(unless (member output qt-wrap-excluded-outputs)
(let ((bin-list (find-files-to-wrap directory))
(vars-to-wrap (variables-for-wrapping
- (append (list directory)
- input-directories))))
+ (cons directory input-directories)
+ directory)))
(when (not (null? vars-to-wrap))
(for-each (cut apply wrap-program <> vars-to-wrap)
bin-list)))))))
--
2.27.0
Information forwarded
to
guix-patches <at> gnu.org
:
bug#42547
; Package
guix-patches
.
(Sat, 19 Sep 2020 20:37:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 42547 <at> debbugs.gnu.org (full text, mbox):
Hi Jakub,
Jakub Kądziołka <kuba <at> kadziolka.net> skribis:
> * guix/build-system/qt.scm (qt-build)[qt-wrap-excluded-inputs]: New argument.
> * guix/build/qt-build-system.scm (variables-for-wrapping): Take the
> output directory as an argument for special handling. Check for
> subdirectories of /share used by Qt before including inputs in
> XDG_DATA_DIRS.
> (wrap-all-programs): Pass the output directory to variables-for-wrapping.
> (wrap-all-programs)[qt-wrap-excluded-inputs]: New argument.
This sounds like a good idea.
Do you know what impact this has on the closure size of packages you
looked at?
There are quite a few packages using ‘qt-build-system’. Probably a
change for ‘staging’ or ‘core-updates’?
> +(define (variables-for-wrapping base-directories output-directory)
>
> - (define (collect-sub-dirs base-directories subdirectory)
> + (define (collect-sub-dirs base-directories subdir-spec)
> (filter-map
> (lambda (dir)
> - (let ((directory (string-append dir subdirectory)))
> - (if (directory-exists? directory) directory #f)))
> + (and
> + (match subdir-spec
> + ((subdir) (directory-exists? (string-append dir subdir)))
> + ((subdir children)
> + (or
> + (or-map
> + (lambda (child)
> + (directory-exists? (string-append dir subdir child)))
> + children)
> + (and (eq? dir output-directory)
> + (directory-exists? (string-append dir subdir))))))
> + (string-append dir (car subdir-spec))))
> base-directories))
I’d move ‘match’ around ‘and’ to avoid ‘car’ on the next-to-last line.
(eq? dir output-directory) should probably be (string=? dir
output-directory). (‘eq?’ is pointer equality.)
I’m also not a fan of ‘or-map’.
> (filter
> (lambda (var-to-wrap) (not (null? (last var-to-wrap))))
> (map
> - (lambda (var-spec)
> - `(,(first var-spec) = ,(collect-sub-dirs base-directories (last var-spec))))
> + (match-lambda
> + ((var . subdir-spec)
> + `(,var = ,(collect-sub-dirs base-directories subdir-spec))))
> (list
> ;; these shall match the search-path-specification for Qt and KDE
> ;; libraries
> - '("XDG_DATA_DIRS" "/share")
> + '("XDG_DATA_DIRS" "/share" ("/applications" "/fonts" "/icons" "/mime"))
So the goal here is to refine what goes to XDG_DATA_DIRS, is that
correct?
Perhaps this should be separate from the patch that removes qttools from
the PATH of wrapped programs?
> (define* (wrap-all-programs #:key inputs outputs
> + (qt-wrap-excluded-inputs '("qttools"))
> (qt-wrap-excluded-outputs '())
> #:allow-other-keys)
> "Implement phase \"qt-wrap\": look for GSettings schemas and
> @@ -90,10 +106,13 @@ add a dependency of that output on Qt."
> (string-append directory "/lib/libexec"))))
>
> (define input-directories
> - ;; FIXME: Filter out unwanted inputs, e.g. cmake
> - (match inputs
> - (((_ . dir) ...)
> - dir)))
> + (filter-map
> + (match-lambda
> + ((name . dir)
> + (if (member name qt-wrap-excluded-inputs)
> + #f
> + dir)))
> + inputs))
Rather: (filter-map (match-lambda
((label . directory)
(and (member label qt-wrap-excluded-inputs)
directory)))
inputs)
Could you send an updated patch?
Thanks,
Ludo’.
Information forwarded
to
guix-patches <at> gnu.org
:
bug#42547
; Package
guix-patches
.
(Mon, 11 Jan 2021 16:22:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 42547 <at> debbugs.gnu.org (full text, mbox):
Hi,
I updated this patch together with other fixes for the qt-build service.
See http://issues.guix.gnu.org/45784 and following
TL;DR for this one:
- split refining what goes to XDG_DATA_DIRS into a separate patch
see http://issues.guix.gnu.org/45787
- most other requested changes applied, see http://issues.guix.gnu.org/45786
Ludovic Courtès <ludo <at> gnu.org> writes:
> Do you know what impact this has on the closure size of packages you
> looked at?
About 220 KB. This is roughly the size of cmake-minimal, which was
references via the env-var.
zeal before: 1420.7 MiB after : 1229.1 MiB
quassel before: 1432.5 MiB after : 1220.8 MiB
Maybe more important then the reduction of size: There are now much less
variables in the wrapper - it is much cleaner now.
> There are quite a few packages using ‘qt-build-system’. Probably a
> change for ‘staging’ or ‘core-updates’?
This might still go into staging: Approx. 175 packages use the
qt-build-system as of today. Not checked for dependencies though.
> I’m also not a fan of ‘or-map’.
Lacking an alternative (for my limited scheme knowledge) I kept this.
Regards
hartmut
bug closed, send any further explanations to
42547 <at> debbugs.gnu.org and Jakub Kądziołka <kuba <at> kadziolka.net>
Request was from
Hartmut Goebel <h.goebel <at> goebel-consult.de>
to
control <at> debbugs.gnu.org
.
(Fri, 29 Jan 2021 22:09:01 GMT)
Full text and
rfc822 format available.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Sat, 27 Feb 2021 12:24:06 GMT)
Full text and
rfc822 format available.
This bug report was last modified 3 years and 30 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.