[bug#42547] [PATCH] build-system/qt: Don't include useless inputs in wrapped variables.

[Ludovic Courtès]

Hi Jakub,

Jakub Kądziołka <kuba@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’.