Subject: [PATCH bazel-zig-cc 0/2] use relative paths

From: ~motiejus <[email-masked]>
Date: Tue, 27 Sep 2022 07:24:28 +0000

This patchset changes absolute paths to tools to relative paths. Work
was started by @kmicklas and completed by myself and @laurynasl. Please
review.

Currently, with absolute paths, bazel's remote cache keys are different
between two workspaces, busting our remote cache. We hope (did not try
yet) that this will normalize them and fix our cache hit ratios.

Fabian, this does not work on Windows yet; if you want to chime in, I
will wait a couple of days for your patch before I cut a release.

1.0 is coming soon.

Ken Micklas (1):
  WIP: use relative paths for tools

Motiejus Jakštys (1):
  defs: fix import paths

 README.md                           |  2 +-
 toolchain/BUILD.sdk.bazel           |  9 +++++++--
 toolchain/defs.bzl                  | 29 +++++++++++++++++++++++++----
 toolchain/private/BUILD.sdk.bazel   |  7 -------
 toolchain/private/cc_toolchains.bzl | 17 ++++++++---------
 toolchain/private/defs.bzl          | 29 ++++++++++++++---------------
 toolchain/toolchain/defs.bzl        |  4 ++--
 7 files changed, 57 insertions(+), 40 deletions(-)
 delete mode 100644 toolchain/private/BUILD.sdk.bazel

-- 
2.34.4

---

From: ~motiejus <[email-masked]>
Date: Wed, 21 Sep 2022 11:14:29 +0100

From: Ken Micklas <[email-masked]>

---
 README.md                           |  2 +-
 toolchain/BUILD.sdk.bazel           |  9 +++++++--
 toolchain/defs.bzl                  |  5 +++--
 toolchain/private/BUILD.sdk.bazel   |  7 -------
 toolchain/private/cc_toolchains.bzl | 12 +++++++-----
 toolchain/toolchain/defs.bzl        |  4 ++--
 6 files changed, 20 insertions(+), 19 deletions(-)
 delete mode 100644 toolchain/private/BUILD.sdk.bazel

