GNU bug report logs - #28444
[PATCH 0/3] Add meson-build-system

Previous Next

Full log

View this message in rfc822 format

From: ludo <at> gnu.org (Ludovic Courtès)
To: Peter Mikkelsen <petermikkelsen10 <at> gmail.com>
Cc: 28444 <at> debbugs.gnu.org
Subject: [bug#28444] [PATCH 3/3] build-system: Add 'meson-build-system'.
Date: Fri, 15 Sep 2017 23:07:39 +0200

Peter Mikkelsen <petermikkelsen10 <at> gmail.com> skribis:

> * Makefile.am (MODULES): Add 'guix/build-system/meson.scm' and
>   'guix/build/meson-build-system.scm'.
> * guix/build-system/meson.scm: New file.
> * guix/build/meson-build-system.scm: New file.
> * doc/guix.texi (Build Systems): Add 'meson-build-system'.

Overall LGTM!  I have minor questions and comments:

> +(define* (configure #:key outputs configure-flags build-type
> +                    #:allow-other-keys)
> +  "Configure the given package"

Make sure to add a period at the end of sentences.  :-)

> +(define* (fix-runpath #:key elf-directories outputs
> +                      #:allow-other-keys)

ELF-DIRECTORIES should default to a list, probably the empty list (here
it defaults to #f, which cannot work.)

> +  "Try to make sure all ELF files in ELF-DIRECTORIES are able to find their
> +local dependencies in their RUNPATH.  Also shrink the RUNPATH to what is needed,
> +since alot of directories are left over from meson."

“a lot”

According to this description, half of it corresponds to the
‘validate-runpath’ phase, no?

The second half is the shrink-RUNPATH thing, but can you clarify why it
is needed?  Which directories in RUNPATH are “left over from meson”?

> +  (define (find-deps dep-name elf-files)
> +    "Find the directories (if any) that contains DEP-NAME.  The directories
> +searched are the ones that ELF-FILES are in."
> +    (let* ((matches (filter (lambda (file)
> +                              (string=? dep-name (basename file)))
> +                            elf-files)))
> +      (map dirname matches)))

Avoid local variable ‘matches’, to keep it concise.  Also, for inner
‘define’s, use a comment instead of a docstring (the docstring would be
inaccessible.)

> +  (define (file-needed file)
> +    "Return a list of libraries that FILE needs."
> +    (let* ((elf (call-with-input-file file
> +                  (compose parse-elf get-bytevector-all)))
> +           (dyninfo (elf-dynamic-info elf)))
> +      (if dyninfo
> +          (elf-dynamic-info-needed dyninfo)
> +          '())))
> +
> +  (define (handle-file file elf-files)
> +    "If FILE needs any libs that are part of ELF-FILES, the RUNPATH
> +is modified accordingly."
> +    (let* ((dep-dirs (apply append (map (lambda (dep-name)
> +                                          (find-deps dep-name elf-files))
> +                                        (file-needed file)))))
> +      (unless (null? dep-dirs)
> +        (augment-rpath file (string-join dep-dirs ":")))))
> +
> +  (define handle-output
> +    (match-lambda
> +              (elf-list (apply append (map (lambda (dir)
> +                                             (find-files dir elf-pred))
> +                                           excisting-elf-dirs))))

(apply append lstlst) = (concatenate lstlst)

That’s it!  Could you comment and send an updated patch?

Thanks for working on it, looks like it’s going to be useful very soon!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.