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.

Sign up for GitHub

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

Jump to bottom

Handle multiple alwayslink error in objc_library #938

Merged
merged 3 commits into from
Jan 21, 2025
Merged

Handle multiple alwayslink error in objc_library #938

merged 3 commits into from
Jan 21, 2025

Conversation

ssaradp
Copy link
Collaborator

@ssaradp ssaradp commented Jan 20, 2025 •
edited
Loading

Allow alwayslink to be passed in apple_framework but set a default value to True so we handle errors as such

	File "/Users/ssarad/.bazel_cache/output/external/BUILD.bazel", line 5, column 16, in <toplevel>
		apple_framework(
	File "/Users/ssarad/.bazel_cache/output/external/rules_ios~/rules/framework.bzl", line 98, column 28, in apple_framework
		library = apple_library(
	File "/Users/ssarad/.bazel_cache/output/external/rules_ios~/rules/library.bzl", line 1109, column 24, in apple_library
		native.objc_library(
Error in objc_library: rule(...) got multiple values for parameter 'alwayslink'

@ssaradp ssaradp marked this pull request as ready for review January 20, 2025 20:41
jszumski
jszumski approved these changes Jan 21, 2025
View reviewed changes
rules/library.bzl Outdated
@@ -1124,7 +1125,7 @@ def apple_library(
defines = defines + objc_defines,
testonly = testonly,
features = features,
alwayslink = True, # ensure symbols from any static deps are always included (see https://github.com/bazelbuild/rules_apple/issues/1938)
alwayslink = default_alwayslink, # ensure symbols from any static deps are always included (see https://github.com/bazelbuild/rules_apple/issues/1938)
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I'd probably drop this comment and move it up to the default_alwayslink declaration

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

moved! I'll enable auto-merge

@ssaradp ssaradp enabled auto-merge (squash) January 21, 2025 16:10
@luispadron luispadron disabled auto-merge January 21, 2025 16:27
@luispadron luispadron enabled auto-merge (squash) January 21, 2025 16:27
@luispadron luispadron merged commit 99632e3 into bazel-ios:master Jan 21, 2025
12 checks passed
@ssaradp ssaradp deleted the sarad/alwayslink-extraargs branch January 21, 2025 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jszumski jszumski jszumski approved these changes

@jschear jschear jschear approved these changes

@luispadron luispadron luispadron approved these changes

@mccorkill1 mccorkill1 mccorkill1 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ssaradp @jszumski @jschear @luispadron @mccorkill1