From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39095) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dsxqF-0001Tk-VO for guix-patches@gnu.org; Fri, 15 Sep 2017 17:08:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dsxqE-0000Ts-1w for guix-patches@gnu.org; Fri, 15 Sep 2017 17:08:03 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:34483) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dsxqD-0000TT-VM for guix-patches@gnu.org; Fri, 15 Sep 2017 17:08:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1dsxqD-0005MA-Q8 for guix-patches@gnu.org; Fri, 15 Sep 2017 17:08:01 -0400 Subject: [bug#28444] [PATCH 3/3] build-system: Add 'meson-build-system'. Resent-Message-ID: From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) References: <20170913125003.13313-1-petermikkelsen10@gmail.com> <20170913125003.13313-3-petermikkelsen10@gmail.com> Date: Fri, 15 Sep 2017 23:07:39 +0200 In-Reply-To: <20170913125003.13313-3-petermikkelsen10@gmail.com> (Peter Mikkelsen's message of "Wed, 13 Sep 2017 14:50:03 +0200") Message-ID: <87a81vllgk.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Peter Mikkelsen Cc: 28444@debbugs.gnu.org Peter Mikkelsen 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 th= eir > +local dependencies in their RUNPATH. Also shrink the RUNPATH to what is= needed, > +since alot of directories are left over from meson." =E2=80=9Ca lot=E2=80=9D According to this description, half of it corresponds to the =E2=80=98validate-runpath=E2=80=99 phase, no? The second half is the shrink-RUNPATH thing, but can you clarify why it is needed? Which directories in RUNPATH are =E2=80=9Cleft over from meson= =E2=80=9D? > + (define (find-deps dep-name elf-files) > + "Find the directories (if any) that contains DEP-NAME. The director= ies > +searched are the ones that ELF-FILES are in." > + (let* ((matches (filter (lambda (file) > + (string=3D? dep-name (basename file))) > + elf-files))) > + (map dirname matches))) Avoid local variable =E2=80=98matches=E2=80=99, to keep it concise. Also, = for inner =E2=80=98define=E2=80=99s, use a comment instead of a docstring (the docstr= ing 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) =3D (concatenate lstlst) That=E2=80=99s it! Could you comment and send an updated patch? Thanks for working on it, looks like it=E2=80=99s going to be useful very s= oon! Ludo=E2=80=99.