diff --git a/README.md b/README.md
index 262b88b..266952f 100644
--- a/README.md
+++ b/README.md
@@ -110,7 +110,7 @@ toolchain(
   generator_location = "toolchain/BUILD:7:19",
   toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
   target_compatible_with = ["@platforms//os:linux", "@platforms//cpu:aarch64", "@zig_sdk//libc:unconstrained"],
-  toolchain = "@zig_sdk//private:aarch64-linux-musl_cc",
+  toolchain = "@zig_sdk//:aarch64-linux-musl_cc",
 )
 ```
 
diff --git a/toolchain/BUILD.sdk.bazel b/toolchain/BUILD.sdk.bazel
index e1505b1..ef5fd5f 100644
--- a/toolchain/BUILD.sdk.bazel
+++ b/toolchain/BUILD.sdk.bazel
@@ -1,11 +1,10 @@
 load("@bazel-zig-cc//toolchain:defs.bzl", "declare_files")
+load("@bazel-zig-cc//toolchain/private:cc_toolchains.bzl", "declare_cc_toolchains")
 
 package(
     default_visibility = ["//visibility:public"],
 )
 
-
-
 declare_files(
     os = {os},
     zig_include_root = {zig_include_root},
@@ -15,3 +14,9 @@ exports_files([
     "glibc-hacks/fcntl.map",
     "glibc-hacks/glibchack-fcntl.h",
 ])
+
+declare_cc_toolchains(
+    os = {os},
+    absolute_path = {absolute_path},
+    zig_include_root = {zig_include_root},
+)
diff --git a/toolchain/defs.bzl b/toolchain/defs.bzl
index 797e4fe..f0e6a72 100644
--- a/toolchain/defs.bzl
+++ b/toolchain/defs.bzl
@@ -186,14 +186,14 @@ def _zig_repository_impl(repository_ctx):
 
     for dest, src in {
         "BUILD": "//toolchain:BUILD.sdk.bazel",
-        "private/BUILD": "//toolchain/private:BUILD.sdk.bazel",
+        # "private/BUILD": "//toolchain/private:BUILD.sdk.bazel",
     }.items():
         repository_ctx.template(
             dest,
             Label(src),
             executable = False,
             substitutions = {
-                "{absolute_path}": _quote(str(repository_ctx.path(""))),
+                "{absolute_path}": _quote("external/zig_sdk"),
                 "{os}": _quote(os),
                 "{zig_include_root}": _quote(zig_include_root),
             },
@@ -246,6 +246,7 @@ def filegroup(name, **kwargs):
     return ":" + name
 
 def declare_files(os, zig_include_root):
+    filegroup(name = "all", srcs = native.glob(["**"]))
     filegroup(name = "empty")
     if os == "windows":
         native.exports_files(["zig.exe"], visibility = ["//visibility:public"])
diff --git a/toolchain/private/BUILD.sdk.bazel b/toolchain/private/BUILD.sdk.bazel
deleted file mode 100644
index 3e7c23e..0000000
--- a/toolchain/private/BUILD.sdk.bazel
+++ /dev/null
@@ -1,7 +0,0 @@
-load("@bazel-zig-cc//toolchain/private:cc_toolchains.bzl", "declare_cc_toolchains")
-
-declare_cc_toolchains(
-    os = {os},
-    absolute_path = {absolute_path},
-    zig_include_root = {zig_include_root},
-)
diff --git a/toolchain/private/cc_toolchains.bzl b/toolchain/private/cc_toolchains.bzl
index 724cbda..6b8b3e6 100644
--- a/toolchain/private/cc_toolchains.bzl
+++ b/toolchain/private/cc_toolchains.bzl
@@ -29,7 +29,7 @@ def declare_cc_toolchains(os, absolute_path, zig_include_root):
                 absolute_tool_paths[name] = path
                 continue
             tool_path = zig_tool_path(os).format(zig_tool = path)
-            absolute_tool_paths[name] = "%s/%s" % (absolute_path, tool_path)
+            absolute_tool_paths[name] = tool_path
 
         linkopts = target_config.linkopts
         dynamic_library_linkopts = target_config.dynamic_library_linkopts
@@ -53,18 +53,20 @@ def declare_cc_toolchains(os, absolute_path, zig_include_root):
             compiler = "clang",
             abi_version = "unknown",
             abi_libc_version = "unknown",
+            # visibility = ["//visibility:private"],
         )
 
         native.cc_toolchain(
             name = zigtarget + "_cc",
             toolchain_identifier = zigtarget + "-toolchain",
             toolchain_config = ":%s_cc_config" % zigtarget,
-            all_files = "@zig_sdk//:zig",
-            ar_files = "@zig_sdk//:zig",
-            compiler_files = "@zig_sdk//:zig",
-            linker_files = "@zig_sdk//:zig",
+            all_files = "@zig_sdk//:all",
+            ar_files = "@zig_sdk//:all",
+            compiler_files = "@zig_sdk//:all",
+            linker_files = "@zig_sdk//:all",
             dwp_files = "@zig_sdk//:empty",
             objcopy_files = "@zig_sdk//:empty",
             strip_files = "@zig_sdk//:empty",
             supports_param_files = 0,
+            # visibility = ["//visibility:private"],
         )
diff --git a/toolchain/toolchain/defs.bzl b/toolchain/toolchain/defs.bzl
index 1d99a61..24a90cc 100644
--- a/toolchain/toolchain/defs.bzl
+++ b/toolchain/toolchain/defs.bzl
@@ -32,7 +32,7 @@ def _declare_toolchain(gotarget, zigtarget, target_compatible_with):
         name = gotarget,
         exec_compatible_with = None,
         target_compatible_with = target_compatible_with,
-        toolchain = "@zig_sdk//private:%s_cc" % zigtarget,
+        toolchain = "@zig_sdk//:%s_cc" % zigtarget,
         toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
     )
 
@@ -41,6 +41,6 @@ def _declare_toolchain(gotarget, zigtarget, target_compatible_with):
         name = zigtarget,
         exec_compatible_with = None,
         target_compatible_with = target_compatible_with,
-        toolchain = "@zig_sdk//private:%s_cc" % zigtarget,
+        toolchain = "@zig_sdk//:%s_cc" % zigtarget,
         toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
     )
-- 
2.34.4

---

From: ~motiejus <[email-masked]>
Date: Mon, 26 Sep 2022 10:14:08 +0300

From: Motiejus Jakštys <[email-masked]>

Empirically these need to come from most specfic to least specific.

The error message is as follows:

    In file included from test/c/main.c:1:
    In file included from external/zig_sdk/lib/libcxx/include/stdio.h:107:
    In file included from external/zig_sdk/lib/libc/include/generic-glibc/stdio.h:38:
    external/zig_sdk/lib/libc/include/generic-glibc/bits/types.h:139:3: error:
    # error
      ^
    external/zig_sdk/lib/libc/include/generic-glibc/bits/types.h:145:1: error: unknown type name '__STD_TYPE'
    __STD_TYPE __DEV_T_TYPE __dev_t;        /* Type of device numbers.  */

Dissected `generic-glibc/bits/types.h:#error`:

    #if __WORDSIZE == 32
    <...>
    # define __STD_TYPE		__extension__ typedef
    #elif __WORDSIZE == 64
    <...>
    # define __STD_TYPE		typedef
    #else
    # error
    #endif

