[libc][bazel] Convert "errno" to libc_support_library.

[vonosmas]

This shouldn't really be a "libc_function" since it can be used as a dependency of various other support libraries and functions, and doesn't correspond to a well-defined endpoint that the users may want to explicitly depend on (if they depend on it implicitly through a libc_function whose implementation relies on errno, the dependency will be pulled in normally). Remove special handling for "errno" in the Bazel test rules.

[llvmbot]

@llvm/pr-subscribers-libc

Author: Alexey Samsonov (vonosmas)

Changes

This shouldn't really be a "libc_function" since it can be used as a dependency of various other support libraries and functions, and doesn't correspond to a well-defined endpoint that the users may want to explicitly depend on (if they depend on it implicitly through a libc_function whose implementation relies on errno, the dependency will be pulled in normally). Remove special handling for "errno" in the Bazel test rules.

---

Full diff: https://github.com/llvm/llvm-project/pull/130368.diff

3 Files Affected:

• (modified) utils/bazel/llvm-project-overlay/libc/BUILD.bazel (+2-2)
• (modified) utils/bazel/llvm-project-overlay/libc/test/libc_test_rules.bzl (+2-2)
• (modified) utils/bazel/llvm-project-overlay/libc/test/src/stdlib/BUILD.bazel (+1-1)

diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index 61de1aa7f2abe..757db65ca8a77 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -1598,9 +1598,9 @@ libc_support_library(
     ],
 )
 
-############################### errno targets ################################
+############################### errno ########################################
 
