Andy Wingo writes:
> Since it seems you have one more round, a couple of nits :)
Thanks!
>> + 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.
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
<http://gcc.gnu.org/ml/gcc/2013-02/msg00124.html>
which lead to the previous iteration of this patch, introducing CROSS_CPATH,
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
<https://lists.gnu.org/archive/html/guix-devel/2016-04/msg00533.html>
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_PATH
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;
>>
>> 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?
No; only a regenerated patch. The only change is, as discussed in
<https://lists.gnu.org/archive/html/guix-devel/2016-04/msg00533.html>
is adding CROSS_ variants of the C*_INCLUDE_PATH variables.
>> +--
>> +2.1.4
>> +
>> --
>> 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 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 :)
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
--
Jan Nieuwenhuizen <jann...@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.nl
