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

Setting --apple_platform_type=ios no longer sets APPLE_PLATFORM_SDK #897

Closed
erikkerber opened this issue Sep 6, 2022 · 10 comments
Closed

Comments

@erikkerber
Copy link

erikkerber commented Sep 6, 2022

If setting --apple_platform_type=ios on a swift_library target, the APPLE_PLATFORM_SDK is still passing MacOSX

bazel build //A:SwiftLibrary --apple_platform_type=ios 
...
ERROR: /Users/ekerber/dev/A/BUILD.bazel:3:22: Compiling Swift module //A:SwiftLibrary failed: (Exit 1): worker failed: error executing command
  (cd /private/var/tmp/_bazel_ekerber/dd323d2a4ff53820c11ccc0d5aebe791/execroot/slack-objc && \
  exec env - \
    APPLE_SDK_PLATFORM=MacOSX \
    APPLE_SDK_VERSION_OVERRIDE=12.3 \
    SWIFT_AVOID_WARNING_USING_OLD_DRIVER=1 \
    XCODE_VERSION_OVERRIDE=13.4.1.13F100 \
  bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/external/build_bazel_rules_swift/tools/worker/worker swiftc @bazel-out/darwin_arm64-dbg/bin/A/SwiftLibrary.swiftmodule-0.params)
# Configuration: 3d388c41403a1b573b39ec2d1e1f088b0392936d11a52587a50d61d38b79fd02
# Execution platform: @local_config_platform//:host
./Users/ekerber/dev/A/Sources/Foo.swift:9:8: error: no such module 'UIKit'
import UIKit

Repros after #858

@BalestraPatrick
Copy link
Member

I think we're seeing the same bug.

@thii
Copy link
Member

thii commented Sep 9, 2022

Are you not passing --cpu at all? I think that is required. rules_swift doesn't do anything to change the target CPU based on the platform.

@thii
Copy link
Member

thii commented Sep 9, 2022

#899 should address this, but I'm really not sure if we need to derive a default cpu from the platform type.

@erikkerber
Copy link
Author

Right, we aren't setting the cpu. If cpu should be explicit to change to iOS that's probably fine, just a change in behavior from before.

@thii
Copy link
Member

thii commented Sep 14, 2022

I think before that change, if you don't set --cpu, the linking action still gets a wrong APPLE_SDK_PLATFORM.

@erikkerber
Copy link
Author

Tested on f805ee0:

bazel aquery //My:Target --apple_platform_type=ios | grep APPLE_SDK_PLATFORM

...
  Environment: [APPLE_SDK_PLATFORM=iPhoneSimulator, APPLE_SDK_VERSION_OVERRIDE=16.0, SWIFT_AVOID_WARNING_USING_OLD_DRIVER=1, XCODE_VERSION_OVERRIDE=14.0.0.14A309]
  Environment: [APPLE_SDK_PLATFORM=iPhoneSimulator, APPLE_SDK_VERSION_OVERRIDE=16.0, SWIFT_AVOID_WARNING_USING_OLD_DRIVER=1, XCODE_VERSION_OVERRIDE=14.0.0.14A309]
...

Then on 2546e60

bazel aquery //My:Target --apple_platform_type=ios | grep APPLE_SDK_PLATFORM

...
  Environment: [APPLE_SDK_PLATFORM=MacOSX,, APPLE_SDK_VERSION_OVERRIDE=16.0, SWIFT_AVOID_WARNING_USING_OLD_DRIVER=1, XCODE_VERSION_OVERRIDE=14.0.0.14A309]
  Environment: [APPLE_SDK_PLATFORM=MacOSX,, APPLE_SDK_VERSION_OVERRIDE=16.0, SWIFT_AVOID_WARNING_USING_OLD_DRIVER=1, XCODE_VERSION_OVERRIDE=14.0.0.14A309]
...

Acknowledging that might have been unexpected behavior before, but it is something our developers had gotten used to using to quick build swift_library targets against iOS SDKs.

@thii
Copy link
Member

thii commented Sep 14, 2022

Ah sorry I was testing a swift_binary

@thii
Copy link
Member

thii commented Sep 21, 2022

Closing this since it looks like an *_library rule shouldn't perform a transition on its own. Related issue bazelbuild/bazel#16099

@thii thii closed this as completed Sep 21, 2022
@keith
Copy link
Member

keith commented Jan 20, 2023

More discussion about this on bazelbuild/rules_apple#1811 as well. As far as it relates to this I think even before that change that broke this, you wouldn't have shared caches with the library and when you built the library as a transitive of an app or test? If that's true maybe it's good that it stopped working since that in itself is surprising 🤔

@erikkerber
Copy link
Author

Right, caches weren't shared. Devs had been used to using this flag in a pinch to have, to put it simply, their basic command-line builds resolve errors finding UIKit.

I know the "answer" is to have them build something that provides the transition, our devs had been used to fixing this on the fly using that papercut.

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

Successfully merging a pull request may close this issue.

4 participants