So we do not have the `__WORDSIZE`. Where does that come from? Probably from a
directory that has an `x86_64` in it. How does that get included? Let's start
with `lib/libcxx/include/stdio.h`:

	16 #include_next <stdio.h>

Now previously our `c++` command line looked like this:
    external/zig_sdk/tools/c++ \
    <...>
    -Iexternal/zig_sdk/lib/include \
    -Iexternal/zig_sdk/lib/libcxx/include \
    -Iexternal/zig_sdk/lib/libcxxabi/include \
    -Iexternal/zig_sdk/lib/libunwind/include \
    -Iexternal/zig_sdk/lib/libc/include/generic-glibc \
    -Iexternal/zig_sdk/lib/libc/include/any-linux-any \
    -Iexternal/zig_sdk/lib/libc/include/x86_64-linux-gnu \
    -Iexternal/zig_sdk/lib/libc/include/x86_64-linux-any \
    -Iexternal/zig_sdk/lib/libc/include/x86-linux-any \
    -Iexternal/zig_sdk/glibc-hacks \
    <...>

So the next place it will find `stdio.h` is in `generic-glibc`, which already
uses the `__WORDSIZE`. If we make the "next" include to be the arch-specific
one instead of the generic-glibc, things start compiling again.

Fix the same fo musl.
---
 toolchain/defs.bzl                  | 24 ++++++++++++++++++++++--
 toolchain/private/cc_toolchains.bzl |  9 +++------
 toolchain/private/defs.bzl          | 29 ++++++++++++++---------------
 3 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/toolchain/defs.bzl b/toolchain/defs.bzl
index f0e6a72..25ca20a 100644
--- a/toolchain/defs.bzl
+++ b/toolchain/defs.bzl
@@ -1,6 +1,13 @@
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
 load("@bazel_tools//tools/build_defs/repo:utils.bzl", "read_user_netrc", "use_netrc")
-load("@bazel-zig-cc//toolchain/private:defs.bzl", "DEFAULT_INCLUDE_DIRECTORIES", "target_structs", "zig_tool_path")
+load("@bazel-zig-cc//toolchain/private:defs.bzl", "target_structs", "zig_tool_path")
+
+# Directories that `zig c++` includes behind the scenes.
+_DEFAULT_INCLUDE_DIRECTORIES = [
+    "libcxx/include",
+    "libcxxabi/include",
+    "libunwind/include",
+]
 
 _fcntl_map = """
 GLIBC_2.2.5 {
@@ -86,12 +93,14 @@ _ZIG_TOOLS = [
     "wasm-ld",  # WebAssembly
 ]
 
+# TODO: ZIG_LIB_DIR equivalent in powershell?
 _ZIG_TOOL_WRAPPER_WINDOWS_CACHE_KNOWN = """@echo off
 set ZIG_LOCAL_CACHE_DIR={cache_prefix}\\bazel-zig-cc
 set ZIG_GLOBAL_CACHE_DIR=%ZIG_LOCAL_CACHE_DIR%
 "{zig}" "{zig_tool}" %*
 """
 
+# TODO: ZIG_LIB_DIR equivalent in powershell?
 _ZIG_TOOL_WRAPPER_WINDOWS_CACHE_GUESS = """@echo off
 if exist "%TMP%\\*" goto :usertmp
 set ZIG_LOCAL_CACHE_DIR=C:\\Temp\\bazel-zig-cc
