From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Nieuwenhuizen Subject: Re: [PATCH v5 01/10] gnu: cross: Use CROSS_*_INCLUDE_PATH for system headers. Date: Wed, 27 Apr 2016 17:25:16 +0200 Message-ID: <87y47z6plv.fsf@drakenvlieg.flower> References: <87lh4080wu.fsf@drakenvlieg.flower> <87inz3tkto.fsf@igalia.com> 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]:51373) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avRLL-0008Q5-El for guix-devel@gnu.org; Wed, 27 Apr 2016 11:25:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1avRLH-0005lA-Dn for guix-devel@gnu.org; Wed, 27 Apr 2016 11:25:35 -0400 In-Reply-To: <87inz3tkto.fsf@igalia.com> (Andy Wingo's message of "Wed, 27 Apr 2016 12:20:03 +0200") 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: Andy Wingo Cc: guix-devel@gnu.org Andy Wingo writes: > Since it seems you have one more round, a couple of nits :) Thanks! >> + static const char *const lang_env_vars[] =3D >> +- { "C_INCLUDE_PATH", "CPLUS_INCLUDE_PATH", >> +- "OBJC_INCLUDE_PATH", "OBJCPLUS_INCLUDE_PATH" }; >> ++ { "CROSS_C_INCLUDE_PATH", "CROSS_CPLUS_INCLUDE_PATH", >> ++ "CROSS_OBJC_INCLUDE_PATH", "CROSS_OBJCPLUS_INCLUDE_PATH" }; >> + cpp_options *cpp_opts =3D cpp_get_options (pfile); >> + size_t idx =3D (cpp_opts->objc ? 2: 0); >> +=20 > > I think this needs a comment somewhere -- you mean to completely replace > C_INCLUDE_PATH et al with CROSS_C_INCLUDE_PATH et al? If you could add > a short rationale somewhere, either in a comment or in the patch > summary, future hackers would really appreciate it :-) I get a bit lost > in these patches-to-patches. Yes. This is analogous to what the current patch does with CPATH, only for C_*INCLUDE_PATH. CROSS_CPATH, CROSS_C*_INCLUDE_PATH is to be used for cross compilers, native compilers are unchanged, of course. There were two urls at the top of the patch, I changed that to Subject: [PATCH] Search path environment variables for cross-compilers. See the discussion at which lead to the previous iteration of this patch, introducing CROSS_C= PATH, CROSS_LIBRARY_PATH and advocated the use of CPATH/CROSS_CPATH. As a concequence of the bug report http://bugs.gnu.org/22186 usage of C_INCLUDE_PATH was re-introduced. As noted in he discussion at this introduces native headers in the search path and it was decided to= keep using C_INCLUDE_PATH for system headers and introduce CROSS_C_INCLUDE_P= ATH et al., next to CROSS_CPATH to support cross compiling. >> ---- gcc-4.7.2/gcc/tlink.c 2012-02-11 09:50:23.000000000 +0100 >> -+++ gcc-4.7.2/gcc/tlink.c 2013-05-23 22:06:19.000000000 +0200 >> -@@ -461,7 +461,7 @@ recompile_files (void) >> +diff --git a/gcc/tlink.c b/gcc/tlink.c >> +index bc358b8..ad6242f 100644 >> +--- a/gcc/tlink.c >> ++++ b/gcc/tlink.c >> +@@ -458,7 +458,7 @@ recompile_files (void) >> file *f; >>=20=20=20 >> putenv (xstrdup ("COMPILER_PATH=3D")); >> @@ -34,10 +62,11 @@ at . >>=20=20=20 >> while ((f =3D file_pop ()) !=3D NULL) >> { >> - > > No change? No; only a regenerated patch. The only change is, as discussed in is adding CROSS_ variants of the C*_INCLUDE_PATH variables. >> +--=20 >> +2.1.4 >> + >> --=20 >> 2.7.3 > > Likewise? In any case you don't need two footers AFAIU. The first footer is of the gcc-patch, the second is the one in the guix patch. To avoid confusion, I have stripped the footer away from the gcc-patch. >> diff --git a/gnu/packages/cross-base.scm b/gnu/packages/cross-base.scm >> index c5bf66f..5d2d0fe 100644 >> --- a/gnu/packages/cross-base.scm >> +++ b/gnu/packages/cross-base.scm >> @@ -122,20 +128,34 @@ may be either a libc package or #f.)" >> "--disable-libquadmath" >> "--disable-decimal-float" ;would need li= bc >> "--disable-libcilkrts" >> - ))) >> + )) >> + >> + ,@(if (cross-newlib? target) >> + '("--with-newlib" >> + "--without-threads" >> + "--without-headers") >> + '())) >>=20=20 >> ,(if libc >> flags >> `(remove (cut string-match "--enable-languages.*"= <>) >> ,flags)))) >> ((#:make-flags flags) >> - (if libc >> + (cond >> + ((mingw-target? target) > > One or two lines of rationale would be appreciated here :) I have added, lik so ;; For a newlib (non-glibc) target ,@(if (cross-newlib? target) '("--with-newlib" the only thing I could explain about this snippet, and hardly needs any explanation: the newlib flag. I could not think of a good explanation for the other changes, except maybe that my first attempt did not work and I just pun in all flags that I new to work from the GUB cross build. Removed all other changes above. > Had to stop reviewing due to time. Looking v good tho! Thanks a lot again for your help. Good questions! Rebuilding, rebuilding etc; a v6 series will follow later. Greetings, Jan --=20 Jan Nieuwenhuizen | GNU LilyPond http://lilypond.org Freelance IT http://JoyofSource.com | Avatar=C2=AE http://AvatarAcademy.nl= =20=20