[bug#47006] [WIP PATCH v2 2/2] gnu: Add zig.

[Sarah Morgensen <iskarian@mgsn.dev>]

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

>> It looks like we're retaining references to the main gcc-7.5.0
>> output, but only to its "include/c++" directory, as a single
>> reference in the zig binary.  I expect this would cause issues if
>> another GCC version were later used with Zig.
> Where does that include/c++ come from?  Could it be related to the libc
> header thing we talked about in IRC?  I don't see any substitution for
> that going on either.

I think the build process for Zig might save it after auto-detecting it
as a default include path or something? In the binary, here's where it
appears:

--8<---------------cut here---------------start------------->8---
-gpsize=
GNU::Linker
/lib/i386-gnu
/../lib/
/usr/lib/
/usr/lib/../
static-lib-linker
GNU::StaticLibTool
/../../../../
/libc
/gnu/store/rn75fm7adgx3pw5j8pg3bczfqq1y17lk-gcc-7.5.0/include/c++
/../include/g++
ld-uClibc-mipsn8.so.0
ld-uClibc.so.0
ld-musl-mipsel.so.1
ld-musl-mips.so.1
/system/bin/linker
/system/bin/linker64
/lib/ld-musl-
ld-linux-riscv32-
ld-linux-riscv64-
--8<---------------cut here---------------end--------------->8---

Searching the Clang source yields a const reference in
'lib/Driver/ToolChains/Linux.cpp' at line 406:

--8<---------------cut here---------------start------------->8---
  const StringRef InstallDir = GCCInstallation.getInstallPath();
  const StringRef TripleStr = GCCInstallation.getTriple().str();
  const Multilib &Multilib = GCCInstallation.getMultilib();

  std::string Path =
      (InstallDir + "/../../../../" + TripleStr + "/libc" + Multilib.osSuffix())
          .str();
--8<---------------cut here---------------end--------------->8---

And I just searched the Clang input for this reference--that exact path
with the exact same context as above is present in libclang,
libclangDriver, and everything that gets linked to them.  So it looks
like this one we can blame on Clang as well.

>> We're retaining double direct references to gcc-7.5.0-lib, and double
>> indirect references to zlib-1.2.11.  This seems to come from clang.
>> (Wow, clang is big. Maybe it should have a separate "lib"
>> output?  Zig, for example, only needs libclang to build, not the
>> binaries.)
>> 
>> Other than those items, it looks good!
> According to `guix refresh', there's only two dependents of Clang 12,
> but since we'd have to rebuild all clangs, that might go into core-
> updates territory.  We might also want to check with an expert on
> X/Mesa, since that seems to be one of our biggest clang dependents.

Yeah, Clang is an absolute beast to mess with, so I don't think we
should let this be a blocker for Zig.  I'll submit a bug report to see
if I can summon the experts, though.

>> +       (patches
>> +        (search-patches
>> +         "zig-disable-libc-note-test.patch"
> Is this test really necessary to skip that test?  If not, let's try to
> use the command line for that.

We could use "-Dskip-compile-errors", but that also skips ~600 other
test cases.

>> +         ;; XXX: Remove the following patch when updating LLVM to
>> 12.0.1.
>> +         "zig-disable-MIPS-tests.patch"
> There's a patch for LLVM 12.0.1 waiting in the ML and we could
> potentially bump lld to 12.0.1 regardless.  (Can LLVM components be
> mix-matched like that?)

I have no idea if they *can*, but I'm pretty sure they're not intended
to be, as LLVM wants you to build everything together in one big blob.

>> +         "zig-fix-cross-native-execution.patch"
> IIUC this is weaker than "-Dskip-non-native".  Is there a reason to
> include specifically these non-native tests?

Yes, as I wrote in the patch header, it fixes the behavior of 'zig run'
and 'zig test' when the target is cross-native.  Would we want to stick
to upstream, even if it's buggy?

We might want to add "-Dskip-non-native" anyway as it speeds up tests by
an order of magnitude, in which case tests will succeed without the
patch.  It looks their CI uses it "-Dskip-non-native" as well, and I
suppose there's not a whole lot Guix can do to mess up Zig's
cross-compiling anyway, since it's pretty self-contained...

>> +    (native-search-paths
>> +     (list
>> +      (search-path-specification
>> +       (variable "ZIG_INCLUDE_DIRS")
>> +       ;; XXX: It doesn't seem as though Zig can distinguish between
>> C and C++
>> +       ;;      include paths, so provide both.
>> +       (files '("include/c++" "include")))
>> +      (search-path-specification
>> +       ;; TODO: Might be confused with "ZIG_LIB_DIR"... Maybe use
>> +       ;;       "ZIG_INCLUDE_PATH" and "ZIG_LIBRARY_PATH"?
>> +       (variable "ZIG_LIB_DIRS")
>> +       (files '("lib" "lib64")))))
> You can rewrite "zig-use-explicit-paths.patch" in-place with Emacs'
> query-replace and/or sed (or even just manually, there are no lines to
> add or remove) if you disagree with my environment variable naming
> choice.  Just make sure you don't accidentally break diff by deleting
> trailing space.

> Another potential naming choice would be to prefix everything with
> ZIG_LIBC_ rather than simply ZIG_.  Of course I thought about that only
> after sending my previous mail ^^"

Ah, I meant to mention it in my last e-mail but I forgot.  I didn't want
to just go changing it on you without discussing it.

As far as I can tell, there's no such thing as a "Zig library" or a "Zig
header"; these are for including system C headers and libraries.  So, I
would just go with LIBRARY_PATH and CPLUS_INCLUDE_PATH unless we
anticipate needing to tell Zig something different than what we tell
GCC/Clang.  Furthermore, the in-development 'zig cc' command is intended
to be a drop-in replacement for GCC/Clang, so it should probably honor
the same environment variables.

--
Sarah