From 76fcb924aeaceaffae6f989c1452694a9f06a8c2 Mon Sep 17 00:00:00 2001 From: Vinnie Magro Date: Thu, 2 Jan 2025 14:58:12 -0800 Subject: [PATCH] [antlir][toolchain] add arch to CxxPlatformInfo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: `_platform_` attrs on cxx rules match against the cxx toolchain name, so we need to include the architecture in there for the times where it is important Test Plan: ``` ❯ buck2 test fbcode//antlir/distro/toolchain/cxx/tests: -- test_platform_preprocessor_flags Buck UI: https://www.internalfb.com/buck2/8f4e70f9-f2a8-4c0e-a4eb-138e75437d18 Test UI: https://www.internalfb.com/intern/testinfra/testrun/9288674294184161 Tests finished: Pass 2. Fail 0. Fatal 0. Skip 0. Build failure 0 ``` ``` ❯ buck2 test -c fbcode.arch=aarch64 fbcode//antlir/distro/toolchain/cxx/tests: -- test_platform_preprocessor_flags Buck UI: https://www.internalfb.com/buck2/8170708c-f8d5-4591-9fe4-3e570ebd22dc Test UI: https://www.internalfb.com/intern/testinfra/testrun/14355223874690734 Tests finished: Pass 2. Fail 0. Fatal 0. Skip 0. Build failure 0 ``` Reviewed By: epilatow Differential Revision: D67605234 fbshipit-source-id: 5e4620066377c0d4a270fe375dd761cacafa28e5 --- antlir/antlir2/os/oses.bzl | 23 +++++++++++++++------- antlir/distro/platform/defs.bzl | 14 ++++++------- antlir/distro/toolchain/cxx/defs.bzl | 22 ++++++++++++++------- antlir/distro/toolchain/cxx/tests/BUCK | 18 +++++++++++++++++ antlir/distro/toolchain/cxx/tests/main.cpp | 1 + antlir/distro/toolchain/cxx/tests/test.py | 16 +++++++++++++++ 6 files changed, 72 insertions(+), 22 deletions(-) diff --git a/antlir/antlir2/os/oses.bzl b/antlir/antlir2/os/oses.bzl index 8f432d9e9b6..72ead592d03 100644 --- a/antlir/antlir2/os/oses.bzl +++ b/antlir/antlir2/os/oses.bzl @@ -5,7 +5,16 @@ load("//antlir/bzl:internal_external.bzl", "internal_external", "is_facebook") -arch_t = enum("x86_64", "aarch64") +arch_t = record( + name = str, + select_key = str, +) + +def new_arch_t(s: str) -> arch_t: + return arch_t( + name = s, + select_key = {"aarch64": "ovr_config//cpu:arm64", "x86_64": "ovr_config//cpu:x86_64"}[s], + ) os_t = record( name = str, @@ -18,8 +27,8 @@ os_t = record( def _new_os(name: str, **kwargs): kwargs.setdefault("architectures", internal_external( - fb = [arch_t("x86_64"), arch_t("aarch64")], - oss = [arch_t("x86_64")], + fb = [new_arch_t("x86_64"), new_arch_t("aarch64")], + oss = [new_arch_t("x86_64")], )) kwargs.setdefault("select_key", "antlir//antlir/antlir2/os:" + name) kwargs.setdefault( @@ -58,15 +67,15 @@ if is_facebook: ), _new_os( name = "centos8", - architectures = [arch_t("x86_64")], + architectures = [new_arch_t("x86_64")], ), _new_os( name = "rhel8", - architectures = [arch_t("x86_64")], + architectures = [new_arch_t("x86_64")], ), _new_os( name = "rhel8.8", - architectures = [arch_t("x86_64")], + architectures = [new_arch_t("x86_64")], ), ]) else: @@ -75,7 +84,7 @@ else: OSES.append( _new_os( name = "centos8", - architectures = [arch_t("x86_64")], + architectures = [new_arch_t("x86_64")], flavor = "antlir//antlir/antlir2/flavor:none", ), ) diff --git a/antlir/distro/platform/defs.bzl b/antlir/distro/platform/defs.bzl index af07f4284b1..eaefb9d5959 100644 --- a/antlir/distro/platform/defs.bzl +++ b/antlir/distro/platform/defs.bzl @@ -5,15 +5,13 @@ # @oss-disable load("@prelude//:rules.bzl", "platform") -load("//antlir/antlir2/os:oses.bzl", "OSES", "arch_t", "os_t") +load("//antlir/antlir2/os:oses.bzl", "OSES", "arch_t", "new_arch_t", "os_t") def _cpu_label(arch: arch_t, *, constraint: bool = False) -> str: - arch = arch.value - if arch == "aarch64": - arch = "arm64" + sk = arch.select_key if constraint: - return "ovr_config//cpu/constraints:" + arch - return "ovr_config//cpu:" + arch + sk = sk.replace("ovr_config//cpu:", "ovr_config//cpu/constraints:") + return sk def _image_platform( *, @@ -42,7 +40,7 @@ def _image_platform( ) def _platform_name(os: os_t, arch: arch_t) -> str: - return os.name + "-" + arch.value + return os.name + "-" + arch.name def define_platforms(): for os in OSES: @@ -93,7 +91,7 @@ def default_image_platform(os: str): # @oss-disable default_arch = "aarch64" if native.host_info().arch.is_aarch64 else "x86_64" # @oss-enable - default_arch = arch_t(default_arch) + default_arch = new_arch_t(default_arch) found_os = None for test_os in OSES: if test_os.name == os: diff --git a/antlir/distro/toolchain/cxx/defs.bzl b/antlir/distro/toolchain/cxx/defs.bzl index 69051549a77..d13ccdf2997 100644 --- a/antlir/distro/toolchain/cxx/defs.bzl +++ b/antlir/distro/toolchain/cxx/defs.bzl @@ -88,12 +88,17 @@ def image_cxx_toolchain( actual = layer, default_os = os.name, ) - _single_image_cxx_toolchain( - name = "{}--{}".format(name, os.name), - layer = ":{}--{}--layer".format(name, os.name), - sysroot = sysroot, - visibility = [], - ) + for arch in os.architectures: + # despite the fact that this has no arch-specific configuration, the + # toolchain name is used by the cxx rules `_*_platform` attrs (eg + # `platform_compiler_flags`) and so it should have an architecture + # to match on + _single_image_cxx_toolchain( + name = "{}--{}-{}".format(name, os.name, arch.name), + layer = ":{}--{}--layer".format(name, os.name), + sysroot = sysroot, + visibility = [], + ) # The "real" toolchain is actually an alias that depends on the selected OS. # This is necessary because all the tools listed above (clang, ld.lld, etc) @@ -105,7 +110,10 @@ def image_cxx_toolchain( name = name, actual = select( { - os.select_key: ":{}--{}".format(name, os.name) + os.select_key: select({ + arch.select_key: ":{}--{}-{}".format(name, os.name, arch.name) + for arch in os.architectures + }) for os in oses } | # This will never actually be configured as DEFAULT for a real diff --git a/antlir/distro/toolchain/cxx/tests/BUCK b/antlir/distro/toolchain/cxx/tests/BUCK index 39997ce864b..136e39e9139 100644 --- a/antlir/distro/toolchain/cxx/tests/BUCK +++ b/antlir/distro/toolchain/cxx/tests/BUCK @@ -21,6 +21,24 @@ prelude.cxx_binary( "//antlir/distro:build-for-distro", ], default_target_platform = default_image_platform("centos9"), + platform_preprocessor_flags = [ + ( + "centos9-x86_64$", + ['-DPLATFORM_PREPROCESSOR_FLAG="centos9-x86_64"'], + ), + ( + "centos10-x86_64$", + ['-DPLATFORM_PREPROCESSOR_FLAG="centos10-x86_64"'], + ), + ( + "centos9-aarch64$", + ['-DPLATFORM_PREPROCESSOR_FLAG="centos9-aarch64"'], + ), + ( + "centos10-aarch64$", + ['-DPLATFORM_PREPROCESSOR_FLAG="centos10-aarch64"'], + ), + ], deps = [ ":dep", "//antlir/distro/deps/jsoncpp:jsoncpp", diff --git a/antlir/distro/toolchain/cxx/tests/main.cpp b/antlir/distro/toolchain/cxx/tests/main.cpp index 1eddeac760d..8fe461dccce 100644 --- a/antlir/distro/toolchain/cxx/tests/main.cpp +++ b/antlir/distro/toolchain/cxx/tests/main.cpp @@ -13,6 +13,7 @@ int main(int argc, char** argv) { Json::Value root; root["clang_version"] = __clang_version__; root["rpmlib_version"] = dep_get_rpmlib_version(); + root["platform_preprocessor_flag"] = PLATFORM_PREPROCESSOR_FLAG; std::cout << root << std::endl; return 0; } diff --git a/antlir/distro/toolchain/cxx/tests/test.py b/antlir/distro/toolchain/cxx/tests/test.py index 8c08619e885..5b0c2465307 100644 --- a/antlir/distro/toolchain/cxx/tests/test.py +++ b/antlir/distro/toolchain/cxx/tests/test.py @@ -110,3 +110,19 @@ def test_rpm_dependencies(self) -> None: any(r.startswith("librpm.so") for r in requires), "'main' did not require librpm.so", ) + + def test_platform_preprocessor_flags(self) -> None: + """ + Check that the preprocessor flags are set based on platform regex + matches in cxx rules. + """ + out = json.loads( + subprocess.run( + [self.binary], check=True, capture_output=True, text=True + ).stdout + ) + platform_preprocessor_flag = out["platform_preprocessor_flag"] + self.assertEqual( + platform_preprocessor_flag, + f"{self.os}-{platform.machine()}", + )