Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ios_bundle rule #1085

Closed
wants to merge 1 commit into from
Closed

Add ios_bundle rule #1085

wants to merge 1 commit into from

Conversation

brentleyjones
Copy link
Collaborator

Note: I haven't tested this rebase of 558ea6a. I'm just placing this here as a request in #1081.

Resolves #1081.

Comment on lines +46 to +49
"custom_bundles": """
A dict of depsets of zipped archives of bundles that need to be expanded into
custom locations of the packaging bundle, where the key is the path to the
custom location.""",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed to support the EarlGray use case where the bundle needs to be placed at a custom location within the packaging bundle.

@brentleyjones
Copy link
Collaborator Author

I'll fix the build errors soon

@brentleyjones
Copy link
Collaborator Author

@erikkerber Can you provide the BUILD file for EarlGrey? And an example one for a loadable bundle? I know we had a lot of trouble getting it to work correctly.

@erikkerber
Copy link
Contributor

The IPC bundle:

load("@build_bazel_rules_apple//apple:ios.bzl", "ios_bundle")
load("@build_bazel_rules_swift//swift:swift.bzl", "swift_library")
load("//bazel/rules:constants.bzl", "DEVICE_FAMILIES", "MIN_IOS_VERSION", "TEAM_ID")
load("//bazel/rules:provisioning_profiles.bzl", "provisioning_profile")

ios_bundle(
    name = "Bundle",
    bundle_id = "com.target.Target.EarlGreyIPCBundle",
    bundle_location = "EarlGreyHelperBundles",
    bundle_name = "FlagshipEarlGreyIPCBundle",
    families = DEVICE_FAMILIES,
    frameworks = ["//src/Apps/Shared/EarlGrey:IPCFramework"],
    infoplists = ["Info.plist"],
    minimum_os_version = MIN_IOS_VERSION,
    provisioning_profile = provisioning_profile(
        "Bundle",
        {
            "//:dev_signed": "Test App Wildcard Development",
            "//conditions:default": None,
        },
        bundle_id = "com.target.Target.EarlGreyIPCBundle",
        team_id = TEAM_ID,
    ),
    version = "//src/Apps/Flagship:Version",
    visibility = ["//src/Apps/Flagship:__pkg__"],
    deps = [":Bundle.__tgt__.library"],
)

swift_library(
    name = "Bundle.__tgt__.library",
    srcs = glob(["Implementations/**/*.swift"]),
    module_name = "FlagshipEarlGreyIPC",
    deps = [
        ":Interfaces",
        "@com_github_alisoftware_ohhttpstubs//:OHHTTPStubs",
        "@com_github_alisoftware_ohhttpstubs//:OHHTTPStubsSwift",
        "@com_github_google_earlgrey//:EarlGreyIPC",
    ],
)

swift_library(
    name = "Interfaces",
    srcs = glob(["Interfaces/**/*.swift"]),
    module_name = "FlagshipEarlGreyIPCInterfaces",
    # TODO: Limit visibility to UI tests
    visibility = ["//visibility:public"],
)

EarlGrey:

load("@build_bazel_rules_apple//apple:ios.bzl", "ios_framework")

# This is the framework that IPC bundles will link against.
# It mirrors the framework that is bundled in the app under test, but has `bundle_only = False`.
ios_framework(
    name = "IPCFramework",
    bundle_id = "com.google.earlgrey.AppFramework",
    bundle_name = "AppFramework",
    families = [
        "iphone",
        "ipad",
    ],
    infoplists = ["@com_github_google_earlgrey//:AppFramework/Info.plist"],
    minimum_os_version = "10.0",
    # TODO: This should be limited to IPC bundles only
    visibility = ["//visibility:public"],
    deps = ["@com_github_google_earlgrey//:EarlGreyIPC"],
)

@erikkerber
Copy link
Contributor

EarlGrey itself:

load("@build_bazel_rules_apple//apple:ios.bzl", "ios_framework")
load("@build_bazel_rules_swift//swift:swift.bzl", "swift_library")
load("@rules_cc//cc:defs.bzl", "objc_library")

