Re: [PATCH v5 01/10] gnu: cross: Use CROSS_*_INCLUDE_PATH for system headers.

[Andy Wingo]

Hi :)

Great news!

Since it seems you have one more round, a couple of nits :)

On Wed 27 Apr 2016 00:23, Jan Nieuwenhuizen <address@hidden> 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 <http://gcc.gnu.org/ml/gcc/2013-02/msg00124.html>.
>   
>     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 <http://gcc.gnu.org/ml/gcc/2013-02/msg00124.html>.
>       {
>         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