@@ -104,6 +113,12 @@ set ZIG_GLOBAL_CACHE_DIR=%ZIG_LOCAL_CACHE_DIR%
 """
 
 _ZIG_TOOL_WRAPPER_CACHE_KNOWN = """#!/bin/sh
+set -e
+if [ -d external/zig_sdk/lib ]; then
+    export ZIG_LIB_DIR=external/zig_sdk/lib
+else
+    export ZIG_LIB_DIR="$(dirname "$0")/../lib"
+fi
 export ZIG_LOCAL_CACHE_DIR="{cache_prefix}/bazel-zig-cc"
 export ZIG_GLOBAL_CACHE_DIR="{cache_prefix}/bazel-zig-cc"
 exec "{zig}" "{zig_tool}" "$@"
@@ -111,6 +126,11 @@ exec "{zig}" "{zig_tool}" "$@"
 
 _ZIG_TOOL_WRAPPER_CACHE_GUESS = """#!/bin/sh
 set -e
+if [ -d external/zig_sdk/lib ]; then
+    export ZIG_LIB_DIR=external/zig_sdk/lib
+else
+    export ZIG_LIB_DIR="$(dirname "$0")/../lib"
+fi
 if [ -n "$TMPDIR" ]; then
     _cache_prefix=$TMPDIR
 elif [ -n "$HOME" ]; then
@@ -258,7 +278,7 @@ def declare_files(os, zig_include_root):
     lazy_filegroups = {}
 
     for target_config in target_structs():
-        for d in DEFAULT_INCLUDE_DIRECTORIES + target_config.includes:
+        for d in _DEFAULT_INCLUDE_DIRECTORIES + target_config.includes:
             d = zig_include_root + d
             if d not in lazy_filegroups:
                 lazy_filegroups[d] = filegroup(name = d, srcs = native.glob([d + "/**"]))
diff --git a/toolchain/private/cc_toolchains.bzl b/toolchain/private/cc_toolchains.bzl
index 6b8b3e6..a5b4b5e 100644
--- a/toolchain/private/cc_toolchains.bzl
+++ b/toolchain/private/cc_toolchains.bzl
@@ -1,4 +1,4 @@
-load(":defs.bzl", "DEFAULT_INCLUDE_DIRECTORIES", "target_structs", "zig_tool_path")
+load(":defs.bzl", "target_structs", "zig_tool_path")
 load("@bazel-zig-cc//toolchain:zig_toolchain.bzl", "zig_cc_toolchain_config")
 
 DEFAULT_TOOL_PATHS = {
@@ -17,9 +17,6 @@ def declare_cc_toolchains(os, absolute_path, zig_include_root):
         zigtarget = target_config.zigtarget
 
         cxx_builtin_include_directories = []
-        for d in DEFAULT_INCLUDE_DIRECTORIES + target_config.includes:
-            d = zig_include_root + d
-            cxx_builtin_include_directories.append(absolute_path + "/" + d)
         for d in getattr(target_config, "toplevel_include", []):
             cxx_builtin_include_directories.append(absolute_path + "/" + d)
 
@@ -53,7 +50,7 @@ def declare_cc_toolchains(os, absolute_path, zig_include_root):
             compiler = "clang",
             abi_version = "unknown",
             abi_libc_version = "unknown",
-            # visibility = ["//visibility:private"],
+            visibility = ["//visibility:private"],
         )
 
         native.cc_toolchain(
@@ -68,5 +65,5 @@ def declare_cc_toolchains(os, absolute_path, zig_include_root):
             objcopy_files = "@zig_sdk//:empty",
             strip_files = "@zig_sdk//:empty",
             supports_param_files = 0,
-            # visibility = ["//visibility:private"],
+            visibility = ["//visibility:private"],
         )
diff --git a/toolchain/private/defs.bzl b/toolchain/private/defs.bzl
index 8a8b4f5..2e939fd 100644
--- a/toolchain/private/defs.bzl
+++ b/toolchain/private/defs.bzl
@@ -1,9 +1,3 @@
-DEFAULT_INCLUDE_DIRECTORIES = [
-    "include",
-    "libcxx/include",
-    "libcxxabi/include",
-]
-
 _ZIG_TOOL_PATH = "tools/{zig_tool}"
 
 # Zig supports even older glibcs than defined below, but we have tested only
@@ -101,12 +95,14 @@ def _target_linux_gnu(gocpu, zigcpu, glibc_version):
         gotarget = "linux_{}_{}".format(gocpu, glibc_suffix),
         zigtarget = "{}-linux-{}".format(zigcpu, glibc_suffix),
         includes = [
-            "libunwind/include",
-            "libc/include/generic-glibc",
+                       "libc/include/{}-linux-gnu".format(zigcpu),
+                       "libc/include/generic-glibc",
+                   ] +
+                   # x86_64-linux-any is x86_64-linux and x86-linux combined.
+                   (["libc/include/x86-linux-any"] if zigcpu == "x86_64" else []) +
+                   (["libc/include/{}-linux-any".format(zigcpu)] if zigcpu != "x86_64" else []) + [
             "libc/include/any-linux-any",
-            "libc/include/{}-linux-gnu".format(zigcpu),
-            "libc/include/{}-linux-any".format(zigcpu),
-        ] + (["libc/include/x86-linux-any"] if zigcpu == "x86_64" else []),
+        ],
         toplevel_include = ["glibc-hacks"] if fcntl_hack else [],
         compiler_extra_includes = ["glibc-hacks/glibchack-fcntl.h"] if fcntl_hack else [],
         linker_version_scripts = ["glibc-hacks/fcntl.map"] if fcntl_hack else [],
@@ -127,11 +123,14 @@ def _target_linux_musl(gocpu, zigcpu):
         gotarget = "linux_{}_musl".format(gocpu),
         zigtarget = "{}-linux-musl".format(zigcpu),
         includes = [
-            "libc/include/generic-musl",
+                       "libc/include/{}-linux-musl".format(zigcpu),
+                       "libc/include/generic-musl",
+                   ] +
+                   # x86_64-linux-any is x86_64-linux and x86-linux combined.
+                   (["libc/include/x86-linux-any"] if zigcpu == "x86_64" else []) +
+                   (["libc/include/{}-linux-any".format(zigcpu)] if zigcpu != "x86_64" else []) + [
             "libc/include/any-linux-any",
-            "libc/include/{}-linux-musl".format(zigcpu),
-            "libc/include/{}-linux-any".format(zigcpu),
-        ] + (["libc/include/x86-linux-any"] if zigcpu == "x86_64" else []),
+        ],
         linkopts = [],
         dynamic_library_linkopts = [],
         copts = ["-D_LIBCPP_HAS_MUSL_LIBC", "-D_LIBCPP_HAS_THREAD_API_PTHREAD"],
-- 
2.34.4

---

From: "builds.sr.ht" <[email-masked]>
Date: Tue, 27 Sep 2022 08:12:08 +0000

bazel-zig-cc/patches/.build.yml: SUCCESS in 47m37s

[use relative paths][0] from [~motiejus][1]

[0]: https://lists.sr.ht/~motiejus/bazel-zig-cc/patches/35627
[1]: mailto:[email-masked]

✓ #851368 SUCCESS bazel-zig-cc/patches/.build.yml https://builds.sr.ht/~motiejus/job/851368

---

From: Laurynas Lubys <[email-masked]>
Date: Tue, 27 Sep 2022 12:44:34 +0300

Mostly looks good.
The `absolute_path` parameter can be inlined to avoid confusion.
I don't see a good way to keep the `private` package. It has to go, so
that all the generated files are available under the `zig_sdk//:all`
fileset.