Skip to content

Commit

Permalink
[antlir][toolchain] add arch to CxxPlatformInfo
Browse files Browse the repository at this point in the history
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
  • Loading branch information
vmagro authored and facebook-github-bot committed Jan 2, 2025
1 parent 40cf0a2 commit 76fcb92
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 22 deletions.
23 changes: 16 additions & 7 deletions antlir/antlir2/os/oses.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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:
Expand All @@ -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",
),
)
14 changes: 6 additions & 8 deletions antlir/distro/platform/defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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(
*,
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
22 changes: 15 additions & 7 deletions antlir/distro/toolchain/cxx/defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
18 changes: 18 additions & 0 deletions antlir/distro/toolchain/cxx/tests/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions antlir/distro/toolchain/cxx/tests/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
16 changes: 16 additions & 0 deletions antlir/distro/toolchain/cxx/tests/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()}",
)

0 comments on commit 76fcb92

Please sign in to comment.