# This project is super sloppy, they allow includes from all of the libraries
ALL_INCLUDES = [
    "AppFramework/Action",
    "AppFramework/Additions",
    "AppFramework/Assertion",
    "AppFramework/AutomationSetup",
    "AppFramework/Config",
    "AppFramework/Core",
    "AppFramework/Delegate",
    "AppFramework/DistantObject",
    "AppFramework/EarlGreyApp",
    "AppFramework/Error",
    "AppFramework/Event",
    "AppFramework/IdlingResources",
    "AppFramework/Keyboard",
    "AppFramework/Matcher",
    "AppFramework/Synchronization",
    "CommonLib/",
    "CommonLib/Additions",
    "CommonLib/Assertion",
    "CommonLib/Config",
    "CommonLib/DistantObject",
    "CommonLib/Error",
    "CommonLib/Event",
    "CommonLib/Exceptions",
    "CommonLib/Matcher",
    "CommonLib/Provider",
    "TestLib/AlertHandling",
    "TestLib/AppleInternals",
    "TestLib/Assertion",
    "TestLib/Condition",
    "TestLib/Config",
    "TestLib/DistantObject",
    "TestLib/EarlGreyImpl",
    "TestLib/Exception",
    "TestLib/Execution",
    "TestLib/XCTestCase",
    "UILib",
    "UILib/Additions",
    "UILib/Provider",
    "UILib/Traversal",
    "UILib/Visibility",
]

APPFRAMEWORK_SRCS = glob(["AppFramework/**/*.m"])
COMMONLIB_SRCS = glob(["CommonLib/**/*.m"])
TESTLIB_SRCS = glob(["TestLib/**/*.m"]) + [
    "AppFramework/Action/GREYActionsShorthand.m",
    "AppFramework/Matcher/GREYMatchersShorthand.m",
]
UILIB_SRCS = glob(["UILib/**/*.m"])

objc_library(
    name = "TestLib",
    copts = ["-DSLOPPY_IMPORTS"],
    module_name = "TestLib",
    includes = ALL_INCLUDES,
    hdrs = glob([
        "AppFramework/**/*.h",
        "CommonLib/**/*.h",
        "TestLib/**/*.h",
        "UILib/**/*.h",
    ], exclude = ["TestLib/Swift/*.h"]),
    srcs = TESTLIB_SRCS + COMMONLIB_SRCS,
    deps = [
        "@com_github_google_edistantobject//:eDistantObject",
    ],
)

# This is the target that will actually be imported into our UI test targets
genrule(
    name = "Imports",
    outs = ["Imports.swift"],
    cmd = """cat <<-END > $@
@_exported import TestLib
END
"""
)

swift_library(
    name = "EarlGrey",
    srcs = glob(["TestLib/Swift/**/*.swift"]) + [":Imports"],
    visibility = ["//visibility:public"],
    deps = [
        ":TestLib",
    ],
)

# This target will get linked into our loadable IPC bundles, via a `AppFramework`
objc_library(
    name = "EarlGreyIPC",
    copts = ["-DSLOPPY_IMPORTS"],
    hdrs = glob([
        "AppFramework/**/*.h",
        "CommonLib/**/*.h",
        "UILib/**/*.h",
    ]),
    includes = ALL_INCLUDES,
    srcs = APPFRAMEWORK_SRCS + COMMONLIB_SRCS + UILIB_SRCS,
    module_name = "EarlGreyIPC",
    sdk_frameworks = [
        "CoreData",
        "CoreGraphics",
        "IOKit",
        "QuartzCore",
        "WebKit",
    ],
    visibility = ["//visibility:public"],
    deps = [
        "@com_github_facebook_fishhook//:fishhook",
        "@com_github_google_edistantobject//:eDistantObject",
    ],
)

# This target will get bundle into the app, only under test
ios_framework(
    name = "AppFramework",
    bundle_id = "com.google.earlgrey.AppFramework",
    bundle_only = True,
    families = ["iphone", "ipad"],
    infoplists = ["AppFramework/Info.plist"],
    minimum_os_version = "10.0",
    visibility = ["//visibility:public"],
    deps = [":EarlGreyIPC"],
)

# This needs to be public, to allow building the loadable bundle's AppFramework.framework
# (which is done outside of this repo, because of )
exports_files(["AppFramework/Info.plist"])

@brentleyjones
Copy link
Collaborator Author

@ajanuar Hopefully the above will set you in the right direction. I'll try to fix this PR soon, just been bogged down.

@ajanuar
Copy link

ajanuar commented Mar 3, 2021

thanks @erikkerber ! one more question, how to use Bundle in ios_ui_test?

@brentleyjones
Copy link
Collaborator Author

Bundle is in the bundles for the app under test. Interfaces is what you depend on in the UI test in order to invoke code in the bundle.

@erikkerber
Copy link
Contributor

erikkerber commented Mar 3, 2021

Yes. In your ios_application:

ios_application(
  ...
  bundles = select({
    "//:dbg": ["//src/EarlGreyIPC:Bundle"],
    "//conditions:default": [],
   })
   ...
}

