[bazel] Add layering-check

[gchatelet]

Summary

In the same vein as https://reviews.llvm.org/D141553
Enable the feature globally to ensure layering and catch circular dependencies
(https://llvm.org/docs/CodingStandards.html#library-layering).

[GMNGeoffrey]

layering_check has already been turned on at the package level for the packages that are layering clean (e.g. https://github.com/llvm/llvm-project/blob/main/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel#L15). You could add more

[gchatelet]

In D143678#4116423, @GMNGeoffrey wrote:

layering_check has already been turned on at the package level for the packages that are layering clean (e.g. https://github.com/llvm/llvm-project/blob/main/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel#L15). You could add more

I did it originally for libc but got bitten by not setting it in the subfolders BUILD.bazel files.
Subfolder BUILD.bazel files create a new package that also needs to receive the feature.
@aeubanks suggested to add it to the .blazerc file instead to be more thorough.

[gchatelet]

I just looked at the premerge checks and I can see how this will be hard to fix globally (if at all possible).
I'll use the package feature instead.

[rupprecht]

In D143678#4116440, @gchatelet wrote:

I just looked at the premerge checks and I can see how this will be hard to fix globally (if at all possible).
I'll use the package feature instead.

IMO we should try to clean those up, and then once we do so, remove the package-level annotations and have the feature globally enabled -- but as build --features=layering_check, not build:ci --features=layering_check.

If there are any that are really problematic, we can add a feature to disable layering check for that rule (or package)

[gchatelet]

In D143678#4117090, @rupprecht wrote:

In D143678#4116440, @gchatelet wrote:

I just looked at the premerge checks and I can see how this will be hard to fix globally (if at all possible).
I'll use the package feature instead.

IMO we should try to clean those up, and then once we do so, remove the package-level annotations and have the feature globally enabled -- but as build --features=layering_check, not build:ci --features=layering_check.

If there are any that are really problematic, we can add a feature to disable layering check for that rule (or package)

I've fixed all layering checks in https://github.com/llvm/llvm-project/commit/1842b5885baa1e002b3fa8b6ac1bae9cf4d39610
I'm reviving this patch to add build --features=layering_check as you suggests.

[gchatelet]

rebase and move layering check as a global option

[aeubanks]

lg if bazel precommit ci lg

[MaskRay]

utils/bazel/.bazelrc

25

Add

[GMNGeoffrey]

Looks like there's still an issue with mpfr. One advantage to adding at the package level is that it will also apply for downstream projects using LLVM. So if someone is building via their own project and editing an LLVM submodule or something then they would still have layering enforced. Just a thought

[gchatelet]

• Fix typo

[gchatelet]

One advantage to adding at the package level is that it will also apply for downstream projects using LLVM. So if someone is building via their own project and editing an LLVM submodule or something then they would still have layering enforced. Just a thought

I've added bazel-group as a reviewer to get input from other people.

[gchatelet]

In D143678#4118904, @GMNGeoffrey wrote:

Looks like there's still an issue with mpfr.

It runs just fine locally with bazelisk-linux-amd64 test --config=ci --sandbox_base=/dev/shm @llvm-project//... and bazelisk-linux-amd64 query //... + @llvm-project//... | xargs bazelisk-linux-amd64 test --config=ci, could it be a caching issue?

[gchatelet]

• Disable layering check when using mpfr_system

[gchatelet]

• Disable layering check when using mpfr_system
• Remove layering_check on MPRFUtils instead

utils/bazel/.bazelrc

 # Shared objects take up more space. With fast linkers and binaries that aren't
 # super large, the benefits of shared objects are minimal.
 build --dynamic_mode=off
 
 # Rely on compiler flags to compile with debug info/strip rather than stripping
 # based on compilation_mode.
 build --strip=never
 
+# Add layering check to all projects.
+build --features=layering_check
+
 ###############################################################################
 # Options to select different strategies for linking potential dependent
 # libraries. The default leaves it disabled.
 ###############################################################################
 
 build:zlib_external --repo_env=BAZEL_LLVM_ZLIB_STRATEGY=external
 build:zlib_system --repo_env=BAZEL_LLVM_ZLIB_STRATEGY=system
 

utils/bazel/llvm-project-overlay/libc/utils/MPFRWrapper/BUILD.bazel

deps = select(
         },
     ),
 )
 
 cc_library(
     name = "mpfr_wrapper",
     srcs = ["MPFRUtils.cpp"],
     hdrs = ["MPFRUtils.h"],
+    # Disable layering check when using mpfr_system.
+    features = select(
+        {
+            "//conditions:default": [],
+            "//libc:mpfr_system": ["-layering_check"],
+        },
+    ),
     deps = [
         "//libc:__support_common",
         "//libc:__support_cpp_bit",
         "//libc:__support_cpp_bitset",
         "//libc:__support_cpp_string_view",
         "//libc:__support_cpp_type_traits",
         "//libc:__support_fputil_fp_bits",
         "//libc:__support_fputil_platform_defs",
         "//libc:libc_root",
-        "//libc/utils/MPFRWrapper:mpfr_impl",
         "//libc/test/UnitTest:LibcUnitTest",
         "//libc/test/UnitTest:fp_test_helpers",
+        "//libc/utils/MPFRWrapper:mpfr_impl",
         "//libc/utils/testutils:libc_test_utils",
     ],
 )