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

incompatible_depset_for_libraries_to_link_getter: use depsets in linking_context in C++ Starlark API #8118

Closed
oquenchil opened this issue Apr 23, 2019 · 3 comments

Comments

Projects
None yet
4 participants
@oquenchil
Copy link
Contributor

commented Apr 23, 2019

Flag: --incompatible_depset_for_libraries_to_link_getter
Available since: 0.26 (June 2019 release)
Will be flipped in: 0.27 (September 2019 release)
Tracking issue: #8118

linking_context.libraries_to_link will return a depset instead of a list. To make code compatible with the list and the depset, simply check whether the value returned from linking_context.libraries_to_link has the attribute "to_list".

libraries_to_link = linking_context.libraries_to_link
if hasattr(libraries_to_link, "to_list"):
   # It's a depset
else:
  # It's a list
@mboes

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2019

How will I be able to use the resulting depset? Will I be able to pass it directly as an input to ctx.actions.run()? If not, then I will have to traverse the depset in order to extract e.g. all the dynamic libraries and feed that as the inputs. Traversing the depset will be a performance pessimization.

I often need the dynamic libraries for purposes other than feeding them to a linker, e.g. to provide them to an interpreter for code that depends on native libraries.

@benjaminp

This comment has been minimized.

Copy link
Collaborator

commented Apr 26, 2019

See #7626. It's already a depset internal, and the current API is flattening the depset every time its referenced.

@mboes

This comment has been minimized.

Copy link
Contributor

commented Apr 26, 2019

Sure, but my question is what about the future? Will there be a way to use the depset without flattening it to eagerly? Internally, LibrariesToLink is something like depset(LibraryToLink) AFAIU. But the inputs argument to ctx.actions.run() is expected to be of type depset(File). So how will this work? Will I be expected to convert depset(LibraryToLink) to depset(File)?

bazel-io pushed a commit that referenced this issue Apr 29, 2019

C++: Make linking_context libraries_to_link return depset
Triggered by the flag: --incompatible_depset_for_libraries_to_link_getter

GitHub tracking issue: #8118

RELNOTES:none
PiperOrigin-RevId: 245747705

@oquenchil oquenchil changed the title incompatible_depsets_in_linking_context_getters: change to getters of linking_context in C++ Starlark API incompatible_depset_for_libraries_to_link_getter of linking_context in C++ Starlark API May 23, 2019

@hlopko hlopko changed the title incompatible_depset_for_libraries_to_link_getter of linking_context in C++ Starlark API incompatible_depset_for_libraries_to_link_getter: use depsets in linking_context in C++ Starlark API May 29, 2019

bazel-io pushed a commit that referenced this issue May 31, 2019

C++: Flips incompatible_depset_for_libraries_to_link_getter
Testing downstream works https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/994

upb breakage is unrelated. Filed protocolbuffers/upb#174 just in case.

GH issue: #8118

Please do not rollback, fix is very easy following instructions in GH issue.

RELNOTES:none
PiperOrigin-RevId: 250844223

@laurentlb laurentlb closed this May 31, 2019

irengrig added a commit to irengrig/bazel that referenced this issue Jun 18, 2019

C++: Flips incompatible_depset_for_libraries_to_link_getter
Testing downstream works https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/994

upb breakage is unrelated. Filed protocolbuffers/upb#174 just in case.

GH issue: bazelbuild#8118

Please do not rollback, fix is very easy following instructions in GH issue.

RELNOTES:none
PiperOrigin-RevId: 250844223

irengrig added a commit to irengrig/bazel that referenced this issue Jul 15, 2019

C++: Flips incompatible_depset_for_libraries_to_link_getter
Testing downstream works https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/994

upb breakage is unrelated. Filed protocolbuffers/upb#174 just in case.

GH issue: bazelbuild#8118

Please do not rollback, fix is very easy following instructions in GH issue.

RELNOTES:none
PiperOrigin-RevId: 250844223
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.