-
Notifications
You must be signed in to change notification settings - Fork 268
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
Move -install_name
linker flag from the {ios,tvos}_framework
macro into the rule implementation.
#1207
Conversation
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
…o into the rule implementation. PiperOrigin-RevId: 382086173
e75e115
to
43708ba
Compare
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
Depends on #1188 |
43708ba
to
d72ca65
Compare
Lyft integration job started: https://buildkite.com/lyft/rules-apple/builds/170 (must be Lyft employee to view) |
@@ -680,6 +665,25 @@ def _ios_framework_impl(ctx): | |||
res_attrs = ["resources"], | |||
) | |||
|
|||
extra_linkopts = [ | |||
"-dynamiclib", | |||
"-Wl,-install_name,@rpath/{name}{extension}/{name}".format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used this version to work around https://github.com/bazelbuild/bazel/blob/1c3a2456c95fd19974a5b2bd33c5ebdb2b2277e4/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java#L295 causing an issue. Think this is bug worthy?
One nice thing about doing it this way: it's the same argument that was being sent before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Yes I think we should report it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, it should have re-branched, not closed this... |
…o into the rule implementation. PiperOrigin-RevId: 382086173 (cherry picked from commit 172ad76) This also changes the arguments to use `-Wl,-install_name,@rpath` to work around an issue in bazel: https://github.com/bazelbuild/bazel/blob/1c3a2456c95fd19974a5b2bd33c5ebdb2b2277e4/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java#L295
…nto the rule implementation.
d72ca65
to
b43a777
Compare
Lyft integration job started: https://buildkite.com/lyft/rules-apple/builds/172 (must be Lyft employee to view) |
PiperOrigin-RevId: 382086173
(cherry picked from commit 172ad76)
Conflicts:
apple/internal/ios_rules.bzl
apple/internal/tvos_rules.bzl
apple/ios.bzl
apple/tvos.bzl