From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH 2/5] gnu: Add avr-gcc. Date: Thu, 14 Apr 2016 19:25:14 +0200 Message-ID: <87y48gje6d.fsf@gnu.org> References: <1460639824-9976-1-git-send-email-dthompson2@worcester.edu> <1460639824-9976-3-git-send-email-dthompson2@worcester.edu> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:42252) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aql18-0002ad-Pc for guix-devel@gnu.org; Thu, 14 Apr 2016 13:25:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aql13-0001hD-PS for guix-devel@gnu.org; Thu, 14 Apr 2016 13:25:22 -0400 In-Reply-To: <1460639824-9976-3-git-send-email-dthompson2@worcester.edu> (David Thompson's message of "Thu, 14 Apr 2016 09:17:01 -0400") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: David Thompson Cc: guix-devel@gnu.org David Thompson skribis: > * gnu/packages/avr.scm (avr-gcc): New variable. [...] > + `(modify-phases ,phases > + ;; Without a working multilib build, the resulting GCC lacks > + ;; support for nearly every AVR chip. > + (add-after 'unpack 'fix-genmultilib > + (lambda _ > + (substitute* "gcc/genmultilib" > + (("#!/bin/sh") (string-append "#!" (which "sh")))) Just: (patch-shebang "gcc/genmultilib"). I think the reason this file is not automatically patched during the =E2=80=98patch-shebangs=E2=80=99 phase is because it does not have the exec= utable bit. Would be worth mentioning in a comment IMO. What=E2=80=99s unclear, though, is why the invalid shebang is a problem at = all given that this file is not executable anyway. Thoughts? > + ((#:configure-flags flags) > + '(list "--target=3Davr" > + "--enable-languages=3Dc,c++" > + "--disable-nls" > + "--disable-libssp" > + "--with-dwarf2")))) I think we should minimize target-specific changes and justify them in a comment when they=E2=80=99re unavoidable. Here, I think we can safely remove --target and --disable-nls. --disable-libssp and --enable-languages are already in =E2=80=98cross-gcc-arguments=E2=80=99, so that leaves us with just --with-d= warf2, IIUC. Why is it needed? > + (native-search-paths > + (list (search-path-specification > + (variable "CROSS_CPATH") > + (files '("avr/include"))) > + (search-path-specification > + (variable "CROSS_LIBRARY_PATH") > + (files '("avr/lib")))))))) That these go in =E2=80=98native-search-paths=E2=80=99 feels wrong. But I think it=E2=80=99s because we=E2=80=99re trying to build avr-libc lik= e a =E2=80=9Cnormal=E2=80=9D package with a cross-toolchain as its input. Instead, the =E2=80=9Cintended use=E2=80=9D is that libc is treated special= ly as in =E2=80=98cross-libc=E2=80=99 in cross-base.scm. Probably we need to: 1. Remove #:configure-flags and =E2=80=98native-inputs=E2=80=99 from the = =E2=80=98avr-libc=E2=80=99 package. 2. Add (supported-systems '()) or similar to the =E2=80=98avr-libc=E2=80= =99 package. 3. Use something similar to =E2=80=98cross-libc=E2=80=99 but that uses av= r-libc instead of glibc in cross-base.scm, thus setting CROSS_CPATH and CROSS_LIBRARY_PATH appropriately. WDYT? Apologies for spoiling the party. ;-) This is clearly a tricky/inelegant area. Thanks, Ludo=E2=80=99.