From: Andy Wingo <wingo@igalia.com>
To: Jan Nieuwenhuizen <janneke@gnu.org>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH v5 01/10] gnu: cross: Use CROSS_*_INCLUDE_PATH for system headers.
Date: Wed, 27 Apr 2016 12:20:03 +0200 [thread overview]
Message-ID: <87inz3tkto.fsf@igalia.com> (raw)
In-Reply-To: <87lh4080wu.fsf@drakenvlieg.flower> (Jan Nieuwenhuizen's message of "Wed, 27 Apr 2016 00:23:29 +0200")
Hi :)
Great news!
Since it seems you have one more round, a couple of nits :)
On Wed 27 Apr 2016 00:23, Jan Nieuwenhuizen <janneke@gnu.org> 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
next prev parent reply other threads:[~2016-04-27 10:20 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-26 22:23 [PATCH v5 01/10] gnu: cross: Use CROSS_*_INCLUDE_PATH for system headers Jan Nieuwenhuizen
2016-04-27 3:15 ` Eric Bavier
2016-04-27 6:57 ` Jan Nieuwenhuizen
2016-04-27 10:20 ` Andy Wingo [this message]
2016-04-27 15:25 ` Jan Nieuwenhuizen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87inz3tkto.fsf@igalia.com \
--to=wingo@igalia.com \
--cc=guix-devel@gnu.org \
--cc=janneke@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).