Buck: Private and project level Objective-C headers no longer available in apple_test target

[mgrebenets]

While trying out the latest master branch (94647d7) I've noticed that behavior of apple_test rule has changed.

While previously I was able to build tests that use project-level headers, this is no longer possible with laster buck version.

E.g. if I have a project-level header in my framework target:

I can usually use it directly in my test cases. This works with Xcode and worked with older versions of buck as well. apple_test seemed to be able to find all headers from it's "parent" apple_library target.

Latest version of buck breaks this behavior.

Is it intentional?

Using private or protected headers in tests is similar to Swift's @testable import and seems to be an established pattern in iOS world.

Tools like CocoaPods offer options like inherit! :search_paths for test target to be able to find host target's headers.

I believe previous buck behavior was more forgiving in that regards and more inline with what other tools do.

If the change was intentional, is there an option or workaround to make apple_test behave same as before?

[robbertvanginkel]

This is not intentional, this is supposed to work. Can you add a sample project/setup that fails?

There hasn't been any significant work on the project generator for a while, looking at the last couple of weeks of commits nothing stands out. Do you have a last known working sha?

[mgrebenets]

@robbertvanginkel

I've tried to create a sample project but couldn't reproduce it.
What I did instead is I captured logs for the version of buck that works (which is pretty much your fork with one extra commit cherry-picked: https://github.com/mgrebenets/buck/tree/uber/modular_swift_appex_fix) and the latest master (6c2f9a).

buck-from-fork.log
buck-from-master.log

After diffing the logs, I noticed one major difference.
The older version of buck has this line:

-I buck-out/gen/pods/mylib.ios/MyLibDebug#iphonesimulator-x86_64,private-headers.hmap

but the logs created by latest buck don't have this line.

I'd assume this is where unit test target would be able to find private headers of the library target

[mgrebenets]

I should add that it fails with buck command line calls like buck test MyLib I didn't even generate Xcode project.

[mgrebenets]

While I couldn't get the initial issue reproduced, I managed to get a similar one here:
https://github.com/mgrebenets/test-app-with-bridging-header

The bridging header is where important things happen.
If I use import statement like so #import "ObjCProtocol.h" I get a failing build for buckw build sample_tests.
If I change it to #import "ObjC/ObjCProtocol.h" the works.

I don't know exactly, but from reading info in other issues I assumed that for headers like "ObjCProtocol.h" a symlinks should be created by buck, so #import "ObjCProtocol.h" would then resolve into a symlink which will then point to correct header. ObjcProtocol.h -> .....ObjC/ObjCProtocol.h.

I've seen these kinds of links created in another project, but they never got created in this sample project. In fact, the ObjCProtocol.h never ends up copied to buck-out directory at all.

[mgrebenets]

So here's few more findings on the issue.

A colleague of mine has found out that using relative paths in bridging headers fixes the build issue with latest version of buck from master.

In other words, if I have a structure like this:

└── Sources
    ├── Module
    │   └── MyHeader.h
    └── Supporting\ Files
        └── Bridging-Header.h

Then if I have this in Bridging-Header.h:

#import "MyHeader.h"

it works with the older fork of buck mentioned in this issue and it works in Xcode thanks to USER_HEADER_SEARCH_PATHS, but it does not work with latest version from master - it can't find "MyHeader.h" any more.

What we had to do as a workaround is to use relative paths in the bridging header, for example:

#import "../Modulde/MyHeader.h" // Path to MyHeader.h relative to Bridging-Header.h

This is a strange workaround and looks like breaking change in latest buck version, but something we probably could live with, because it's still backwards compatible with Xcode build system.

---

The other issue is still there, the one where it can't find private and protected headers for Objective-C libraries test targets.

It only happens for libraries that have Objective-C code only.
What I noticed though is if I add modular=False to the apple_library rule for this Objective-C module, the error goes away...

---

So to sum it up, I now have a project in which I can reproduce the issue:

https://github.com/mgrebenets/buck-private-header-demo

If you look in BUCK file, line 8:

    # If you comment this line, the build will pass...
    modular=True,

If this line is commented, then buckw test mylib command is successful, otherwise it fails wiht this error:

Tests/MyLibTests.m:5:9: fatal error: 'MyLibPrivateClass.h' file not found
#import "MyLibPrivateClass.h"
        ^~~~~~~~~~~~~~~~~~~~~
1 error generated.
Build failed: Command failed with exit code 1.
stderr: Tests/MyLibTests.m:5:9: fatal error: 'MyLibPrivateClass.h' file not found
#import "MyLibPrivateClass.h"

This seems to be the breaking change compared to older versions.

---

@milend @robbertvanginkel

Is there something I'm doing wrong?

I did my best going through diffs of swift and apple folders of Java source, but the code base unfortunatelly doesn't make too much sense to me since I'm not familiar with it.

I have noticed few things like use_modulewrap (didn't change anything) and addition of isModular (which made me try modular=False)
.

The main 2 questions I have:

• Should we only set modular=True for Swift and Mix-n-Match apple library targets, but not for Objective-C-only targets? (that's how it works at least on the master)
• Should we always use full relative paths in Swift bridging headers or is there some other flag for apple_binary which we can use to alter the behavior (it used to work earlier too)?

Thanks in advance 👍

---

UPD. Unfortunately setting modular=False is not the best workaround for us :(
After making this change, all statements like @import MyLib; will stop working.
I guess it totally makes sense, since there's no more underlying module for MyLib, but still doesn't make much sense why setting modular=True renders private headers "invisible" to test target.

[mgrebenets]

Could it be the changes in AppleLibraryDescription.java?
There's quite a few changes and some of them use isModular check.

For example, on the older fork I can see code like this:

 ImmutableList.of(privateInput.get(), publicInput.get())))

but on the latest master there's no privateInput. Well, there's no publicInput either, so things might have been refactored.
Still feels like this is where things changed.

[robbertvanginkel]

Modular and bridging-headers are be two mutually exclusive concepts in swift. Either you have a modular library and import objective c into your swift using the -import-underlying-module flag (buck will automatically create a modulemap for the headers that are under exported_headers, or you specify a bridging header it will add that to the swiftc command.

It sounds like this is intended, but we should probably make the error messaging more clear by just throwing an error if you specify both a bridging header and modular=True.

[mgrebenets]

I may have mixed things up.
In fact, bridging header and using private headers in apple_test rule are 2 separate issues and I've linked 2 separate examples for them.
It's just that at some point I thought those may be connected (and both behaviors have changed compared to the older fork).

Bridging headers aside, what do you think may be the root cause for a combination or apple_library + apple_test to stop working for Objective-C only library when modular=True is set?
It's the 2nd example here: https://github.com/mgrebenets/buck-private-header-demo
The library doesn't have any bridging header.

I'll just stick the BUCK file here as well for reference:

apple_library(
    name='MyLib',
    module_name='MyLib',
    header_path_prefix='MyLib',
    # If you comment this line, the build will pass...
    modular=True,
    visibility=['PUBLIC'],
    headers=glob([
        'Sources/**/*.h',
    ]),
    exported_headers=glob([
        'Sources/**/*.h',
    ], exclude=[
        # I tried commenting this line as well, doesn't change the outcome either.
        'Sources/Nested/MyLibPrivateClass.h',
    ]),
    srcs=glob([
        'Sources/**/*.m',
    ]),
    tests=[
        ':MyLibTests',
    ],
    frameworks=[
        '$SDKROOT/System/Library/Frameworks/Foundation.framework'
    ]
)

apple_test(
    name='MyLibTests',
    module_name='MyLibTests',
    deps=[
        ':MyLib',
    ],
    info_plist='Tests/Info.plist',
    info_plist_substitutions={
        'PRODUCT_BUNDLE_IDENTIFIER': 'org.test.' + 'MyLibTests',
    },
    headers=glob([
        'Tests/**/*.h',
    ]),
    srcs=glob([
        'Tests/**/*.m',
    ]),
    frameworks=[
        '$PLATFORM_DIR/Developer/Library/Frameworks/XCTest.framework',
    ]
)

[robbertvanginkel]

Thinking about the private imports do not work with modular=True on master. The limitation here is that once you expose a library's interface modularly, you can only reference those headers with the modulemap, otherwise you'll get duplicate symbol errors.

When buck creates a test command, it add something like -I dependency#headers,modular -I dependency#headers-private, where dependency#headers-modular contains a module.modulemap file and dependency#headers-private contains a superset of headers from dependency#headers-modular. The compiler can't recognise the headers in dependency#headers-private as part of the module and will complain that some things have duplicate definitions inside and outside of the module. For that reason if your library is marked as modular there are no private interface exposed at this time.

The fork you're referencing contains a commit that does something like that (https://github.com/robbertvanginkel/buck/commit/7933322b91248246f1012da298b6b3ed59dee007), but that is a really hacky workaround that hijacks some of the c code to expose a unified input without modulemap if the rule is being tested.

Ideally, we would find some way to get that functionality into master but I haven't had the ability to look at how we could achieve that in a reasonable way.

[mgrebenets]

I see, thanks for the explanation!
I'll try to cherry-pick the commit you referenced and see if it works.

Would it be completely impossible to try to hack around this issue on BUCK file level?
E.g. try to add -I dependency#headers-private to compiler_flags of apple_test rule somehow?
Or to have some genrule that creates symlinks for private headers and puts them somewhere just for buck to pick up?

Sorry if I'm talking nonsense :)
Would be nice to have a workaround on config level though.

[robbertvanginkel]

No, there's no way around this on the buck-file level. If you want private imports in tests, for now you'll have to disable modular (which also doesn't allow you to use it from swift).

[mgrebenets]

If anyone runs into this problem, we have somewhat ugly-ish workaround for it.

The trick is to add those private headers that apple_test can't find to its headers option, for example

apple_test(
    headers=glob([
        'Tests/**/*.h',
    ] + [
        'Sources/PrivateHeader1.h',
        'Sources/PrivateHeader2.h',
        'Sources/PrivateHeaderEtc.h',
    ]),

Now apple_test target will be able to find them.

Final change is to use <MyLib/PrivateHeader.h> imports in the test files, instead of "PrivateHeader.h", for example:

// MyLibTests.m

// #import "PrivateHeader.h" // --> This will NOT work with modular=True

#import <MyLib/PrivateHeader.h> // --> This works

The <MyLib/PrivateHeader.h> is also backwards compatible with Xcode, i.e. it still works with our current pre-buck Xcode projects, which we have to keep around still.

I'm yet to check if it works with projects generated by buck project command.

[yiding]

I agree that it doesn't make sense for private headers to be exported in the module map. If a test wants to do whitebox testing over the libraries, it should just include the headers in the test rule directly. If swift targets support multiple header maps, we might be able to create two of them, one for public and one for private headers. I don't know if that's possible, and it's not something I'm looking at.

[manojkarki]

Modular and bridging-headers are be two mutually exclusive concepts in swift. Either you have a modular library and import objective c into your swift using the -import-underlying-module flag (buck will automatically create a modulemap for the headers that are under exported_headers, or you specify a bridging header it will add that to the swiftc command.

It sounds like this is intended, but we should probably make the error messaging more clear by just throwing an error if you specify both a bridging header and modular=True.

What do you mean by

Modular and bridging-headers are be two mutually exclusive concepts in swift.

Is it BUCK only limitation because I think it is possible to have semantic import (modular lib) on a mixed Swift + Objective C lib?
Can you please elaborate.

[mgrebenets]

Always sounded to me like a buck only limitation, but I just don’t have enough knowledge on how things work 🤷‍♂️

Anyways. I sound like an old broken record, but I think importing main app module in tests is an OK thing to do.

Though there’s a workaround with moving main app code into s library, it always felt like a tool enforcing a change, rather than making it work “like with Xcode”.

[manojkarki]

Let's see if someone (@robbertvanginkel ?) can answer.
We currently have modular = false for mixed projects which is bad in the way that we cannot have semantic imports and take all the benefits of it (faster compilation and avoiding erroneous macro expansion )