And like Brentley said, Interfaces will be in your deps for your UI test target/BUILD file.

@ajanuar
Copy link

ajanuar commented Mar 3, 2021

Thanks! Will try today.

@brentleyjones
Copy link
Collaborator Author

Build is green. For this to get over the finish line it needs some tests. @erikkerber (or a delegate) or @ajanuar, could either of you contribute some?

@ajanuar
Copy link

ajanuar commented Mar 6, 2021

I can run our EarlGreyTest now, thanks all. And yes, I can help to add some tests.

@ajanuar
Copy link

ajanuar commented Mar 11, 2021

Hello guys,

I have case that need to create ios_bundle binary (type=loadable_bundle) with linker flags (-bundle_loader path/to/app/exe_bin).

if I do this, when I run EarlGreyTests (bazel test :UITest), it will detect there will be duplicate symbols from two different binaries (EarlGreyBundle & App) & App will be crash.

Is there any way to avoid this?

objc_library(
  name = "X",
  deps = ["Y", "Z"]
)

ios_bundle(
  name = "Bundle",
  deps = ["X"],
 ...
)

objc_library(
  name = "TestLib",
  srcs = [...],
)

ios_ui_test(
  name = "UITest",
  test_host = ":App",
  deps = ["TestLib"]
)

objc_library(
  name = "AppLib",
  srcs = [...],
  deps = ["Y", "Z"]
)

ios_application(
  name = "App",
  deps = ["AppLib"],
  bundles = ["Bundle"],
  ...
)

