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

Reland: Retrieve single arch cpu from cc_toolchain instead of from ctx.fragments.apple #749

Conversation

thii
Copy link
Member

@thii thii commented Jan 25, 2022

ctx.fragments.apple.single_arch_cpu is returning the default macos
cpu for the host platform for tools while it should return the cpu of
the execution platform.

The original patch relied on ctx.fragments.apple.single_arch_platform
which can also be wrong in some cases. Now we only look at the platform
prefix in the cpu string instead.

There is a caveat with this workaround: When building a swift_library
directly without a --cpu flag, the cpu value would be the cpu value
of the exec platform, regardless of the --apple_platform_type value.

thii added a commit to thii/rules_apple that referenced this pull request Jan 25, 2022
Comment on lines 673 to 674
cpu = _single_arch_cpu(cc_toolchain)
platform = apple_fragment.single_arch_platform
Copy link
Member

Choose a reason for hiding this comment

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

not sure which cases this is wrong, but I assume single_arch_cpu + single_arch_platform would be "wrong together", vs now picking from the toolchain cpu + the platform here, will that cause mismatches?

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like the platform was correct, only the cpu was wrong. cc_toolchain returns the exec cpu if you don't pass a target cpu (or if it's not behind a transition that passes one). The command that causes the failure were:

https://github.com/bazelbuild/rules_apple/blob/76fce2997cf536cc60bd22861ba3554b5d37b0fa/test/ios_framework_import_test.sh#L104-L108

I tested building a cc_library and it behaved the same way, not sure if it's intentional or not.

/tmp/test » bazel build -s --apple_platform_type=ios my_cc_lib
INFO: Analyzed target //:my_cc_lib (36 packages loaded, 248 targets configured).
INFO: Found 1 target...
SUBCOMMAND: # //:my_cc_lib [action 'Compiling empty.cc', configuration: c385b0b904b06189db9fdf2833ddb9ef733c132f2a6eb928f8d74adca7ae3091, execution platform: @local_config_platform//:host]
(cd /private/var/tmp/_bazel_admin/1063c9c2120b66f3130a9f592ab85144/execroot/__main__ && \
  exec env - \
    APPLE_SDK_PLATFORM=MacOSX \
    APPLE_SDK_VERSION_OVERRIDE=11.3 \
    PATH=/bin:/usr/bin:/usr/local/bin \
    XCODE_VERSION_OVERRIDE=13.0.0.13A233 \
    ZERO_AR_DATE=1 \
  external/local_config_cc/wrapped_clang_pp '-D_FORTIFY_SOURCE=1' -fstack-protector -fcolor-diagnostics -Wall -Wthread-safety -Wself-assign -fno-omit-frame-pointer -O0 -DDEBUG '-std=c++11' 'DEBUG_PREFIX_MAP_PWD=.' -iquote . -iquote bazel-out/darwin-fastbuild/bin -MD -MF bazel-out/darwin-fastbuild/bin/_objs/my_cc_lib/empty.d '-frandom-seed=bazel-out/darwin-fastbuild/bin/_objs/my_cc_lib/empty.o' -isysroot __BAZEL_XCODE_SDKROOT__ -F__BAZEL_XCODE_SDKROOT__/System/Library/Frameworks -F__BAZEL_XCODE_DEVELOPER_DIR__/Platforms/iPhoneSimulator.platform/Developer/Library/Frameworks '-mmacosx-version-min=15.0' -no-canonical-prefixes -pthread -no-canonical-prefixes -Wno-builtin-macro-redefined '-D__DATE__="redacted"' '-D__TIMESTAMP__="redacted"' '-D__TIME__="redacted"' -target x86_64-apple-macosx -c empty.cc -o bazel-out/darwin-fastbuild/bin/_objs/my_cc_lib/empty.o)
# Configuration: c385b0b904b06189db9fdf2833ddb9ef733c132f2a6eb928f8d74adca7ae3091
# Execution platform: @local_config_platform//:host
SUBCOMMAND: # //:my_cc_lib [action 'Linking libmy_cc_lib.a', configuration: c385b0b904b06189db9fdf2833ddb9ef733c132f2a6eb928f8
d74adca7ae3091, execution platform: @local_config_platform//:host]
(cd /private/var/tmp/_bazel_admin/1063c9c2120b66f3130a9f592ab85144/execroot/__main__ && \
  exec env - \
    APPLE_SDK_PLATFORM=MacOSX \
    APPLE_SDK_VERSION_OVERRIDE=11.3 \
    PATH=/bin:/usr/bin:/usr/local/bin \
    XCODE_VERSION_OVERRIDE=13.0.0.13A233 \
    ZERO_AR_DATE=1 \
  external/local_config_cc/libtool @bazel-out/darwin-fastbuild/bin/libmy_cc_lib.a-2.params)
# Configuration: c385b0b904b06189db9fdf2833ddb9ef733c132f2a6eb928f8d74adca7ae3091
# Execution platform: @local_config_platform//:host
INFO: From Linking libmy_cc_lib.a:
warning: /Applications/Xcode-13.0.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: archive library: bazel-out/darwin-fastbuild/bin/libmy_cc_lib.a the table of contents is empty (no object file members in the library define global symbols)
Target //:my_cc_lib up-to-date:
  bazel-bin/libmy_cc_lib.a

Copy link
Member

Choose a reason for hiding this comment

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

I think folks rely on this working for Swift library targets specifically. It would be nice if it worked for the cc libraries to but I guess I'm less surprised but it doesn't

Copy link
Member Author

Choose a reason for hiding this comment

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

Added 6673a36 to continue supporting that.

…`ctx.fragments.apple`

`ctx.fragments.apple.single_arch_cpu` is returning the default macos
cpu for the host platform for tools while it should return the cpu of
the execution platform.

There is a caveat with this workaround: When building a `swift_library`
directly without a `--cpu` flag, the `cpu` value would be the cpu value
of the exec platform, regardless of the `--apple_platform_type` value.
@thii thii force-pushed the reland-retrieve-single-arch-cpu-from-cc_toolchain-instead-of-from-ctx.fragments.apple branch from 7ff0857 to 255b87b Compare January 26, 2022 03:43
This ensures building `swift_library` with a non-default platform type
(e.g. `--apple_platform_type=ios`) continues to work.
@thii thii marked this pull request as ready for review January 26, 2022 22:19
@thii thii merged commit cec0f85 into bazelbuild:master Jan 26, 2022
@thii thii deleted the reland-retrieve-single-arch-cpu-from-cc_toolchain-instead-of-from-ctx.fragments.apple branch January 26, 2022 22:20
tymurmustafaiev pushed a commit to tymurmustafaiev/rules_swift that referenced this pull request Jul 19, 2023
…`ctx.fragments.apple` when building for darwin cpus (bazelbuild#749)

Workaround for bazelbuild/bazel#14291.

`ctx.fragments.apple.single_arch_cpu` is returning the default macos
cpu for the host platform for tools while it should return the cpu of
the execution platform.
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 this pull request may close these issues.

None yet

2 participants