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

Fix alwayslink in objc_import #15718

Merged
merged 1 commit into from
Jun 22, 2022
Merged

Fix alwayslink in objc_import #15718

merged 1 commit into from
Jun 22, 2022

Conversation

ckolli5
Copy link

@ckolli5 ckolli5 commented Jun 22, 2022

When ObjcCommon was rewritten in Starlark in 4a0cc3b the archives list from objc_import (passed in via extra_import_libraries) got dropped from the force_load_library list in objc_provider.

This fix adds them back so -force_load is correctly applied when alwayslink is set.

Here is the original Java implementation for reference:

if (alwayslink) {
for (CompilationArtifacts artifacts : compilationArtifacts.asSet()) {
for (Artifact archive : artifacts.getArchive().asSet()) {
objcProvider.add(FORCE_LOAD_LIBRARY, archive);
}
}
for (Artifact archive : extraImportLibraries) {
objcProvider.add(FORCE_LOAD_LIBRARY, archive);
}
}

This for loop is what was omitted in the rewrite and is what is added in this PR:

for (Artifact archive : extraImportLibraries) {
  objcProvider.add(FORCE_LOAD_LIBRARY, archive);
}

Closes #15313.

PiperOrigin-RevId: 455164591
Change-Id: Icc0a5aab26ec150475d82b57549b263418776141

When `ObjcCommon` was rewritten in Starlark in 4a0cc3b the `archives` list from `objc_import` (passed in via `extra_import_libraries`) got dropped from the `force_load_library` list in `objc_provider`.

This fix adds them back so `-force_load` is correctly applied when `alwayslink` is set.

Here is the original Java implementation for reference:
https://github.com/bazelbuild/bazel/blob/4a0cc3b3f297f8df60022ae977e170148a4c7ae4/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java#L415-L424

This `for` loop is what was omitted in the rewrite and is what is added in this PR:
```java
for (Artifact archive : extraImportLibraries) {
  objcProvider.add(FORCE_LOAD_LIBRARY, archive);
}
```

Closes #15313.

PiperOrigin-RevId: 455164591
Change-Id: Icc0a5aab26ec150475d82b57549b263418776141
@ckolli5 ckolli5 merged commit d8965c3 into bazelbuild:release-5.3.0 Jun 22, 2022
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.

1 participant