From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Wingo Subject: Re: [PATCH v5 01/10] gnu: cross: Use CROSS_*_INCLUDE_PATH for system headers. Date: Wed, 27 Apr 2016 12:20:03 +0200 Message-ID: <87inz3tkto.fsf@igalia.com> References: <87lh4080wu.fsf@drakenvlieg.flower> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:52665) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1avMaP-0007El-4D for guix-devel@gnu.org; Wed, 27 Apr 2016 06:20:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1avMaK-0000Cr-Uh for guix-devel@gnu.org; Wed, 27 Apr 2016 06:20:49 -0400 In-Reply-To: <87lh4080wu.fsf@drakenvlieg.flower> (Jan Nieuwenhuizen's message of "Wed, 27 Apr 2016 00:23:29 +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: Jan Nieuwenhuizen Cc: guix-devel@gnu.org Hi :) Great news! Since it seems you have one more round, a couple of nits :) On Wed 27 Apr 2016 00:23, Jan Nieuwenhuizen writes: > +diff --git a/gcc/incpath.c b/gcc/incpath.c > +index f495c0a..ba12249 100644 > +--- a/gcc/incpath.c > ++++ b/gcc/incpath.c > +@@ -461,8 +461,8 @@ register_include_chains (cpp_reader *pfile, const char *sysroot, > + int stdinc, int cxx_stdinc, int verbose) > + { > + static const char *const lang_env_vars[] = > +- { "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 = cpp_get_options (pfile); > + size_t idx = (cpp_opts->objc ? 2: 0); > + 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. > ---- 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; > > putenv (xstrdup ("COMPILER_PATH=")); > @@ -34,10 +62,11 @@ at . > > while ((f = file_pop ()) != NULL) > { > - No change? > ---- gcc-4.7.3/gcc/gcc.c 2013-03-08 08:25:09.000000000 +0100 > -+++ gcc-4.7.3/gcc/gcc.c 2013-05-24 08:58:16.000000000 +0200 > -@@ -3726,7 +3726,7 @@ process_command (unsigned int decoded_op > +diff --git a/gcc/gcc.c b/gcc/gcc.c > +index adbf0c4..70448c6 100644 > +--- a/gcc/gcc.c > ++++ b/gcc/gcc.c > +@@ -3853,7 +3853,7 @@ process_command (unsigned int decoded_options_count, > } > > temp = getenv (LIBRARY_PATH_ENV); > @@ -46,3 +75,6 @@ at . > { > const char *startp, *endp; > char *nstore = (char *) alloca (strlen (temp) + 3); > +-- > +2.1.4 > + > -- > 2.7.3 Likewise? In any case you don't need two footers AFAIU. > 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 libc > "--disable-libcilkrts" > - ))) > + )) > + > + ,@(if (cross-newlib? target) > + '("--with-newlib" > + "--without-threads" > + "--without-headers") > + '())) > > ,(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 :) Had to stop reviewing due to time. Looking v good tho! A