Stop using a filelist for linker invocations (attempt 2)

Summary: This is a modified version of D68593087. We still remove the filelist, but keep the linker order exactly the same. We have too many tests implicitly relying on link order that break if we modify the link order, so although it is somewhat arbitrary keep it the same.

Reviewed By: rmaz

Differential Revision: D69531936

fbshipit-source-id: a6fbfa52054cd302bf4ef4c5b076b62790b16c90

prelude/cxx/cxx_executable.bzl

@@ -784,9 +784,6 @@ def cxx_executable(ctx: AnalysisContext, impl_params: CxxRuleConstructorParams,
     sub_targets["linker.argsfile"] = [DefaultInfo(
         default_output = binary.linker_argsfile,
     )]
-    sub_targets["linker.filelist"] = [DefaultInfo(
-        default_outputs = filter(None, [binary.linker_filelist]),
-    )]
 
     link_cmd_debug_output = make_link_command_debug_output(binary)
     if link_cmd_debug_output != None:

prelude/cxx/cxx_library.bzl

@@ -1336,9 +1336,6 @@ def _form_library_outputs(
                         "linker.command": [DefaultInfo(
                             default_outputs = filter(None, [link_cmd_debug_output_file]),
                         )],
-                        "linker.filelist": [DefaultInfo(
-                            default_outputs = filter(None, [shlib.linker_filelist]),
-                        )],
                         "unstripped": [DefaultInfo(
                             default_output = unstripped,
                         )],

prelude/cxx/cxx_link_utility.bzl

@@ -20,8 +20,8 @@ load(
     "LinkArgs",
     "LinkOrdering",  # @unused Used as a type
     "unpack_link_args",
-    "unpack_link_args_excluding_filelist",
-    "unpack_link_args_filelist",
+    "unpack_link_args_excluding_object_files_and_lazy_archives",
+    "unpack_link_args_object_files_and_lazy_archives_only",
 )
 load("@prelude//linking:lto.bzl", "LtoMode")
 load(
@@ -69,12 +69,6 @@ LinkArgsOutput = record(
     link_args = ArgLike,
     hidden = list[typing.Any],
     pdb_artifact = Artifact | None,
-    # The filelist artifact which contains the list of all object files.
-    # Only present for Darwin linkers. Note that object files referenced
-    # _inside_ the filelist are _not_ part of the `hidden` field above.
-    # That's by design - we do not want to materialise _all_ object files
-    # to inspect the filelist. Intended to be used for debugging.
-    filelist = Artifact | None,
 )
 
 def get_extra_darwin_linker_flags() -> cmd_args:
@@ -88,15 +82,13 @@ def make_link_args(
         actions: AnalysisActions,
         cxx_toolchain_info: CxxToolchainInfo,
         links: list[LinkArgs],
-        suffix = None,
         output_short_path: [str, None] = None,
         link_ordering: [LinkOrdering, None] = None) -> LinkArgsOutput:
     """
     Merges LinkArgs. Returns the args, files that must be present for those
     args to work when passed to a linker, and optionally an artifact where DWO
     outputs will be written to.
     """
-    suffix = "" if suffix == None else "-" + suffix
     args = cmd_args()
     hidden = []
 
@@ -136,44 +128,23 @@ def make_link_args(
         pdb_artifact = actions.declare_output(pdb_filename)
         hidden.append(pdb_artifact.as_output())
 
-    filelists = None
     if linker_type == LinkerType("darwin"):
-        filelists = filter(None, [unpack_link_args_filelist(link) for link in links])
-        hidden.extend(filelists)
-
-    for link in links:
-        if filelists:
-            # If we are using a filelist, only add argument that aren't already in the
-            # filelist. This is to avoid duplicate inputs in the link command.
-            args.add(
-                unpack_link_args_excluding_filelist(
-                    link,
-                    link_ordering = link_ordering,
-                    link_metadata_flag = linker_info.link_metadata_flag,
-                ),
-            )
-        else:
-            args.add(
-                unpack_link_args(
-                    link,
-                    link_ordering = link_ordering,
-                    link_metadata_flag = linker_info.link_metadata_flag,
-                ),
-            )
-
-    # On Darwin, filelist args _must_ come last as the order can affect symbol
-    # resolution and result in binary size increases.
-    filelist_file = None
-    if filelists:
-        path = actions.write("filelist%s.txt" % suffix, filelists)
-        args.add(cmd_args(["-Xlinker", "-filelist", "-Xlinker", path]))
-        filelist_file = path
+        # We order inputs partially for binary size, and partially for historical
+        # reasons. It is important that lazy object file (non link-whole archives)
+        # be listed last on the command line so they are not unnecessarily loaded.
+        # Some applications implicitly rely upon this ordering, so it is difficult
+        # to change.  Note link_ordering configuration is not respected. The
+        # link_metadata_flag is also ignored as that is a flag to the GNU linker wrapper,
+        # which does not apply to Darwin.
+        args.add(filter(None, [unpack_link_args_excluding_object_files_and_lazy_archives(link) for link in links]))
+        args.add(filter(None, [unpack_link_args_object_files_and_lazy_archives_only(link) for link in links]))
+    else:
+        args.add(filter(None, [unpack_link_args(link, link_ordering = link_ordering, link_metadata_flag = linker_info.link_metadata_flag) for link in links]))
 
     return LinkArgsOutput(
         link_args = args,
         hidden = [args] + hidden,
         pdb_artifact = pdb_artifact,
-        filelist = filelist_file,
     )
 
 def shared_libs_symlink_tree_name(output: Artifact) -> str:

prelude/cxx/dist_lto/darwin_dist_lto.bzl

@@ -699,7 +699,6 @@ def cxx_darwin_dist_link(
         dwp = None,
         external_debug_info = external_debug_info,
         linker_argsfile = linker_argsfile_out,
-        linker_filelist = None,  # DistLTO doesn't use filelists
         linker_command = None,  # There is no notion of a single linker command for DistLTO
         index_argsfile = index_argsfile_out,
         dist_thin_lto_codegen_argsfile = opt_flags_for_debugging_argsfile,

prelude/cxx/dist_lto/dist_lto.bzl

@@ -679,7 +679,6 @@ def cxx_gnu_dist_link(
         dwp = dwp_output,
         external_debug_info = external_debug_info,
         linker_argsfile = linker_argsfile_out,
-        linker_filelist = None,  # DistLTO doesn't use filelists
         linker_command = None,  # There is no notion of a single linker command for DistLTO
         index_argsfile = index_argsfile_out,
         dist_thin_lto_codegen_argsfile = None,  # Only Darwin builds provide is argsfile

prelude/cxx/link.bzl

@@ -212,20 +212,11 @@ def cxx_link_into(
         split_debug_output = split_debug_lto_info.output
     expect(not generates_split_debug(cxx_toolchain_info) or split_debug_output != None)
 
-    link_args_suffix = None
-    if opts.identifier:
-        link_args_suffix = opts.identifier
-    if opts.category_suffix:
-        if link_args_suffix:
-            link_args_suffix += "-" + opts.category_suffix
-        else:
-            link_args_suffix = opts.category_suffix
     link_args_output = make_link_args(
         ctx,
         ctx.actions,
         cxx_toolchain_info,
         links_with_linker_map,
-        suffix = link_args_suffix,
         output_short_path = output.short_path,
         link_ordering = value_or(
             opts.link_ordering,
@@ -370,7 +361,6 @@ def cxx_link_into(
         dwp = dwp_artifact,
         external_debug_info = external_debug_info,
         linker_argsfile = argfile,
-        linker_filelist = link_args_output.filelist,
         linker_command = command,
         import_library = opts.import_library,
         pdb = link_args_output.pdb_artifact,

prelude/linking/link_info.bzl

@@ -241,24 +241,8 @@ def wrap_link_info(
         metadata = inner.metadata,
     )
 
-# Returns true if the command line argument representation of this linkable,
-# could be passed within a filelist.
-def _is_linkable_included_in_filelist(linkable: LinkableTypes) -> bool:
-    if isinstance(linkable, ArchiveLinkable):
-        # Link whole archives don't appear in the filelist, but are passed directly to the linker
-        # with a -force-load (MachO) or -whole-archive (ELF) flag. Regular archives do appear in the filelist.
-        return not linkable.link_whole
-    elif isinstance(linkable, SharedLibLinkable) or \
-         isinstance(linkable, FrameworksLinkable) or \
-         isinstance(linkable, SwiftRuntimeLinkable) or \
-         isinstance(linkable, SwiftmoduleLinkable):
-        # These are all passed directly via various command line flags, not via a filelist.
-        return False
-    elif isinstance(linkable, ObjectsLinkable):
-        # Object files always appear in the filelist.
-        return True
-    else:
-        fail("Encountered unhandled filelist-like linkable {}".format(str(linkable)))
+def _is_linkable_comprised_of_object_files_or_a_lazy_archive(linkable: LinkableTypes) -> bool:
+    return isinstance(linkable, ObjectsLinkable) or (isinstance(linkable, ArchiveLinkable) and not linkable.link_whole)
 
 # Adds appropriate args representing `linkable` to `args`
 def append_linkable_args(args: cmd_args, linkable: LinkableTypes):
@@ -300,23 +284,25 @@ def append_linkable_args(args: cmd_args, linkable: LinkableTypes):
 
 LinkInfoArgumentFilter = enum(
     "all",
-    "filelist_only",
-    "excluding_filelist",
+    "object_files_and_lazy_archives_only",
+    "exclude_object_files_and_lazy_archives",
 )
 
 def link_info_to_args(value: LinkInfo, argument_type_filter: LinkInfoArgumentFilter = LinkInfoArgumentFilter("all")) -> cmd_args:
     result = cmd_args()
 
-    do_pre_post_flags = argument_type_filter == LinkInfoArgumentFilter("all") or argument_type_filter == LinkInfoArgumentFilter("excluding_filelist")
+    do_pre_post_flags = argument_type_filter == LinkInfoArgumentFilter("all") or argument_type_filter == LinkInfoArgumentFilter("exclude_object_files_and_lazy_archives")
     if do_pre_post_flags:
         result.add(value.pre_flags)
 
     for linkable in value.linkables:
-        if (argument_type_filter == LinkInfoArgumentFilter("all")) or (
-            argument_type_filter == LinkInfoArgumentFilter("filelist_only") and _is_linkable_included_in_filelist(linkable)
-        ) or (
-            argument_type_filter == LinkInfoArgumentFilter("excluding_filelist") and not _is_linkable_included_in_filelist(linkable)
-        ):
+        if argument_type_filter == LinkInfoArgumentFilter("all"):
+            append_linkable_args(result, linkable)
+
+        elif argument_type_filter == LinkInfoArgumentFilter("object_files_and_lazy_archives_only") and _is_linkable_comprised_of_object_files_or_a_lazy_archive(linkable):
+            append_linkable_args(result, linkable)
+
+        elif argument_type_filter == LinkInfoArgumentFilter("exclude_object_files_and_lazy_archives") and not _is_linkable_comprised_of_object_files_or_a_lazy_archive(linkable):
             append_linkable_args(result, linkable)
 
     if do_pre_post_flags:
@@ -346,21 +332,11 @@ def _link_info_stripped_link_args(infos: LinkInfos):
     info = infos.stripped or infos.default
     return link_info_to_args(info, argument_type_filter = LinkInfoArgumentFilter("all"))
 
-def _link_info_default_filelist(infos: LinkInfos):
-    info = infos.default
-    return link_info_to_args(info, argument_type_filter = LinkInfoArgumentFilter("filelist_only"))
+def _link_info_object_files_and_lazy_archives_only_args(infos: LinkInfos):
+    return link_info_to_args(infos.default, argument_type_filter = LinkInfoArgumentFilter("object_files_and_lazy_archives_only"))
 
-def _link_info_stripped_filelist(infos: LinkInfos):
-    info = infos.stripped or infos.default
-    return link_info_to_args(info, argument_type_filter = LinkInfoArgumentFilter("filelist_only"))
-
-def _link_info_default_excluding_filelist_args(infos: LinkInfos):
-    info = infos.default
-    return link_info_to_args(info, argument_type_filter = LinkInfoArgumentFilter("excluding_filelist"))
-
-def _link_info_stripped_excluding_filelist_args(infos: LinkInfos):
-    info = infos.stripped or infos.default
-    return link_info_to_args(info, argument_type_filter = LinkInfoArgumentFilter("excluding_filelist"))
+def _link_info_excluding_object_files_and_lazy_archives_args(infos: LinkInfos):
+    return link_info_to_args(infos.default, argument_type_filter = LinkInfoArgumentFilter("exclude_object_files_and_lazy_archives"))
 
 def link_info_to_metadata_args(info: LinkInfo, args: cmd_args | None = None) -> ArgLike:
     if args == None:
@@ -373,34 +349,14 @@ def _link_info_metadata_args(infos: LinkInfos):
     info = infos.stripped or infos.default
     return link_info_to_metadata_args(info)
 
-def _link_info_has_default_filelist(children: list[bool], infos: [LinkInfos, None]) -> bool:
-    if infos:
-        info = infos.default
-        if len(link_info_to_args(info, argument_type_filter = LinkInfoArgumentFilter("filelist_only")).inputs):
-            return True
-    return any(children)
-
-def _link_info_has_stripped_filelist(children: list[bool], infos: [LinkInfos, None]) -> bool:
-    if infos:
-        info = infos.stripped or infos.default
-        if len(link_info_to_args(info, argument_type_filter = LinkInfoArgumentFilter("filelist_only")).inputs):
-            return True
-    return any(children)
-
 # TransitiveSet of LinkInfos.
 LinkInfosTSet = transitive_set(
     args_projections = {
         "default": _link_info_default_args,
-        "default_excluding_filelist": _link_info_default_excluding_filelist_args,
-        "default_filelist": _link_info_default_filelist,
+        "exclude_object_files_and_lazy_archives": _link_info_excluding_object_files_and_lazy_archives_args,
         "metadata": _link_info_metadata_args,
+        "object_files_and_lazy_archives_only": _link_info_object_files_and_lazy_archives_only_args,
         "stripped": _link_info_stripped_link_args,
-        "stripped_excluding_filelist": _link_info_stripped_excluding_filelist_args,
-        "stripped_filelist": _link_info_stripped_filelist,
-    },
-    reductions = {
-        "has_default_filelist": _link_info_has_default_filelist,
-        "has_stripped_filelist": _link_info_has_stripped_filelist,
     },
 )
 
@@ -447,11 +403,6 @@ LinkedObject = record(
     # This argsfile is generated in the `cxx_link` step and contains a list of arguments
     # passed to the linker. It is being exposed as a sub-target for debugging purposes.
     linker_argsfile = field(Artifact | None, None),
-    # The filelist is generated in the `cxx_link` step and contains a list of
-    # object files (static libs or plain object files) passed to the linker.
-    # It is being exposed for debugging purposes. Only present when a Darwin
-    # linker is used.
-    linker_filelist = field(Artifact | None, None),
     # The linker command as generated by `cxx_link`. Exposed for debugging purposes only.
     # Not present for DistLTO scenarios.
     linker_command = field([cmd_args, None], None),
@@ -732,51 +683,39 @@ def unpack_link_args(
 
     fail("Unpacked invalid empty link args")
 
-def unpack_link_args_filelist(args: LinkArgs) -> [ArgLike, None]:
+def unpack_link_args_excluding_object_files_and_lazy_archives(args: LinkArgs) -> [ArgLike, None]:
     if args.tset != None:
-        tset = args.tset.infos
-        stripped = args.tset.prefer_stripped
-        if not tset.reduce("has_stripped_filelist" if stripped else "has_default_filelist"):
-            return None
-        return tset.project_as_args("stripped_filelist" if stripped else "default_filelist")
+        if args.tset.prefer_stripped:
+            fail("Preferring stripped link infos is not supported by this function.")
+
+        return args.tset.infos.project_as_args("exclude_object_files_and_lazy_archives")
 
     if args.infos != None:
         result_args = cmd_args()
         for info in args.infos:
-            result_args.add(link_info_to_args(info, argument_type_filter = LinkInfoArgumentFilter("filelist_only")))
-
-        if not len(result_args.inputs):
-            return None
-
+            result_args.add(link_info_to_args(info, argument_type_filter = LinkInfoArgumentFilter("exclude_object_files_and_lazy_archives")))
         return result_args
 
     if args.flags != None:
-        return None
+        return args.flags
 
     fail("Unpacked invalid empty link args")
 
-def unpack_link_args_excluding_filelist(
-        args: LinkArgs,
-        link_ordering: [LinkOrdering, None] = None,
-        link_metadata_flag: str | None = None) -> ArgLike:
-    cmd = link_args_metadata_with_flag(args, link_metadata_flag)
+def unpack_link_args_object_files_and_lazy_archives_only(args: LinkArgs) -> [ArgLike, None]:
     if args.tset != None:
-        ordering = link_ordering.value if link_ordering else "preorder"
-
-        tset = args.tset.infos
         if args.tset.prefer_stripped:
-            cmd.add(tset.project_as_args("stripped_excluding_filelist", ordering = ordering))
-        else:
-            cmd.add(tset.project_as_args("default_excluding_filelist", ordering = ordering))
-        return cmd
+            fail("Preferring stripped link infos is not supported by this function.")
+
+        return args.tset.infos.project_as_args("object_files_and_lazy_archives_only")
 
     if args.infos != None:
-        cmd.add([link_info_to_args(info, LinkInfoArgumentFilter("excluding_filelist")) for info in args.infos])
-        return cmd
+        result_args = cmd_args()
+        for info in args.infos:
+            result_args.add(link_info_to_args(info, argument_type_filter = LinkInfoArgumentFilter("object_files_and_lazy_archives_only")))
+        return result_args
 
     if args.flags != None:
-        cmd.add(args.flags)
-        return cmd
+        return None
 
     fail("Unpacked invalid empty link args")
 
@@ -1015,7 +954,6 @@ LinkCommandDebugOutput = record(
     filename = str,
     command = ArgLike,
     argsfile = Artifact,
-    filelist = Artifact | None,
     dist_thin_lto_codegen_argsfile = Artifact | None,
     dist_thin_lto_index_argsfile = Artifact | None,
 )
@@ -1040,7 +978,6 @@ def make_link_command_debug_output(linked_object: LinkedObject) -> [LinkCommandD
         filename = linked_object.output.short_path,
         command = linked_object.linker_command,
         argsfile = linked_object.linker_argsfile,
-        filelist = linked_object.linker_filelist,
         dist_thin_lto_index_argsfile = linked_object.dist_thin_lto_index_argsfile,
         dist_thin_lto_codegen_argsfile = linked_object.dist_thin_lto_codegen_argsfile,
     )
@@ -1051,7 +988,6 @@ def make_link_command_debug_output(linked_object: LinkedObject) -> [LinkCommandD
 #
 # For local thin-LTO:
 # - linker argfile
-# - linker filelist (if present - only applicable to Darwin linkers)
 #
 # For distributed thin-LTO:
 # - thin-link argsfile (without inputs just flags)
@@ -1076,8 +1012,8 @@ def make_link_command_debug_output_json_info(ctx: AnalysisContext, debug_outputs
                 "filename": debug_output.filename,
             })
 
-            # Ensure all argsfile and filelists get materialized, as those are needed for debugging
-            associated_artifacts.extend(filter(None, [debug_output.argsfile, debug_output.filelist]))
+            # Ensure all argsfile get materialized, as those are needed for debugging
+            associated_artifacts.extend(filter(None, [debug_output.argsfile]))
 
     # Explicitly drop all inputs by using `with_inputs = False`, we don't want
     # to materialize all inputs to the link actions (which includes all object files