From: Jan Nieuwenhuizen <janneke@gnu.org>
To: Andy Wingo <wingo@igalia.com>
Cc: guix-devel@gnu.org
Subject: Re: [PATCH v4 1/9] gnu: cross: Use CROSS_*_INCLUDE_PATH for system headers.
Date: Tue, 26 Apr 2016 10:37:40 +0200 [thread overview]
Message-ID: <87shy8ydd7.fsf@drakenvlieg.flower> (raw)
In-Reply-To: <87oa8wx0jg.fsf@igalia.com> (Andy Wingo's message of "Tue, 26 Apr 2016 10:00:03 +0200")
Andy Wingo writes:
> Well, you had two versions there, and the second one that just used let*
> looked fine to me. Dunno. I find that the structure imposed by
> and-let* tends to contort things in a way I don't want -- sometimes it's
> only some of the bindings that I want to test for truthiness. I find
> myself fighting and-let* more than enjoying it. Anyway. It is a very
> minor point and I don't think it matters here :)
Okay.
>> - (let ((libc (assoc-ref inputs "libc"))
>> + (let ((libc (assoc-ref inputs libc))
>> (linux (assoc-ref inputs "xlinux-headers")))
>
> FYI while I usually don't vertically line up subexpressions, it would
> appear to be the guix style ;) So this edit undoes some valid code. I
> don't think it matters though.
There's something wrong here that I keep correcting, I'll have a look.
I think it should be "libc" ... I have wondered about a patch where I
change libc to "libc" -- how did that ever work? Apparently, I broke it
here.
> OK so this is literally a patch in a patch and it gets complicated to
> review :) But cool, I had a question about one piece:
What I did was import gcc-4.9.3 into git, applied the original
gcc-cross-environment-variables.patch from Guix, made some changes and
used git to produce a new gcc-cross-environment-variables.patch.
I'm not too happy with this, maybe I should look into making the guix
patch-in-patch nicer? Anyway, here inline is the actual diff between
the original gcc-cross-environment-variables.patch and the new one:
diff --git a/gcc/incpath.c b/gcc/incpath.c
index 3a34998..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);
diff --git a/gcc/tlink.c b/gcc/tlink.c
index 65d4a84..ad6242f 100644
--- a/gcc/tlink.c
+++ b/gcc/tlink.c
@@ -458,7 +458,7 @@ recompile_files (void)
file *f;
putenv (xstrdup ("COMPILER_PATH="));
- putenv (xstrdup (LIBRARY_PATH_ENV "="));
+ putenv (xstrdup ("LIBRARY_PATH_ENV="));
while ((f = file_pop ()) != NULL)
{
> The Scheme parts LGTM; would you mind giving details about the GCC
> patch?
You spotted the putenv problem, quite visible above; I'm not sure why
this still works for me, I'll fix (i.e., revert) this.
As to your other two questions about if (temp): those are actually not
changes wrt the current gcc-cross-environment-variables.patch.
>> +- if (temp && *cross_compile == '0')
>> ++ if (temp)
>> + {
>> + const char *startp, *endp;
>> + char *nstore = (char *) alloca (strlen (temp) + 3);
>
> Why this change?
Not a change per se, AIUI our cross patch makes COMPILER_PATH now
valid, as we populate it from CROSS_*.
>> +#define LIBRARY_PATH_ENV "CROSS_LIBRARY_PATH"
>> +
>> #endif /* ! GCC_SYSTEM_H */
>> -
>
> I wasn't quite able to understand this bit.
>> - putenv (xstrdup ("LIBRARY_PATH="));
>> -+ putenv (xstrdup (LIBRARY_PATH_ENV "="));
>> ++ putenv (xstrdup ("LIBRARY_PATH_ENV="));
>>
>> while ((f = file_pop ()) != NULL)
>> {
>
> Surely this is incorrect?
Oops yes, this is wrong. Thanks
>> -- if (temp && *cross_compile == '0')
>> -+ if (temp)
>> - {
>> - const char *startp, *endp;
>> - char *nstore = (char *) alloca (strlen (temp) + 3);
>> --
>> 2.7.3
>
> Similar comment as above.
Not a change per se, AIUI our cross patch makes LIBRARY_PATH[_ENV] now
valid, as we populate it from CROSS_LIBRARY_PATH.
Thanks again, greetings,
Jan
--
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.nl
next prev parent reply other threads:[~2016-04-26 8:38 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-26 17:38 rfc/rfh: i686-w64-mingw32 cross target Jan Nieuwenhuizen
2016-03-27 14:20 ` [RFCv2] build: i686-w64-mingw32: new " Jan Nieuwenhuizen
2016-03-27 16:17 ` Pjotr Prins
2016-03-31 20:16 ` rfc/rfh: i686-w64-mingw32 " Ricardo Wurmus
2016-03-31 20:26 ` Jan Nieuwenhuizen
2016-03-31 20:48 ` Ricardo Wurmus
2016-04-02 7:30 ` Jan Nieuwenhuizen
2016-04-07 21:12 ` Ludovic Courtès
2016-04-14 6:30 ` Jan Nieuwenhuizen
2016-04-19 14:54 ` Ludovic Courtès
2016-04-24 21:40 ` [PATCH v4 1/9] gnu: cross: Use CROSS_*_INCLUDE_PATH for system headers Jan Nieuwenhuizen
2016-04-25 10:38 ` Andy Wingo
2016-04-25 11:28 ` Jan Nieuwenhuizen
2016-04-26 8:00 ` Andy Wingo
2016-04-26 8:37 ` Jan Nieuwenhuizen [this message]
2016-04-26 9:03 ` Jan Nieuwenhuizen
2016-04-25 22:06 ` Jan Nieuwenhuizen
2016-04-26 9:02 ` Andy Wingo
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=87shy8ydd7.fsf@drakenvlieg.flower \
--to=janneke@gnu.org \
--cc=guix-devel@gnu.org \
--cc=wingo@igalia.com \
/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).