From 393633eed04bc485064cc39d06d5dd6ad016fa4b Mon Sep 17 00:00:00 2001 From: Brentley Jones Date: Mon, 28 Oct 2024 13:12:25 -0500 Subject: [PATCH 1/2] Represent structured resources as files instead of folders in Xcode (#3098) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current way matches how Xcode bundles structured resources, and we did it this way for BwX mode. But representing these resources as folders has a few downsides: 1. More files can appear in Xcode than are actually referenced as inputs, because Bazel works on the level of files, not directories 2. The work to translate the files to folder paths isn’t free 3. The translation of files to folder paths broke with rules_apple’s recent runfiles support In a future change I’ll remove all of the code around `.isFolder`, since folder-type files can go down the `file_path` path. Signed-off-by: Brentley Jones --- .../internal/files/incremental_resources.bzl | 43 +------------------ 1 file changed, 2 insertions(+), 41 deletions(-) diff --git a/xcodeproj/internal/files/incremental_resources.bzl b/xcodeproj/internal/files/incremental_resources.bzl index a075455216..f8b1bc6bfb 100644 --- a/xcodeproj/internal/files/incremental_resources.bzl +++ b/xcodeproj/internal/files/incremental_resources.bzl @@ -5,7 +5,6 @@ load("@build_bazel_rules_apple//apple:resources.bzl", "resources_common") load("//xcodeproj/internal:configuration.bzl", "calculate_configuration") load("//xcodeproj/internal:memory_efficiency.bzl", "memory_efficient_depset") load("//xcodeproj/internal:target_id.bzl", "get_id") -load(":files.bzl", "join_paths_ignoring_empty") # Utility @@ -207,51 +206,18 @@ def _add_structured_resources_to_bundle( bundle, *, files, - focused_resource_short_paths, - nested_path): - if nested_path: - inner_dir = nested_path.split("/")[0] - else: - inner_dir = None - + focused_resource_short_paths): for file in files.to_list(): if file.short_path not in focused_resource_short_paths: continue - if not inner_dir: - bundle.resources.append(file) - continue - - # Special case for localized - if inner_dir.endswith(".lproj"): - bundle.resources.append(file) - continue - - if file.is_directory: - dir = file.path - else: - dir = file.dirname - - if not dir.endswith(nested_path): - continue - - folder_path = paths.join(dir[:-(1 + len(nested_path))], inner_dir) - if file.is_source: - bundle.folder_resources.append(folder_path) - else: - bundle.generated_folder_resources.append( - struct( - owner = file.owner, - path = folder_path, - ), - ) + bundle.resources.append(file) def _add_structured_resources( *, bundle_path, files, focused_resource_short_paths, - nested_path, resource_bundle_targets, root_bundle): bundle = resource_bundle_targets.get(bundle_path) @@ -268,14 +234,12 @@ def _add_structured_resources( bundle, files = files, focused_resource_short_paths = focused_resource_short_paths, - nested_path = nested_path, ) else: _add_structured_resources_to_bundle( root_bundle, files = files, focused_resource_short_paths = focused_resource_short_paths, - nested_path = join_paths_ignoring_empty(bundle_path, nested_path), ) def _handle_processable_resources( @@ -386,18 +350,15 @@ def _handle_unprocessed_resources( continue bundle_path = None - nested_path = parent_dir for parent_bundle_path in parent_bundle_paths: if parent_dir.startswith(parent_bundle_path): bundle_path = parent_bundle_path - nested_path = parent_dir[len(bundle_path) + 1:] break _add_structured_resources( bundle_path = bundle_path, files = files, focused_resource_short_paths = focused_resource_short_paths, - nested_path = nested_path, resource_bundle_targets = resource_bundle_targets, root_bundle = root_bundle, ) From 9ed62ea30bfd9733b6613273ca8eec2d2b845857 Mon Sep 17 00:00:00 2001 From: Brentley Jones Date: Mon, 28 Oct 2024 13:23:06 -0500 Subject: [PATCH 2/2] Properly handle generated processed resources (#3099) In f20bd24fda09f479adb6b29a6d1f7257774d782c I forgot to account for generated resource files. Signed-off-by: Brentley Jones --- .../write_files_and_groups_tests.bzl | 21 ++++++++++ .../ReadGeneratedFilePathsFile.swift | 9 +++- .../files/incremental_input_files.bzl | 6 +++ .../internal/files/incremental_resources.bzl | 41 +++++++++++++------ .../internal/incremental_xcode_targets.bzl | 10 +++-- xcodeproj/internal/pbxproj_partials.bzl | 25 +++++++---- .../internal/xcodeproj_incremental_rule.bzl | 8 ++++ 7 files changed, 92 insertions(+), 28 deletions(-) diff --git a/test/internal/pbxproj_partials/write_files_and_groups_tests.bzl b/test/internal/pbxproj_partials/write_files_and_groups_tests.bzl index 64e465750a..b6d8770c2d 100644 --- a/test/internal/pbxproj_partials/write_files_and_groups_tests.bzl +++ b/test/internal/pbxproj_partials/write_files_and_groups_tests.bzl @@ -72,6 +72,13 @@ def _write_files_and_groups_test_impl(ctx): ) for (path, owner) in ctx.attr.generated_files.items() ] + generated_file_paths = [ + struct( + path = path, + owner = mock_actions.mock_label(owner), + ) + for (path, owner) in ctx.attr.generated_file_paths.items() + ] generated_folders = [ struct( path = path, @@ -97,6 +104,7 @@ def _write_files_and_groups_test_impl(ctx): files = depset(files), file_paths = depset(ctx.attr.file_paths), folders = depset(ctx.attr.folders), + generated_file_paths = depset(generated_file_paths), generated_folders = depset(generated_folders), generator_name = "a_generator_name", install_path = ctx.attr.install_path, @@ -197,6 +205,7 @@ write_files_and_groups_test = unittest.make( "file_paths": attr.string_list(mandatory = True), "files": attr.string_list(mandatory = True), "folders": attr.string_list(mandatory = True), + "generated_file_paths": attr.string_dict(mandatory = True), "generated_files": attr.string_dict(mandatory = True), "generated_folders": attr.string_dict(mandatory = True), "install_path": attr.string(mandatory = True), @@ -231,6 +240,7 @@ def write_files_and_groups_test_suite(name): files = [], file_paths = [], folders = [], + generated_file_paths = {}, generated_files = {}, generated_folders = {}, install_path, @@ -253,6 +263,7 @@ def write_files_and_groups_test_suite(name): files = files, file_paths = file_paths, folders = folders, + generated_file_paths = generated_file_paths, generated_files = generated_files, generated_folders = generated_folders, install_path = install_path, @@ -358,6 +369,10 @@ def write_files_and_groups_test_suite(name): "a/path/to/a/folder", "another/path/to/another/folder", ], + generated_file_paths = { + "bazel-out/ios-sim-config/bin/a/path/to/a/generated/file_as_file_path": "//a/path/to/a/generated", + "bazel-out/ios-sim-config/bin/another/path/to/another/generated/file_as_file_path": "//another/path/to/another/generated", + }, generated_files = { "bazel-out/ios-sim-config/bin/a/path/to/a/generated/file": "//a/path/to/a/generated", "bazel-out/ios-sim-config/bin/another/path/to/another/generated/file": "//another/path/to/another/generated", @@ -437,6 +452,12 @@ ios-sim-config file another/path/to/another/generated ios-sim-config +file_as_file_path +a/path/to/a/generated +ios-sim-config +file_as_file_path +another/path/to/another/generated +ios-sim-config """, _GENERATED_FOLDER_PATHS_FILE: """\ folder diff --git a/tools/generators/files_and_groups/src/Generator/ReadGeneratedFilePathsFile.swift b/tools/generators/files_and_groups/src/Generator/ReadGeneratedFilePathsFile.swift index 267b092475..59d3ae08ac 100644 --- a/tools/generators/files_and_groups/src/Generator/ReadGeneratedFilePathsFile.swift +++ b/tools/generators/files_and_groups/src/Generator/ReadGeneratedFilePathsFile.swift @@ -2,7 +2,7 @@ import Foundation import PBXProj import ToolCommon -struct GeneratedPath { +struct GeneratedPath: Hashable { let config: String let package: BazelPath let path: BazelPath @@ -65,6 +65,11 @@ extension Generator.ReadGeneratedFilePathsFile { )) } - return generatedPaths + + // The file can have at most 1 duplicate for each entry because of + // preprocessed resource files being represented as file paths, while + // they can also be an input to another action (e.g. codegen). Because + // of this we use a `Set` to deduplicate the paths. + return Array(Set(generatedPaths)) } } diff --git a/xcodeproj/internal/files/incremental_input_files.bzl b/xcodeproj/internal/files/incremental_input_files.bzl index c716dc7606..2587df96a8 100644 --- a/xcodeproj/internal/files/incremental_input_files.bzl +++ b/xcodeproj/internal/files/incremental_input_files.bzl @@ -490,6 +490,9 @@ def _collect_incremental_input_files( if label not in bundle_labels ], ) + extra_generated_file_paths = memory_efficient_depset( + resources_result.generated_folder_resources, + ) extra_generated_folders = memory_efficient_depset( resources_result.generated_folder_resources, transitive = [ @@ -505,6 +508,7 @@ def _collect_incremental_input_files( ) else: extra_folders = EMPTY_DEPSET + extra_generated_file_paths = EMPTY_DEPSET extra_generated_folders = EMPTY_DEPSET resource_bundle_labels = memory_efficient_depset( transitive = [ @@ -552,6 +556,7 @@ def _collect_incremental_input_files( transitive = transitive_extra_files, ), extra_folders = extra_folders, + extra_generated_file_paths = extra_generated_file_paths, extra_generated_folders = extra_generated_folders, infoplist = infoplist, non_arc_srcs = memory_efficient_depset(non_arc_srcs), @@ -660,6 +665,7 @@ def _collect_mixed_language_input_files( extra_file_paths = mergeable_info.extra_file_paths, extra_files = mergeable_info.extra_files, extra_folders = EMPTY_DEPSET, + extra_generated_file_paths = EMPTY_DEPSET, extra_generated_folders = EMPTY_DEPSET, infoplist = None, non_arc_srcs = mergeable_info.non_arc_srcs, diff --git a/xcodeproj/internal/files/incremental_resources.bzl b/xcodeproj/internal/files/incremental_resources.bzl index f8b1bc6bfb..c8799d305d 100644 --- a/xcodeproj/internal/files/incremental_resources.bzl +++ b/xcodeproj/internal/files/incremental_resources.bzl @@ -38,6 +38,7 @@ def _create_bundle(name = None): name = name, resources = [], resource_file_paths = [], + generated_resource_file_paths = [], folder_resources = [], generated_folder_resources = [], dependency_paths = [], @@ -72,19 +73,14 @@ def _handle_processed_resource( processed_origins, ) - file_paths = [] owner = file.owner for short_path in origin_short_paths: - file_path = _handle_processed_resource_origin( + _handle_processed_resource_origin( bundle = bundle, short_path = short_path, focused_resource_short_paths = focused_resource_short_paths, owner = owner, ) - if file_path: - file_paths.append(file_path) - - return file_paths def _handle_processed_resource_origin( *, @@ -93,18 +89,20 @@ def _handle_processed_resource_origin( focused_resource_short_paths, owner): if short_path not in focused_resource_short_paths: - return None + return if short_path.startswith("../"): file_path = "external" + short_path[2:] else: file_path = short_path + is_generated = file_path.startswith("bazel-out/") + # If a file is a child of a folder-type file, the parent folder-type file # should be added to the bundle instead of the child file folder_type_prefix = _path_folder_type_prefix(file_path) if folder_type_prefix: - if file_path.startswith("bazel-out/"): + if is_generated: bundle.generated_folder_resources.append( struct( owner = owner, @@ -113,9 +111,15 @@ def _handle_processed_resource_origin( ) else: bundle.folder_resources.append(folder_type_prefix) - return None - - return file_path + elif is_generated: + bundle.generated_resource_file_paths.append( + struct( + owner = owner, + path = file_path, + ), + ) + else: + bundle.resource_file_paths.append(file_path) def _handle_unprocessed_resource( *, @@ -172,7 +176,7 @@ def _add_processed_resources_to_bundle( focused_resource_short_paths, processed_origins): for file in files.to_list(): - file_paths = _handle_processed_resource( + _handle_processed_resource( bundle = bundle, bundle_metadata = bundle_metadata, bundle_path = bundle_path, @@ -180,7 +184,6 @@ def _add_processed_resources_to_bundle( focused_resource_short_paths = focused_resource_short_paths, processed_origins = processed_origins, ) - bundle.resource_file_paths.extend(file_paths) def _add_unprocessed_resources_to_bundle( *, @@ -400,6 +403,12 @@ def _collect_incremental_resources( * `resources`: A `list` of two element `tuple`s. The first element is the label of the target that owns the resource. The second element is a `File` for a resource. + * `resource_file_paths`: A `list` of two element `tuple`s. The first + element is the label of the target that owns the resource. The + second element is a file path string of a non-generated resource. + * `generated_resource_file_paths`: A `list` of two element `tuple`s. + The first element is the label of the target that owns the resource. + The second element is a file path string of a generated resource. * `xccurrentversions`: A `list` of `.xccurrentversion` `File`s. """ root_bundle = _create_bundle() @@ -483,6 +492,8 @@ def _collect_incremental_resources( for child_bundle_path in parent_bundle_paths: bundle = resource_bundle_targets[child_bundle_path] if (not bundle.resources and + not bundle.resource_file_paths and + not bundle.generated_resource_file_paths and not bundle.folder_resources and not bundle.generated_folder_resources and not bundle.dependency_paths): @@ -515,6 +526,9 @@ def _collect_incremental_resources( resource_file_paths = memory_efficient_depset( bundle.resource_file_paths, ), + generated_resource_file_paths = memory_efficient_depset( + bundle.generated_resource_file_paths, + ), folder_resources = memory_efficient_depset( bundle.folder_resources, ), @@ -528,6 +542,7 @@ def _collect_incremental_resources( bundles = frozen_bundles, resources = root_bundle.resources, resource_file_paths = root_bundle.resource_file_paths, + generated_resource_file_paths = root_bundle.generated_resource_file_paths, folder_resources = root_bundle.folder_resources, generated_folder_resources = root_bundle.generated_folder_resources, xccurrentversions = xccurrentversions, diff --git a/xcodeproj/internal/incremental_xcode_targets.bzl b/xcodeproj/internal/incremental_xcode_targets.bzl index e508333f07..e0b437c247 100644 --- a/xcodeproj/internal/incremental_xcode_targets.bzl +++ b/xcodeproj/internal/incremental_xcode_targets.bzl @@ -24,9 +24,10 @@ def _from_resource_bundle(bundle): compile_stub_needed = False, inputs = struct( entitlements = EMPTY_DEPSET, - extra_files = bundle.resources, extra_file_paths = bundle.resource_file_paths, + extra_files = bundle.resources, extra_folders = bundle.folder_resources, + extra_generated_file_paths = bundle.generated_resource_file_paths, extra_generated_folders = bundle.generated_folder_resources, infoplist = None, non_arc_srcs = EMPTY_DEPSET, @@ -172,16 +173,17 @@ def _make_incremental_xcode_target( def _merge_xcode_inputs(*, dest_inputs, mergeable_info): return struct( - extra_files = memory_efficient_depset( - transitive = [dest_inputs.extra_files, mergeable_info.extra_files], - ), extra_file_paths = memory_efficient_depset( transitive = [ dest_inputs.extra_file_paths, mergeable_info.extra_file_paths, ], ), + extra_files = memory_efficient_depset( + transitive = [dest_inputs.extra_files, mergeable_info.extra_files], + ), extra_folders = dest_inputs.extra_folders, + extra_generated_file_paths = dest_inputs.extra_generated_file_paths, extra_generated_folders = dest_inputs.extra_generated_folders, infoplist = dest_inputs.infoplist, non_arc_srcs = mergeable_info.non_arc_srcs, diff --git a/xcodeproj/internal/pbxproj_partials.bzl b/xcodeproj/internal/pbxproj_partials.bzl index 34f3b4639f..97e5072128 100644 --- a/xcodeproj/internal/pbxproj_partials.bzl +++ b/xcodeproj/internal/pbxproj_partials.bzl @@ -60,14 +60,14 @@ def _generated_dirname(file): return file.dirname -def _always_generated_file_path(file): +def _xcfilelist_always_generated_file_path(file): return "$(BAZEL_OUT){}".format(file.path[9:]) -def _generated_file_path(file): +def _xcfilelist_generated_file_path(file): if file.is_source: return None - return _always_generated_file_path(file) + return _xcfilelist_always_generated_file_path(file) def _source_file(file): if not file.is_source: @@ -108,10 +108,10 @@ def _generated_file(file): return _generated_file_or_folder(file.path, file.owner) -def _generated_folder(generated_folder): +def _generated_file_path(generated_file_path): return _generated_file_or_folder( - generated_folder.path, - generated_folder.owner, + generated_file_path.path, + generated_file_path.owner, ) # Partials @@ -405,6 +405,7 @@ def _write_files_and_groups( files, file_paths, folders, + generated_file_paths, generated_folders, generator_name, install_path, @@ -429,6 +430,8 @@ def _write_files_and_groups( file paths. folders: A `depset` of paths to non-generated folders to include in the project. + generated_file_paths: A `depset` of file paths to generated files to + include in the project. generated_folders: A `depset` of paths to generated folders to include in the project. generator_name: The name of the `xcodeproj` generator target. @@ -517,6 +520,10 @@ def _write_files_and_groups( generated_file_paths_args.set_param_file_format("multiline") generated_file_paths_args.add_all(files, map_each = _generated_file) + generated_file_paths_args.add_all( + generated_file_paths, + map_each = _generated_file_path, + ) actions.write(generated_file_paths_file, generated_file_paths_args) @@ -533,7 +540,7 @@ def _write_files_and_groups( generated_folder_paths_args.add_all( generated_folders, - map_each = _generated_folder, + map_each = _generated_file_path, ) actions.write(generated_folder_paths_file, generated_folder_paths_args) @@ -677,12 +684,12 @@ def _write_generated_xcfilelist( # Info.plists are tracked as build files by Xcode, so top-level targets # will fail the first time they are built if we don't track them - args.add_all(infoplists, map_each = _always_generated_file_path) + args.add_all(infoplists, map_each = _xcfilelist_always_generated_file_path) # Source files are tracked as build files by Xcode, so building targets that # directly use generated source files will fail the first time they are # built if we don't track them - args.add_all(srcs, map_each = _generated_file_path) + args.add_all(srcs, map_each = _xcfilelist_generated_file_path) xcfilelist = actions.declare_file( "{}-generated.xcfilelist".format(generator_name), diff --git a/xcodeproj/internal/xcodeproj_incremental_rule.bzl b/xcodeproj/internal/xcodeproj_incremental_rule.bzl index f2560b8b9b..fc761d0fec 100644 --- a/xcodeproj/internal/xcodeproj_incremental_rule.bzl +++ b/xcodeproj/internal/xcodeproj_incremental_rule.bzl @@ -135,12 +135,16 @@ def _collect_files( transitive_file_paths = [] transitive_files = [unsupported_extra_files] transitive_folders = [] + transitive_generated_file_paths = [] transitive_generated_folders = [] transitive_srcs = [] for xcode_target in all_targets: transitive_file_paths.append(xcode_target.inputs.extra_file_paths) transitive_files.append(xcode_target.inputs.extra_files) transitive_folders.append(xcode_target.inputs.extra_folders) + transitive_generated_file_paths.append( + xcode_target.inputs.extra_generated_file_paths, + ) transitive_generated_folders.append( xcode_target.inputs.extra_generated_folders, ) @@ -169,6 +173,7 @@ def _collect_files( transitive = transitive_files, ) folders = depset(transitive = transitive_folders) + generated_file_paths = depset(transitive = transitive_generated_file_paths) generated_folders = depset(transitive = transitive_generated_folders) return ( @@ -176,6 +181,7 @@ def _collect_files( file_paths, files, folders, + generated_file_paths, generated_folders, infoplists, srcs, @@ -376,6 +382,7 @@ def _write_project_contents( file_paths, files, folders, + generated_file_paths, generated_folders, infoplists, srcs, @@ -436,6 +443,7 @@ def _write_project_contents( files = files, file_paths = file_paths, folders = folders, + generated_file_paths = generated_file_paths, generated_folders = generated_folders, generator_name = name, install_path = install_path,