Skip to content

Commit

Permalink
Set up placeholder tool in (apple|cxx|swift)_toolchain
Browse files Browse the repository at this point in the history
Summary:
**Context**

Supporting fat and non-fat toolchains becomes problematic because a fat toolchain usually combines tools using `command_alias()`. So if we set the platform constraints for the individual tools (which we need, so that the thin toolchain gets the correct platform), we end up producing conflicting constraints.

```
   ┌─────────────────┐          ┌───────────────┐
   │ linux-toolchain │          │ fat-toolchain │
   └─────────────────┘          └───────────────┘
            │                           │
            │                           │
            │                           │
            │                           │
"tool" = attrs.exec_dep()   "tool" = attrs.exec_dep()
            │                           │
            │                           │
            │                           │
            │                           │
            │                           │
            │                           │
            │                           ▼
            │          ┌────────────────────────────────┐
            │          │command_alias(name = "fat-tool")│
            │          └────────────────────────────────┘
            │                           │
            │                      platform_exe
            │                           │
            │                           │
            │         ┌─────────────────┴─────────────────────┐
            │         │                                       │
            │         │                                       │
            │         │                                       │
            │         │                                       │
            ▼         ▼                                       ▼
     ┌─────────────────────────────────┐    ┌───────────────────────────────────┐
     │sh_binary(name="linux-only-tool")│    │sh_binary(name="windows-only-tool")│
     └─────────────────────────────────┘    └───────────────────────────────────┘
```

If we were to set the `target_compatible_with` constraints on `linux-only-tool` and `windows-only-tool`, then `fat-tool` is not resolved to any exec platform, as no exec platform satisfies *both* Windows and Linux.

**Solution/Workaround**

There are two solutions to the problem:

1. Set up mirrored aliases of all underlying tools
2. Set up an placeholder tool on `(apple|cxx|swift)_toolchain()` for the purposes of setting exec platform constraints

We opt for solution #2 as it's much simpler and does not require extensive changes to existing toolchain setup. Since the tools are optional, they can be unused without any downsides.

Reviewed By: rmaz

Differential Revision: D44090642

fbshipit-source-id: 399cfbfdf4dcbb4b334e22545cfea5e86e31c837
  • Loading branch information
milend authored and facebook-github-bot committed Mar 16, 2023
1 parent 36a2f60 commit c54962a
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 0 deletions.
8 changes: 8 additions & 0 deletions prelude/apple/apple_rules_impl.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,10 @@ extra_attributes = {
"lipo": attrs.dep(providers = [RunInfo]),
"min_version": attrs.option(attrs.string(), default = None),
"momc": attrs.dep(providers = [RunInfo]),
# A placeholder tool that can be used to set up toolchain constraints.
# Useful when fat and thin toolchahins share the same underlying tools via `command_alias()`,
# which requires setting up separate platform-specific aliases with the correct constraints.
"placeholder_tool": attrs.option(attrs.dep(providers = [RunInfo]), default = None),
"platform_path": attrs.option(attrs.source(), default = None), # Mark as optional until we remove `_internal_platform_path`
# Defines whether the Xcode project generator needs to check
# that the selected Xcode version matches the one defined
Expand Down Expand Up @@ -189,6 +193,10 @@ extra_attributes = {
},
"swift_toolchain": {
"architecture": attrs.option(attrs.string(), default = None), # TODO(T115173356): Make field non-optional
# A placeholder tool that can be used to set up toolchain constraints.
# Useful when fat and thin toolchahins share the same underlying tools via `command_alias()`,
# which requires setting up separate platform-specific aliases with the correct constraints.
"placeholder_tool": attrs.option(attrs.dep(providers = [RunInfo]), default = None),
"platform_path": attrs.option(attrs.source(), default = None), # Mark as optional until we remove `_internal_platform_path`
"sdk_modules": attrs.list(attrs.dep(), default = []), # A list or a root target that represent a graph of sdk modules (e.g Frameworks)
"sdk_path": attrs.option(attrs.source(), default = None), # Mark as optional until we remove `_internal_sdk_path`
Expand Down
4 changes: 4 additions & 0 deletions prelude/cxx/cxx_toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ def cxx_toolchain_extra_attributes(is_toolchain_rule):
"linker": dep_type(providers = [RunInfo]),
"nm": dep_type(providers = [RunInfo]),
"objcopy_for_shared_library_interface": dep_type(providers = [RunInfo]),
# A placeholder tool that can be used to set up toolchain constraints.
# Useful when fat and thin toolchahins share the same underlying tools via `command_alias()`,
# which requires setting up separate platform-specific aliases with the correct constraints.
"placeholder_tool": attrs.option(dep_type(providers = [RunInfo]), default = None),
# Used for resolving any 'platform_*' attributes.
"platform_name": attrs.option(attrs.string(), default = None),
"private_headers_symlinks_enabled": attrs.bool(default = True),
Expand Down

0 comments on commit c54962a

Please sign in to comment.