bazel-ios / rules_ios

Bazel rules for building iOS applications and frameworks
Apache License 2.0
278 stars 85 forks source link

precompiled_apple_resource_bundle causing AppStore rejection #800

Closed andre-alves closed 1 year ago

andre-alves commented 1 year ago

Hello!

My app started getting rejected from AppStore after updating to rules_ios 3.1.0 and rules_apple 3.1.1, with error:

ITMS-90207: Invalid Bundle - The bundle at '/Payload/MyApp.app/AccountStrings.bundle' does not contain a bundle executable.

The Info.plist I provide to precompiled_apple_resource_bundle:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
    <key>CFBundleName</key>
    <string>$(PRODUCT_NAME)</string>
    <key>CFBundleVersion</key>
    <string>1.0</string>
    <key>CFBundleShortVersionString</key>
    <string>1.0</string>
    <key>CFBundleIdentifier</key>
    <string>$(PRODUCT_BUNDLE_IDENTIFIER)</string>
    <key>CFBundlePackageType</key>
    <string>BNDL</string>
</dict>
</plist>

Checking the compiled Info.plist inside the .bundle, there is a new key which is probably causing the rejection:

    <key>CFBundleExecutable</key>
    <string></string>

This key doesn't exist when using apple_resource_bundle from rules_apple.

I used rules_ios 2.1.0 and rules_apple 2.5.0 before and never had this issue before.

luispadron commented 1 year ago

Do you happen to have a diff/version of the plist with rules_ios 2.1.0?

luispadron commented 1 year ago

Looking at the history I cant really see where that value was changed in the 3.0 bump

andre-alves commented 1 year ago

Old (rules_ios 2.1.0 and rules_apple 2.5.0):

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
  <dict>
    <key>BuildMachineOSBuild</key>
    <string>22G120</string>
    <key>CFBundleIdentifier</key>
    <string>com.myapp.resource.AccountStrings</string>
    <key>CFBundleName</key>
    <string>.bundle</string>
    <key>CFBundlePackageType</key>
    <string>BNDL</string>
    <key>CFBundleShortVersionString</key>
    <string>1.0</string>
    <key>CFBundleSupportedPlatforms</key>
    <array>
      <string>iPhoneOS</string>
    </array>
    <key>CFBundleVersion</key>
    <string>1.0</string>
    <key>DTCompiler</key>
    <string>com.apple.compilers.llvm.clang.1_0</string>
    <key>DTPlatformBuild</key>
    <string>21A325</string>
    <key>DTPlatformName</key>
    <string>iphoneos</string>
    <key>DTPlatformVersion</key>
    <string>17.0</string>
    <key>DTSDKBuild</key>
    <string>21A325</string>
    <key>DTSDKName</key>
    <string>iphoneos17.0</string>
    <key>DTXcode</key>
    <string>1500</string>
    <key>DTXcodeBuild</key>
    <string>15A240d</string>
    <key>MinimumOSVersion</key>
    <string>17.0</string>
    <key>UIDeviceFamily</key>
    <array>
      <integer>1</integer>
      <integer>2</integer>
    </array>
  </dict>
</plist>

New (rules_ios 3.1.0 and rules_apple 3.1.1):

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
  <dict>
    <key>BuildMachineOSBuild</key>
    <string>22G120</string>
    <key>CFBundleExecutable</key>
    <string></string>
    <key>CFBundleIdentifier</key>
    <string>com.myapp.resource.AccountStrings</string>
    <key>CFBundleName</key>
    <string>.bundle</string>
    <key>CFBundlePackageType</key>
    <string>BNDL</string>
    <key>CFBundleShortVersionString</key>
    <string>1.0</string>
    <key>CFBundleSupportedPlatforms</key>
    <array>
      <string>iPhoneOS</string>
    </array>
    <key>CFBundleVersion</key>
    <string>1.0</string>
    <key>DTCompiler</key>
    <string>com.apple.compilers.llvm.clang.1_0</string>
    <key>DTPlatformBuild</key>
    <string>21A325</string>
    <key>DTPlatformName</key>
    <string>iphoneos</string>
    <key>DTPlatformVersion</key>
    <string>17.0</string>
    <key>DTSDKBuild</key>
    <string>21A325</string>
    <key>DTSDKName</key>
    <string>iphoneos17.0</string>
    <key>DTXcode</key>
    <string>1500</string>
    <key>DTXcodeBuild</key>
    <string>15A240d</string>
    <key>MinimumOSVersion</key>
    <string>17.0</string>
    <key>UIDeviceFamily</key>
    <array>
      <integer>1</integer>
      <integer>2</integer>
    </array>
  </dict>
</plist>

The only difference is the new key:

    <key>CFBundleExecutable</key>
    <string></string>
luispadron commented 1 year ago

I see.. I wonder if we need to set this to False now? https://github.com/bazel-ios/rules_apple/blob/b43c7f60b584d68d7d187c236d4328162ba2e806/apple/internal/resource_actions/plist.bzl#L226

cc: @jerrymarino

luispadron commented 1 year ago

Although it seems like the key shouldn't be included because we set this to None in the rule: https://github.com/bazel-ios/rules_apple/blob/b43c7f60b584d68d7d187c236d4328162ba2e806/apple/internal/resource_actions/plist.bzl#L278

luispadron commented 1 year ago

Seems like we have custom logic for this that might need to be fixed up: https://github.com/bazel-ios/rules_ios/blob/2972023f32434c18783fbb98382d1b8ae5d226d0/rules/precompiled_apple_resource_bundle.bzl#L112-L117

andre-alves commented 1 year ago

Although it seems like the key shouldn't be included because we set this to None in the rule: https://github.com/bazel-ios/rules_apple/blob/b43c7f60b584d68d7d187c236d4328162ba2e806/apple/internal/resource_actions/plist.bzl#L278

Turns out the condition changed, it's now or:

https://github.com/bazelbuild/rules_apple/commit/f909b0b6f31dee7ee9ba9998a92404eaeb7af727#diff-fd21ff183af5a0af8861d50117a7a377b04a0b7d9924600f424261b7db57e2deL279

luispadron commented 1 year ago

Nice find, can you try this branch @andre-alves: https://github.com/bazel-ios/rules_ios/pull/801

andre-alves commented 1 year ago

Sure, thanks for the quick response @luispadron!

I will try submitting it to AppStore using your branch.

luispadron commented 1 year ago

Could you try just checking the plist changes if you get a chance, that might be faster.

andre-alves commented 1 year ago

Confirmed, CFBundleExecutable is no longer added.

Thank you!

luispadron commented 1 year ago

Sweet, I'll merge and release a patch version. Thanks for reporting and following up

andre-alves commented 1 year ago

AppStore finished processing, all good as well!