Skip to content

Commit

Permalink
include dep in symlink tree if lib output style is shared
Browse files Browse the repository at this point in the history
Summary:
NOTE: `runtime_dependency_handling` hasn't been used yet, so this will not impact existing builds.

As discussed previously (see bottom of diff summary), compilation is broken under some scenarios when `runtime_dependency_handling = "symlink"`, which will prevent this setting from being rolled out unconditionally (or at least more broadly) across arvr.

I think I figured out why and how to fix it.

Looking at this table (the cells are the lib_output_style):

    --------------------------------------------------------------
    | preferred_linkage |              link_strategy             |
    |                   |----------------------------------------|
    |                   | static   | static_pic   |   shared     |
    -------------------------------------------------------------|
    |      static       | *archive | *pic_archive | *pic_archive |
    |      shared       | shared   |   shared     |   shared     |
    |       any         | *archive | *pic_archive |   shared     |
    --------------------------------------------------------------

I think this is the behavior we want:

```
if lib_output_style is shared:
    include all shlibs in symlink tree
```

Whereas we are are currently doing:

```
if runtime_dependency_handling is symlink or link_strategy is shared:
    include all shlibs in symlink tree
```

This row is problematic under the current implementation:

    --------------------------------------------------------------
    | preferred_linkage |              link_strategy             |
    |                   |----------------------------------------|
    |                   | static   | static_pic   |   shared     |
    -------------------------------------------------------------|
    |       any         | *archive | *pic_archive |   shared     |
    --------------------------------------------------------------

If link_strategy is *not* shared, then the archive or pic_archive version of the library is linked. If runtime_dependency_handling is set to symlink mode, then the shared library will *also* be included in the symlink tree, which seems to break the compilation.

To fix this, we can look at the lib_output_style of each dependency to determine what is included in the symlink tree instead of using just the link strategy.

Reviewed By: jtbraun

Differential Revision: D67610026

fbshipit-source-id: f44da42372197ed3e6f23b9cb7dd390d57ffd7a4
  • Loading branch information
jrodal98 authored and facebook-github-bot committed Jan 6, 2025
1 parent 236be3c commit 86209bb
Showing 1 changed file with 15 additions and 1 deletion.
16 changes: 15 additions & 1 deletion prelude/cxx/cxx_executable.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ load(
"@prelude//cxx:cxx_bolt.bzl",
"cxx_use_bolt",
)
load(
"@prelude//cxx:cxx_toolchain_types.bzl",
"PicBehavior",
)
load(
"@prelude//cxx:link_groups_types.bzl",
"LinkGroupsDebugLinkInfo",
Expand Down Expand Up @@ -56,13 +60,15 @@ load(
)
load(
"@prelude//linking:link_info.bzl",
"LibOutputStyle",
"LinkArgs",
"LinkCommandDebugOutput",
"LinkInfo",
"LinkOrdering", # @unused Used as a type
"LinkStrategy",
"LinkedObject", # @unused Used as a type
"ObjectsLinkable",
"get_lib_output_style",
"make_link_command_debug_output",
"make_link_command_debug_output_json_info",
"to_link_strategy",
Expand Down Expand Up @@ -515,11 +521,19 @@ def cxx_executable(ctx: AnalysisContext, impl_params: CxxRuleConstructorParams,
# Only setup a shared library symlink tree when shared linkage or link_groups is used
gnu_use_link_groups = cxx_is_gnu(ctx) and len(link_group_mappings) > 0
shlib_deps = []
if impl_params.runtime_dependency_handling == RuntimeDependencyHandling("symlink") or link_strategy == LinkStrategy("shared") or gnu_use_link_groups:
if link_strategy == LinkStrategy("shared") or gnu_use_link_groups:
shlib_deps = (
[d.shared_library_info for d in link_deps] +
[d.shared_library_info for d in impl_params.extra_link_roots]
)
elif impl_params.runtime_dependency_handling == RuntimeDependencyHandling("symlink"):
for d in link_deps + impl_params.extra_link_roots:
if d.linkable_graph == None:
continue
preferred_linkage = d.linkable_graph.nodes.value.linkable.preferred_linkage
output_style = get_lib_output_style(link_strategy, preferred_linkage, PicBehavior("supported"))
if output_style == LibOutputStyle("shared_lib"):
shlib_deps.append(d.shared_library_info)

shlib_info = merge_shared_libraries(ctx.actions, deps = shlib_deps)

Expand Down

0 comments on commit 86209bb

Please sign in to comment.