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 <jann...@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.nl
