facebook / buck2

Build system, successor to Buck
https://buck2.build/
Apache License 2.0
3.31k stars 190 forks source link

Trouble linking Apple Frameworks #616

Open zjturner opened 3 weeks ago

zjturner commented 3 weeks ago

I have a cxx_library which contains a couple of objective c++ files. Because of this, they carry with them a transitive link dependency on an apple framework. I've tried something like this:

cxx_library(
    name = "foo",
    frameworks = select({
        "config//os:macos": ["$SDKROOT/AVFoundation.framework"],
        "DEFAULT": []
    })

When I do this, I get the following error:

       error: Object of type `struct` has no attribute `_apple_toolchain`
         --> Client/dependencies/buck2/prelude/apple/apple_frameworks.bzl:93:28
          |
       93 |     apple_toolchain_info = ctx.attrs._apple_toolchain[AppleToolchainInfo]
          |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^
          |

It isn't clear to me how this is supposed to work, as cxx_library contains no _apple_toolchain attribute in the first place. I know that there is some common prelude code that is shared among the apple rules and the regular cxx rules, and so this particular function will work when invoked from an apple_library rule, but on the other hand, the cxx_library rule contains a frameworks attribute, so I would expect to be able to do this.

I see there is other code throughout the prelude that uses a function like has_apple_toolchain(), and then has a fallback path if it doesn't, but this particular code in apple_frameworks.bzl does not have such a test. Is this a bug in the prelude?

I can submit a PR if there's consensus that this is a bug.

Higher level, I don't really understand the need for a separate family of apple rules. It creates some awkward problems for us, because we have quite a bit of mixed Objective C++ / Regular C++ libraries, and the rule dichotomy makes it seemingly impossible(?) to support this use case.

Why can't the cxx_library family of rules simply accept an _apple_toolchain attribute and then just use that rule for all C / C++ / Objective C / Objective C++ code rather than have to use a completely different rule? Even better, why can't it just read the AppleToolchainInfo provider directly off of the _cxx attribute?

One potential solution idea I came up with involves this function:

https://github.com/facebook/buck2/blob/e5492aac499a7d7ee070293708781b61a5792fc9/prelude/cxx/cxx_context.bzl#L24

def get_cxx_platform_info(ctx: AnalysisContext) -> CxxPlatformInfo:
    apple_toolchain = getattr(ctx.attrs, "_apple_toolchain", None)
    if apple_toolchain:
        return apple_toolchain[AppleToolchainInfo].cxx_platform_info
    return ctx.attrs._cxx_toolchain[CxxPlatformInfo]

Perhaps there could be a similar function called get_apple_toolchain_info that looks like this:

def get_apple_toolchain_info(ctx: AnalysisContext) -> AppleToolchainInfo:
    apple_toolchain = getattr(ctx.attrs, "_apple_toolchain", None)
    if not apple_toolchain:
        cxx = getattr(ctx.attrs, "_cxx_toolchain", None)
        if cxx:
            apple_toolchain = cxx[AppleToolchainInfo] if AppleToolchainInfo in cxx else None
    return apple_toolchain

then, instead of accessing ctx.attrs._apple_toolchain directly, we could call this function. This would allow me to return an AppleToolchainInfo directly from my cxx toolchain.

I can probably unblock myself by manually sticking command line arguments onto the command line, but that kind of dirties up the target definition and it seems like there's at least some intent that this is supposed to "just work". So I'd really like to find a way to get this working upstream.

zjturner commented 3 weeks ago

So I figured out that I was using the frameworks argument wrong, I need to specify "$SDKROOT/System/Frameworks/AVFoundation.framework" and then it works. But I think that's just "happens" to fix the problem because it's a builtin location that the prelude has firsthand knowledge of. the problem above would still arise for other frameworks that aren't in the -isysroot location. So my primary question is solved, but I'd like to leave this open to possibly get some context about the design of the rule hierarchy, and whether we can support providing an AppleToolchainInfo directly from the _cxx_toolchain. I made this work locally FWIW but will hold off with the PR until I have some clarity on the team's desired direction.