[ios] Simplify handling of assets catalog
If the source files are listed as part of an assets catalog (i.e. one
element in the path ends with `.xcassets`), then consider the file as
part of an assets catalog.
When generating the Xcode project, if a file in part of an assets
catalog, only list the catalog, not the individual files (Xcode will
discover the files).
Bug: none
Change-Id: Ifaa940e51080ec05bda751a2028c6df2534866ec
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/16900
Reviewed-by: David Turner <digit@google.com>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
diff --git a/build/gen.py b/build/gen.py
index 169427e..9168035 100755
--- a/build/gen.py
+++ b/build/gen.py
@@ -800,8 +800,9 @@
'src/gn/action_target_generator_unittest.cc',
'src/gn/analyzer_unittest.cc',
'src/gn/args_unittest.cc',
- 'src/gn/builder_unittest.cc',
'src/gn/builder_record_map_unittest.cc',
+ 'src/gn/builder_unittest.cc',
+ 'src/gn/bundle_data_unittest.cc',
'src/gn/c_include_iterator_unittest.cc',
'src/gn/command_format_unittest.cc',
'src/gn/commands_unittest.cc',
diff --git a/docs/reference.md b/docs/reference.md
index 9621765..e98a86a 100644
--- a/docs/reference.md
+++ b/docs/reference.md
@@ -1603,6 +1603,11 @@
generate iOS/macOS bundle. In cross-platform projects, it is advised to put it
behind iOS/macOS conditionals.
+ If any source files in a bundle_data target match `*/*.xcassets/*` then they
+ will be considered part of an assets catalog, and instead of being copied to
+ the final bundle the assets catalog itself will be added to the inputs of the
+ assets catalog compilation step. See "compile_xcassets" tool.
+
See "gn help create_bundle" for more information.
```
@@ -4175,6 +4180,10 @@
Expands to the list of flags specified in corresponding
create_bundle target.
+ The inputs for compile_xcassets tool will be found from the bundle_data
+ dependencies by looking for any file matching "*/*.xcassets/*" pattern.
+ The "$assets.xcassets" directory will be added as input to the tool.
+
The Swift tool has multiple input and outputs. It must have exactly one
output of .swiftmodule type, but can have one or more object file outputs,
in addition to other type of outputs. The following expansions are available:
diff --git a/src/gn/bundle_data.cc b/src/gn/bundle_data.cc
index 62791c3..fdc22fd 100644
--- a/src/gn/bundle_data.cc
+++ b/src/gn/bundle_data.cc
@@ -13,49 +13,28 @@
#include "gn/substitution_writer.h"
#include "gn/target.h"
-namespace {
-
-// Return directory of |path| without the trailing directory separator.
-std::string_view FindDirNoTrailingSeparator(std::string_view path) {
- std::string_view::size_type pos = path.find_last_of("/\\");
- if (pos == std::string_view::npos)
- return std::string_view();
- return std::string_view(path.data(), pos);
-}
-
-bool IsSourceFileFromAssetsCatalog(std::string_view source,
- SourceFile* asset_catalog) {
- // Check whether |source| matches one of the following pattern:
- // .*\.xcassets/Contents.json
- // .*\.xcassets/[^/]*\.appiconset/[^/]*
- // .*\.xcassets/[^/]*\.colorset/[^/]*
- // .*\.xcassets/[^/]*\.dataset/[^/]*
- // .*\.xcassets/[^/]*\.imageset/[^/]*
- // .*\.xcassets/[^/]*\.launchimage/[^/]*
- // .*\.xcassets/[^/]*\.symbolset/[^/]*
- bool is_file_from_asset_catalog = false;
- std::string_view dir = FindDirNoTrailingSeparator(source);
- if (source.ends_with("/Contents.json") && dir.ends_with(".xcassets")) {
- is_file_from_asset_catalog = true;
- } else if (dir.ends_with(".appiconset") || dir.ends_with(".colorset") ||
- dir.ends_with(".dataset") || dir.ends_with(".imageset") ||
- dir.ends_with(".launchimage") || dir.ends_with(".symbolset")) {
- dir = FindDirNoTrailingSeparator(dir);
- is_file_from_asset_catalog = dir.ends_with(".xcassets");
- }
- if (is_file_from_asset_catalog && asset_catalog) {
- std::string asset_catalog_path(dir);
- *asset_catalog = SourceFile(std::move(asset_catalog_path));
- }
- return is_file_from_asset_catalog;
-}
-
-} // namespace
-
BundleData::BundleData() = default;
BundleData::~BundleData() = default;
+SourceFile BundleData::GetAssetsCatalogDirectory(const SourceFile& source) {
+ SourceFile assets_catalog_dir;
+ std::string_view path = source.value();
+ while (!path.empty()) {
+ if (path.ends_with(".xcassets")) {
+ assets_catalog_dir = SourceFile(path);
+ }
+
+ const auto sep = path.find_last_of("/\\");
+ if (sep == std::string_view::npos) {
+ break;
+ }
+
+ path = path.substr(0, sep);
+ }
+ return assets_catalog_dir;
+}
+
void BundleData::AddBundleData(const Target* target, bool is_create_bundle) {
DCHECK_EQ(target->output_type(), Target::BUNDLE_DATA);
for (const auto& pattern : bundle_deps_filter_) {
@@ -89,9 +68,9 @@
for (const Target* target : bundle_deps_) {
SourceFiles file_rule_sources;
for (const SourceFile& source_file : target->sources()) {
- SourceFile assets_catalog;
- if (IsSourceFileFromAssetsCatalog(source_file.value(), &assets_catalog)) {
- assets_catalog_sources.push_back(assets_catalog);
+ SourceFile assets_catalog_dir = GetAssetsCatalogDirectory(source_file);
+ if (!assets_catalog_dir.is_null()) {
+ assets_catalog_sources.push_back(assets_catalog_dir);
assets_catalog_deps.push_back(target);
} else {
file_rule_sources.push_back(source_file);
diff --git a/src/gn/bundle_data.h b/src/gn/bundle_data.h
index f168264..bf0d460 100644
--- a/src/gn/bundle_data.h
+++ b/src/gn/bundle_data.h
@@ -32,6 +32,20 @@
BundleData();
~BundleData();
+ // Return the assets catalog directory path from `source` as a SourceFile,
+ // if present, or a null instance (in case assets catalog are nested, this
+ // returns the outer most path).
+ //
+ // Example:
+ // "//my/bundle/foo.xcassets/my/file"
+ // -> "//my/bundle/foo.xcassets"
+ // "//my/bundle/foo.xcassets/nested/bar.xcassets/my/file"
+ // -> "//my/bundle/foo.xcassets"
+ // "//my/bundle/my/file"
+ // -> ""
+ //
+ static SourceFile GetAssetsCatalogDirectory(const SourceFile& source);
+
// Adds a bundle_data target to the recursive collection of all bundle_data
// that the target depends on.
void AddBundleData(const Target* target, bool is_create_bundle);
diff --git a/src/gn/bundle_data_unittest.cc b/src/gn/bundle_data_unittest.cc
new file mode 100644
index 0000000..2c2ff40
--- /dev/null
+++ b/src/gn/bundle_data_unittest.cc
@@ -0,0 +1,34 @@
+// Copyright 2021 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+
+#include "gn/bundle_data.h"
+#include "util/test/test.h"
+
+TEST(BundleDataTest, GetAssetsCatalogDirectory) {
+ struct TestCase {
+ SourceFile source_file;
+ SourceFile catalog_dir;
+ };
+
+ static TestCase kTestCases[] = {
+ {
+ .source_file = SourceFile("//my/bundle/foo.xcassets/my/file"),
+ .catalog_dir = SourceFile("//my/bundle/foo.xcassets"),
+ },
+ {
+ .source_file = SourceFile(
+ "//my/bundle/foo.xcassets/nested/bar.xcassets/my/file"),
+ .catalog_dir = SourceFile("//my/bundle/foo.xcassets"),
+ },
+ {
+ .source_file = SourceFile("//my/bundle/my/file"),
+ .catalog_dir = SourceFile(),
+ },
+ };
+
+ for (const TestCase& test_case : kTestCases) {
+ const SourceFile assets_catalog_dir =
+ BundleData::GetAssetsCatalogDirectory(test_case.source_file);
+ EXPECT_EQ(assets_catalog_dir, test_case.catalog_dir);
+ }
+}
diff --git a/src/gn/function_toolchain.cc b/src/gn/function_toolchain.cc
index b36b1e9..c42950a 100644
--- a/src/gn/function_toolchain.cc
+++ b/src/gn/function_toolchain.cc
@@ -812,6 +812,10 @@
Expands to the list of flags specified in corresponding
create_bundle target.
+ The inputs for compile_xcassets tool will be found from the bundle_data
+ dependencies by looking for any file matching "*/*.xcassets/*" pattern.
+ The "$assets.xcassets" directory will be added as input to the tool.
+
The Swift tool has multiple input and outputs. It must have exactly one
output of .swiftmodule type, but can have one or more object file outputs,
in addition to other type of outputs. The following expansions are available:
diff --git a/src/gn/functions_target.cc b/src/gn/functions_target.cc
index 9dff1c9..5a13779 100644
--- a/src/gn/functions_target.cc
+++ b/src/gn/functions_target.cc
@@ -306,6 +306,11 @@
generate iOS/macOS bundle. In cross-platform projects, it is advised to put it
behind iOS/macOS conditionals.
+ If any source files in a bundle_data target match `*/*.xcassets/*` then they
+ will be considered part of an assets catalog, and instead of being copied to
+ the final bundle the assets catalog itself will be added to the inputs of the
+ assets catalog compilation step. See "compile_xcassets" tool.
+
See "gn help create_bundle" for more information.
Variables
diff --git a/src/gn/ninja_bundle_data_target_writer_unittest.cc b/src/gn/ninja_bundle_data_target_writer_unittest.cc
index 06b5eb2..6fa4fe1 100644
--- a/src/gn/ninja_bundle_data_target_writer_unittest.cc
+++ b/src/gn/ninja_bundle_data_target_writer_unittest.cc
@@ -31,6 +31,10 @@
SourceFile("//foo/Foo.xcassets/foo.imageset/FooIcon-29@2x.png"));
bundle_data.sources().push_back(
SourceFile("//foo/Foo.xcassets/foo.imageset/FooIcon-29@3x.png"));
+ bundle_data.sources().push_back(
+ SourceFile("//foo/Foo.xcassets/file/with/no/known/pattern"));
+ bundle_data.sources().push_back(
+ SourceFile("//foo/Foo.xcassets/nested/bar.xcassets/my/file"));
bundle_data.action_values().outputs() = SubstitutionList::MakeForTest(
"{{bundle_resources_dir}}/{{source_file_part}}");
bundle_data.SetToolchain(setup.toolchain());
@@ -50,7 +54,9 @@
"../../foo/Foo.xcassets/foo.imageset/Contents.json "
"../../foo/Foo.xcassets/foo.imageset/FooIcon-29.png "
"../../foo/Foo.xcassets/foo.imageset/FooIcon-29@2x.png "
- "../../foo/Foo.xcassets/foo.imageset/FooIcon-29@3x.png\n";
+ "../../foo/Foo.xcassets/foo.imageset/FooIcon-29@3x.png "
+ "../../foo/Foo.xcassets/file/with/no/known/pattern "
+ "../../foo/Foo.xcassets/nested/bar.xcassets/my/file\n";
std::string out_str = out.str();
EXPECT_EQ(expected, out_str);
}
diff --git a/src/gn/ninja_create_bundle_target_writer_unittest.cc b/src/gn/ninja_create_bundle_target_writer_unittest.cc
index d2c5c24..997c058 100644
--- a/src/gn/ninja_create_bundle_target_writer_unittest.cc
+++ b/src/gn/ninja_create_bundle_target_writer_unittest.cc
@@ -199,6 +199,10 @@
SourceFile("//foo/Foo.xcassets/foo.dataset/Contents.json"));
bundle_data.sources().push_back(
SourceFile("//foo/Foo.xcassets/foo.dataset/FooScript.js"));
+ bundle_data.sources().push_back(
+ SourceFile("//foo/Foo.xcassets/file/with/no/known/pattern"));
+ bundle_data.sources().push_back(
+ SourceFile("//foo/Foo.xcassets/nested/bar.xcassets/my/file"));
bundle_data.action_values().outputs() = SubstitutionList::MakeForTest(
"{{bundle_resources_dir}}/{{source_file_part}}");
bundle_data.SetToolchain(setup.toolchain());
@@ -313,6 +317,10 @@
SourceFile("//foo/Foo.xcassets/foo.dataset/Contents.json"));
bundle_data2.sources().push_back(
SourceFile("//foo/Foo.xcassets/foo.dataset/FooScript.js"));
+ bundle_data2.sources().push_back(
+ SourceFile("//foo/Foo.xcassets/file/with/no/known/pattern"));
+ bundle_data2.sources().push_back(
+ SourceFile("//foo/Foo.xcassets/nested/bar.xcassets/my/file"));
bundle_data2.action_values().outputs() = SubstitutionList::MakeForTest(
"{{bundle_resources_dir}}/{{source_file_part}}");
bundle_data2.SetToolchain(setup.toolchain());
diff --git a/src/gn/target_unittest.cc b/src/gn/target_unittest.cc
index db43d74..8af8f6f 100644
--- a/src/gn/target_unittest.cc
+++ b/src/gn/target_unittest.cc
@@ -1088,6 +1088,10 @@
SourceFile("//foo/Foo.xcassets/foo.imageset/FooEmpty-29@2x.png"));
f.sources().push_back(
SourceFile("//foo/Foo.xcassets/foo.imageset/FooEmpty-29@3x.png"));
+ f.sources().push_back(
+ SourceFile("//foo/Foo.xcassets/file/with/no/known/pattern"));
+ f.sources().push_back(
+ SourceFile("//foo/Foo.xcassets/nested/bar.xcassets/my/file"));
f.action_values().outputs() = SubstitutionList::MakeForTest(
"{{bundle_resources_dir}}/{{source_file_part}}");
ASSERT_TRUE(f.OnResolved(&err));
diff --git a/src/gn/xcode_writer.cc b/src/gn/xcode_writer.cc
index 83c7c16..ea84a3d 100644
--- a/src/gn/xcode_writer.cc
+++ b/src/gn/xcode_writer.cc
@@ -25,6 +25,7 @@
#include "gn/args.h"
#include "gn/build_settings.h"
#include "gn/builder.h"
+#include "gn/bundle_data.h"
#include "gn/commands.h"
#include "gn/deps_iterator.h"
#include "gn/filesystem_utils.h"
@@ -447,6 +448,63 @@
return attributes;
}
+// Helper class used to collect the source files that will be added to
+// and PBXProject.
+class WorkspaceSources {
+ public:
+ WorkspaceSources(const BuildSettings* build_settings);
+ ~WorkspaceSources();
+
+ // Records `source` as part of the project. The source may be dropped if
+ // it should not be listed in the project (e.g. a generated file). Also
+ // for files in an assets catalog, only the catalog itself will be added.
+ void AddSourceFile(const SourceFile& source);
+
+ // Insert all the recorded source into `project`.
+ void AddToProject(PBXProject& project) const;
+
+ private:
+ const SourceDir build_dir_;
+ const std::string root_dir_;
+ SourceFileSet source_files_;
+};
+
+WorkspaceSources::WorkspaceSources(const BuildSettings* build_settings)
+ : build_dir_(build_settings->build_dir()),
+ root_dir_(build_settings->root_path_utf8()) {}
+
+WorkspaceSources::~WorkspaceSources() = default;
+
+void WorkspaceSources::AddSourceFile(const SourceFile& source) {
+ if (IsStringInOutputDir(build_dir_, source.value())) {
+ return;
+ }
+
+ if (IsPathAbsolute(source.value())) {
+ return;
+ }
+
+ SourceFile assets_catalog_dir = BundleData::GetAssetsCatalogDirectory(source);
+ if (!assets_catalog_dir.is_null()) {
+ source_files_.insert(assets_catalog_dir);
+ } else {
+ source_files_.insert(source);
+ }
+}
+
+void WorkspaceSources::AddToProject(PBXProject& project) const {
+ // Sort the files to ensure a deterministic generation of the project file.
+ std::vector<SourceFile> sources(source_files_.begin(), source_files_.end());
+ std::sort(sources.begin(), sources.end());
+
+ const SourceDir source_dir("//");
+ for (const SourceFile& source : sources) {
+ const std::string source_path =
+ RebasePath(source.value(), source_dir, root_dir_);
+ project.AddSourceFileToIndexingTarget(source_path, source_path);
+ }
+}
+
} // namespace
// Class representing the workspace embedded in an xcodeproj file used to
@@ -634,34 +692,30 @@
}
bool XcodeProject::AddSourcesFromBuilder(const Builder& builder, Err* err) {
- SourceFileSet sources;
+ WorkspaceSources sources(build_settings_);
// Add sources from all targets.
for (const Target* target : builder.GetAllResolvedTargets()) {
for (const SourceFile& source : target->sources()) {
- if (ShouldIncludeFileInProject(source))
- sources.insert(source);
+ sources.AddSourceFile(source);
}
for (const SourceFile& source : target->config_values().inputs()) {
- if (ShouldIncludeFileInProject(source))
- sources.insert(source);
+ sources.AddSourceFile(source);
}
for (const SourceFile& source : target->public_headers()) {
- if (ShouldIncludeFileInProject(source))
- sources.insert(source);
+ sources.AddSourceFile(source);
}
const SourceFile& bridge_header = target->swift_values().bridge_header();
- if (!bridge_header.is_null() && ShouldIncludeFileInProject(bridge_header)) {
- sources.insert(bridge_header);
+ if (!bridge_header.is_null()) {
+ sources.AddSourceFile(bridge_header);
}
if (target->output_type() == Target::ACTION ||
target->output_type() == Target::ACTION_FOREACH) {
- if (ShouldIncludeFileInProject(target->action_values().script()))
- sources.insert(target->action_values().script());
+ sources.AddSourceFile(target->action_values().script());
}
}
@@ -671,13 +725,11 @@
continue;
const SourceFile build = builder.loader()->BuildFileForLabel(item->label());
- if (ShouldIncludeFileInProject(build))
- sources.insert(build);
+ sources.AddSourceFile(build);
for (const SourceFile& source :
item->settings()->import_manager().GetImportedFiles()) {
- if (ShouldIncludeFileInProject(source))
- sources.insert(source);
+ sources.AddSourceFile(source);
}
}
@@ -687,8 +739,7 @@
continue;
const SourceFile source = FilePathToSourceFile(build_settings_, path);
- if (ShouldIncludeFileInProject(source))
- sources.insert(source);
+ sources.AddSourceFile(source);
}
// Add any files from --xcode-additional-files-patterns, using the root
@@ -707,25 +758,13 @@
for (base::FilePath path = it.Next(); !path.empty(); path = it.Next()) {
const SourceFile source = FilePathToSourceFile(build_settings_, path);
- if (ShouldIncludeFileInProject(source))
- sources.insert(source);
+ sources.AddSourceFile(source);
}
}
}
}
- // Sort files to ensure deterministic generation of the project file (and
- // nicely sorted file list in Xcode).
- std::vector<SourceFile> sorted_sources(sources.begin(), sources.end());
- std::sort(sorted_sources.begin(), sorted_sources.end());
-
- const SourceDir source_dir("//");
- for (const SourceFile& source : sorted_sources) {
- const std::string source_file = RebasePath(
- source.value(), source_dir, build_settings_->root_path_utf8());
- project_.AddSourceFileToIndexingTarget(source_file, source_file);
- }
-
+ sources.AddToProject(project_);
return true;
}