-libc_function(
+libc_support_library(
     name = "errno",
     srcs = ["src/errno/libc_errno.cpp"],
     hdrs = ["src/errno/libc_errno.h"],
diff --git a/utils/bazel/llvm-project-overlay/libc/test/libc_test_rules.bzl b/utils/bazel/llvm-project-overlay/libc/test/libc_test_rules.bzl
index 23dc1afec9e42..8c20a9172989c 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/libc_test_rules.bzl
+++ b/utils/bazel/llvm-project-overlay/libc/test/libc_test_rules.bzl
@@ -27,14 +27,14 @@ def libc_test(name, srcs, libc_function_deps = [], copts = [], deps = [], local_
       local_defines: The list of target local_defines if any.
       **kwargs: Attributes relevant for a libc_test. For example, name, srcs.
     """
-    all_function_deps = libc_function_deps + ["//libc:errno"]
     native.cc_test(
         name = name,
         srcs = srcs,
         local_defines = local_defines + LIBC_CONFIGURE_OPTIONS,
-        deps = [libc_internal_target(d) for d in all_function_deps] + [
+        deps = [libc_internal_target(d) for d in libc_function_deps] + [
             "//libc/test/UnitTest:LibcUnitTest",
             "//libc:__support_macros_config",
+            "//libc:errno",
             "//libc:func_aligned_alloc",
             "//libc:func_free",
             "//libc:func_malloc",
diff --git a/utils/bazel/llvm-project-overlay/libc/test/src/stdlib/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/src/stdlib/BUILD.bazel
index abe5e69fae926..5f43ec7c7a109 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/src/stdlib/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/src/stdlib/BUILD.bazel
@@ -186,7 +186,7 @@ libc_test_library(
         "//libc:__support_cpp_type_traits",
         "//libc:__support_ctype_utils",
         "//libc:__support_macros_properties_architectures",
-        "//libc:errno.__internal__",
+        "//libc:errno",
         "//libc/test/UnitTest:LibcUnitTest",
     ],
 )

[lntue]

Currently, bazel build is essentially in overlay mode. So I think we should define -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_SYSTEM for the errno target. Can you try to see if it's fine for our tests?

[vonosmas]

Currently, bazel build is essentially in overlay mode. So I think we should define -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_SYSTEM for the errno target. Can you try to see if it's fine for our tests?

I believe this is orthogonal to the PR, since the PR only changes the bazel plumbing, but doesn't modify how the library is used in any meaningful way?

In any case, I looked at libc/src/errno/libc_errno.cpp , and it seems that:

• for release build, the default would already be -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_SYSTEM, since LIBC_FULL_BUILD is not defined, but LIBC_COPT_PUBLIC_PACKAGING is.
• for unit tests, the default would be -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_THREAD_LOCAL. If I explicitly set it to LIBC_ERRNO_MODE_SYSTEM, the tests still pass. Let me know if you'd like me to do it, but I'd prefer to land it in a separate CL.

[lntue]

Currently, bazel build is essentially in overlay mode. So I think we should define -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_SYSTEM for the errno target. Can you try to see if it's fine for our tests?

I believe this is orthogonal to the PR, since the PR only changes the bazel plumbing, but doesn't modify how the library is used in any meaningful way?

In any case, I looked at libc/src/errno/libc_errno.cpp , and it seems that:

• for release build, the default would already be -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_SYSTEM, since LIBC_FULL_BUILD is not defined, but LIBC_COPT_PUBLIC_PACKAGING is.
• for unit tests, the default would be -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_THREAD_LOCAL. If I explicitly set it to LIBC_ERRNO_MODE_SYSTEM, the tests still pass. Let me know if you'd like me to do it, but I'd prefer to land it in a separate CL.

Yes, the reason I asked was before it looks like this PR will silently change the errno config for unit tests, so if it's still working then probably we should explicitly set that flag in case it is changed in the future.

[lntue]

Currently, bazel build is essentially in overlay mode. So I think we should define -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_SYSTEM for the errno target. Can you try to see if it's fine for our tests?

I believe this is orthogonal to the PR, since the PR only changes the bazel plumbing, but doesn't modify how the library is used in any meaningful way?

In any case, I looked at libc/src/errno/libc_errno.cpp , and it seems that:

• for release build, the default would already be -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_SYSTEM, since LIBC_FULL_BUILD is not defined, but LIBC_COPT_PUBLIC_PACKAGING is.
• for unit tests, the default would be -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_THREAD_LOCAL. If I explicitly set it to LIBC_ERRNO_MODE_SYSTEM, the tests still pass. Let me know if you'd like me to do it, but I'd prefer to land it in a separate CL.

and a separate patch is totally fine for me.

[vonosmas]

Currently, bazel build is essentially in overlay mode. So I think we should define -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_SYSTEM for the errno target. Can you try to see if it's fine for our tests?

I believe this is orthogonal to the PR, since the PR only changes the bazel plumbing, but doesn't modify how the library is used in any meaningful way?
In any case, I looked at libc/src/errno/libc_errno.cpp , and it seems that:

• for release build, the default would already be -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_SYSTEM, since LIBC_FULL_BUILD is not defined, but LIBC_COPT_PUBLIC_PACKAGING is.
• for unit tests, the default would be -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_THREAD_LOCAL. If I explicitly set it to LIBC_ERRNO_MODE_SYSTEM, the tests still pass. Let me know if you'd like me to do it, but I'd prefer to land it in a separate CL.

Yes, the reason I asked was before it looks like this PR will silently change the errno config for unit tests, so if it's still working then probably we should explicitly set that flag in case it is changed in the future.

This PR doesn't change the errno config for unit tests - the tests used to depend on errno.__internal__ library created by libc_function, which doesn't have LIBC_COPT_PUBLIC_PACKAGING define. They will still depend on (effectively, internal) version of errno after this PR lands.

[vonosmas]

Currently, bazel build is essentially in overlay mode. So I think we should define -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_SYSTEM for the errno target. Can you try to see if it's fine for our tests?

I believe this is orthogonal to the PR, since the PR only changes the bazel plumbing, but doesn't modify how the library is used in any meaningful way?
In any case, I looked at libc/src/errno/libc_errno.cpp , and it seems that:

• for release build, the default would already be -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_SYSTEM, since LIBC_FULL_BUILD is not defined, but LIBC_COPT_PUBLIC_PACKAGING is.
• for unit tests, the default would be -DLIBC_ERRNO_MODE=LIBC_ERRNO_MODE_THREAD_LOCAL. If I explicitly set it to LIBC_ERRNO_MODE_SYSTEM, the tests still pass. Let me know if you'd like me to do it, but I'd prefer to land it in a separate CL.

and a separate patch is totally fine for me.

Thanks! I'll land this PR after an LGTM, and will send a follow-up CL to explicitly depend on system errno in llvm-libc Bazel build.