Note:
For comparison, in BUCK there is rule to create binary (https://buck.build/rule/apple_binary.html) which allow to provide srcs and linker_flags.

Example in BUCK

apple_binary(
  name = "LoadableBundle",
  srcs = [...],
  linker_flags = [
    "-bundle_loader",
    "/path/to/app/exe"
  ]
)

@brentleyjones
Copy link
Collaborator Author

Can you show an error? We never had duplicate symbol errors.

@ajanuar
Copy link

ajanuar commented Mar 11, 2021

An example:

Mar 11 22:17:09 Alberts-MacBook-Pro-2 SandboxApp[87775]: objc[87775]: Class CheckmarkItemCell is implemented in both /Users/albertjanuar/Library/Developer/CoreSimulator/Devices/FC18A482-CAF8-4D84-9C59-2438D3227792/data/Containers/Bundle/Application/BB27D7F3-C937-4ABF-BE04-D91B156F2A6D/SandboxApp.app/SandboxApp (0x10d1459d0) and /Users/albertjanuar/Library/Developer/CoreSimulator/Devices/FC18A482-CAF8-4D84-9C59-2438D3227792/data/Containers/Bundle/Application/BB27D7F3-C937-4ABF-BE04-D91B156F2A6D/SandboxApp.app/EarlGreyHelperBundles/CulinaryEarlGreyHelper.bundle/CulinaryEarlGreyHelper (0x14e07cb10). One of the two will be used. Which one is undefined.

It lists all public headers from Y & Z. Is it because objc_library's output a fully linked static library?

@erikkerber
Copy link
Contributor

I wouldn't expect something called CheckmarkItemCell to be a symbol in your EarlGrey bundle. Make sure you don't have common dependencies between your app and the CulinaryEarlGreyHelper?

@hendych
Copy link

hendych commented Mar 12, 2021

Hi @erikkerber @brentleyjones I just want to clarify what @ajanuar meant. Maybe this is different case by case (or maybe OOT, tied to specific earlgrey 2 usage).

In earlgrey, the helper bundle act as an extension of .app let's use https://github.com/google/EarlGrey/blob/earlgrey2/docs/setup.md and https://github.com/google/eDistantObject/blob/master/docs/setup.md#where-to-write-code as a sample.

From what I learn from the docs, FooClass will be linked only to the helper bundle (host) binary. In our case, FooClass also import Culinary module because it needs access to the module, while .app also linked to the Culinary module. (we used modular)

Thus, there will be 2 duplicate symbol in the .app and helper bundle binary. The duplicate error cause singleton classes not properly initialized.

Does it mean that we have to split shared header between Culinary and it's dependencies module too to be used by helper bundle? Or maybe just us that is wrong using it 😄

@brentleyjones
Copy link
Collaborator Author

We didn't have something like Culinary depended on by helper bundle binary. The bundle only provided interfaces that allowed communication with the app, the app had all of the dependencies.

This rule could be fixed to include the -bundle_loader, but it created a unique circular dependency that I didn't have time to go into. I would look at how tests and test_host work, and look at how the custom bundle location works with this PR, and determine a way to get it to behave more like tests (making the bundle "depend" on a certain app), instead of the model I followed which has the app depend on the bundle.

@hendych
Copy link

hendych commented Mar 12, 2021

We didn't have something like Culinary depended on by helper bundle binary. The bundle only provided interfaces that allowed communication with the app, the app had all of the dependencies.

This rule could be fixed to include the -bundle_loader, but it created a unique circular dependency that I didn't have time to go into. I would look at how tests and test_host work, and look at how the custom bundle location works with this PR, and determine a way to get it to behave more like tests (making the bundle "depend" on a certain app), instead of the model I followed which has the app depend on the bundle.

Noted on that. I understand better on how to use it. Thank you by the way!

@ajanuar
Copy link

ajanuar commented Mar 15, 2021

We didn't have something like Culinary depended on by helper bundle binary. The bundle only provided interfaces that allowed communication with the app, the app had all of the dependencies.

This rule could be fixed to include the -bundle_loader, but it created a unique circular dependency that I didn't have time to go into. I would look at how tests and test_host work, and look at how the custom bundle location works with this PR, and determine a way to get it to behave more like tests (making the bundle "depend" on a certain app), instead of the model I followed which has the app depend on the bundle.

Is the idea to add new parameter to ios_ui_test like test_host_bundles, and copy those bundles when the test is being bundled?

if test_host_archive:
direct_runfiles.append(test_host_archive)

@brentleyjones
Copy link
Collaborator Author

ios_unit_test has a test_host attribute that causes the test host to be the -bundle_loader for the unit test. I was saying to copy that logic, to allow a bundle to have a -bundle_loader. A bundle having a bundle loader doesn't depend on UI tests or unit test, so I wouldn't be adding fields to any test rules.

@ajanuar
Copy link

ajanuar commented Mar 17, 2021

Hi @brentleyjones , let me clarify my understanding, these rules below are in circular dependencies.

objc_library(
  name = "X",
)

ios_bundle(
  name = "Bundle",
  deps = [":X"],
  bundle_location = "EarlGreyHelperBundles",
  bundle_loader = ":App",
  ...
)

objc_library(
  name = "UITestLib",
)

ios_ui_test(
  name = "UITest",
  deps = ["UITestLib"],
  test_host = ":App",
)

objc_library(
  name = "AppLib",
)

ios_application(
  name = "App",
  deps = [":AppLib"],
  bundles = [":Bundle"], # We can't do this because of cyclic.
)

Assume we can use the ios_unit_test logic, then can we write these rules below?

objc_library(
  name = "X",
)

ios_bundle(
  name = "Bundle",
  deps = [":X"],
  bundle_location = "EarlGreyHelperBundles",
  bundle_loader = ":App",
  ...
)

objc_library(
  name = "UITestLib",
)

ios_ui_test(
  name = "UITest",
  deps = ["UITestLib"],
  test_host = ":App",
)

objc_library(
  name = "AppLib",
)

ios_application(
  name = "App",
  deps = [":AppLib"],
)

I am available to help if you can guide me steps to enable this. thanks!

@brentleyjones
Copy link
Collaborator Author

Yes, that is the correct understanding.

@ajanuar
Copy link

ajanuar commented Mar 22, 2021

Hi @brentleyjones , sharing a little update. we've successfully built ios bundle with workaround for our EarlGrey test case. This won't be ideal, but it runs perfectly for us.

objc_library(
  name = "X",
)

ios_bundle(
  name = "Bundle",
  deps = [":X"],
  bundle_location = "EarlGreyHelperBundles",
  bundle_loader = ":FakeApp", #workaround
  ...
)

objc_library(
  name = "UITestLib",
)

ios_ui_test(
  name = "UITest",
  deps = ["UITestLib"],
  test_host = ":App",
)

objc_library(
  name = "AppLib",
)

ios_application(
  name = "FakeApp",
  deps = [":AppLib"],
)

ios_application(
  name = "App",
  deps = [":AppLib"],
  bundles = [":Bundle"], # We can't do this because of cyclic.
)

I also create PR to add test #1108, not sure these tests are sufficient. Thanks!

@dwirandyh
Copy link

Hi guys, may i know is there any chance to merge this PR into master branch?

@brentleyjones
Copy link
Collaborator Author

If anyone wants an ios_bundle rule, feel free to open a PR that adds it. It needs to address my concerns about the bundle loader though. I don't have plans to update and land this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

macos_bundle with ios base_sdk
5 participants