diff options
-rw-r--r-- | tools/aapt2/cmd/Link.cpp | 10 | ||||
-rw-r--r-- | tools/aapt2/cmd/Link_test.cpp | 77 | ||||
-rw-r--r-- | tools/aapt2/format/binary/TableFlattener.cpp | 1 | ||||
-rw-r--r-- | tools/aapt2/java/ClassDefinition.h | 29 | ||||
-rw-r--r-- | tools/aapt2/java/JavaClassGenerator.cpp | 63 | ||||
-rw-r--r-- | tools/aapt2/java/JavaClassGenerator.h | 2 | ||||
-rw-r--r-- | tools/aapt2/java/JavaClassGenerator_test.cpp | 2 | ||||
-rw-r--r-- | tools/aapt2/process/SymbolTable.cpp | 4 | ||||
-rw-r--r-- | tools/aapt2/test/Fixture.cpp | 85 | ||||
-rw-r--r-- | tools/aapt2/test/Fixture.h | 35 |
10 files changed, 257 insertions, 51 deletions
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp index b760958e0edc..13e090d9d843 100644 --- a/tools/aapt2/cmd/Link.cpp +++ b/tools/aapt2/cmd/Link.cpp @@ -2263,6 +2263,16 @@ int LinkCommand::Action(const std::vector<std::string>& args) { return 1; } + if (shared_lib_ && options_.private_symbols) { + // If a shared library styleable in a public R.java uses a private attribute, attempting to + // reference the private attribute within the styleable array will cause a link error because + // the private attribute will not be emitted in the public R.java. + context.GetDiagnostics()->Error(DiagMessage() + << "--shared-lib cannot currently be used in combination with" + << " --private-symbols"); + return 1; + } + if (options_.merge_only && !static_lib_) { context.GetDiagnostics()->Error( DiagMessage() << "the --merge-only flag can be only used when building a static library"); diff --git a/tools/aapt2/cmd/Link_test.cpp b/tools/aapt2/cmd/Link_test.cpp index 062dd8eac975..73072a963d09 100644 --- a/tools/aapt2/cmd/Link_test.cpp +++ b/tools/aapt2/cmd/Link_test.cpp @@ -14,13 +14,16 @@ * limitations under the License. */ -#include "AppInfo.h" #include "Link.h" +#include <android-base/file.h> + +#include "AppInfo.h" #include "LoadedApk.h" #include "test/Test.h" using testing::Eq; +using testing::HasSubstr; using testing::Ne; namespace aapt { @@ -317,4 +320,76 @@ TEST_F(LinkTest, AppInfoWithUsesSplit) { ASSERT_TRUE(Link(link_args, feature2_files_dir, &diag)); } +TEST_F(LinkTest, SharedLibraryAttributeRJava) { + StdErrDiagnostics diag; + const std::string lib_values = + R"(<resources> + <attr name="foo"/> + <public type="attr" name="foo" id="0x00010001"/> + <declare-styleable name="LibraryStyleable"> + <attr name="foo" /> + </declare-styleable> + </resources>)"; + + const std::string client_values = + R"(<resources> + <attr name="bar" /> + <declare-styleable name="ClientStyleable"> + <attr name="com.example.lib:foo" /> + <attr name="bar" /> + </declare-styleable> + </resources>)"; + + // Build a library with a public attribute + const std::string lib_res = GetTestPath("library-res"); + ASSERT_TRUE(CompileFile(GetTestPath("res/values/values.xml"), lib_values, lib_res, &diag)); + + const std::string lib_apk = GetTestPath("library.apk"); + const std::string lib_java = GetTestPath("library_java"); + // clang-format off + auto lib_manifest = ManifestBuilder(this) + .SetPackageName("com.example.lib") + .Build(); + + auto lib_link_args = LinkCommandBuilder(this) + .SetManifestFile(lib_manifest) + .AddFlag("--shared-lib") + .AddParameter("--java", lib_java) + .AddCompiledResDir(lib_res, &diag) + .Build(lib_apk); + // clang-format on + ASSERT_TRUE(Link(lib_link_args, &diag)); + + const std::string lib_r_java = lib_java + "/com/example/lib/R.java"; + std::string lib_r_contents; + ASSERT_TRUE(android::base::ReadFileToString(lib_r_java, &lib_r_contents)); + EXPECT_THAT(lib_r_contents, HasSubstr(" public static int foo=0x00010001;")); + EXPECT_THAT(lib_r_contents, HasSubstr(" com.example.lib.R.attr.foo")); + + // Build a client that uses the library attribute in a declare-styleable + const std::string client_res = GetTestPath("client-res"); + ASSERT_TRUE(CompileFile(GetTestPath("res/values/values.xml"), client_values, client_res, &diag)); + + const std::string client_apk = GetTestPath("client.apk"); + const std::string client_java = GetTestPath("client_java"); + // clang-format off + auto client_manifest = ManifestBuilder(this) + .SetPackageName("com.example.client") + .Build(); + + auto client_link_args = LinkCommandBuilder(this) + .SetManifestFile(client_manifest) + .AddParameter("--java", client_java) + .AddParameter("-I", lib_apk) + .AddCompiledResDir(client_res, &diag) + .Build(client_apk); + // clang-format on + ASSERT_TRUE(Link(client_link_args, &diag)); + + const std::string client_r_java = client_java + "/com/example/client/R.java"; + std::string client_r_contents; + ASSERT_TRUE(android::base::ReadFileToString(client_r_java, &client_r_contents)); + EXPECT_THAT(client_r_contents, HasSubstr(" com.example.lib.R.attr.foo, 0x7f010000")); +} + } // namespace aapt diff --git a/tools/aapt2/format/binary/TableFlattener.cpp b/tools/aapt2/format/binary/TableFlattener.cpp index eb0ade62d542..4b90b4f534cc 100644 --- a/tools/aapt2/format/binary/TableFlattener.cpp +++ b/tools/aapt2/format/binary/TableFlattener.cpp @@ -570,7 +570,6 @@ class PackageFlattener { ResourceEntry* entry = sorted_entries->at(entryIndex); // Populate the config masks for this entry. - if (entry->visibility.level == Visibility::Level::kPublic) { config_masks[entry->id.value()] |= util::HostToDevice32(ResTable_typeSpec::SPEC_PUBLIC); } diff --git a/tools/aapt2/java/ClassDefinition.h b/tools/aapt2/java/ClassDefinition.h index 1e4b6816075a..995495ac56a8 100644 --- a/tools/aapt2/java/ClassDefinition.h +++ b/tools/aapt2/java/ClassDefinition.h @@ -70,8 +70,8 @@ class PrimitiveMember : public ClassMember { return name_; } - void Print(bool final, text::Printer* printer, bool strip_api_annotations = false) - const override { + void Print(bool final, text::Printer* printer, + bool strip_api_annotations = false) const override { using std::to_string; ClassMember::Print(final, printer, strip_api_annotations); @@ -127,13 +127,13 @@ using IntMember = PrimitiveMember<uint32_t>; using ResourceMember = PrimitiveMember<ResourceId>; using StringMember = PrimitiveMember<std::string>; -template <typename T> +template <typename T, typename StringConverter> class PrimitiveArrayMember : public ClassMember { public: explicit PrimitiveArrayMember(const android::StringPiece& name) : name_(name.to_string()) {} void AddElement(const T& val) { - elements_.push_back(val); + elements_.emplace_back(val); } bool empty() const override { @@ -158,7 +158,7 @@ class PrimitiveArrayMember : public ClassMember { printer->Println(); } - printer->Print(to_string(*current)); + printer->Print(StringConverter::ToString(*current)); if (std::distance(current, end) > 1) { printer->Print(", "); } @@ -175,7 +175,24 @@ class PrimitiveArrayMember : public ClassMember { std::vector<T> elements_; }; -using ResourceArrayMember = PrimitiveArrayMember<ResourceId>; +struct FieldReference { + explicit FieldReference(std::string reference) : ref(std::move(reference)) { + } + std::string ref; +}; + +struct ResourceArrayMemberStringConverter { + static std::string ToString(const std::variant<ResourceId, FieldReference>& ref) { + if (auto id = std::get_if<ResourceId>(&ref)) { + return to_string(*id); + } else { + return std::get<FieldReference>(ref).ref; + } + } +}; + +using ResourceArrayMember = PrimitiveArrayMember<std::variant<ResourceId, FieldReference>, + ResourceArrayMemberStringConverter>; // Represents a method in a class. class MethodDefinition : public ClassMember { diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp index f0f839d968d5..59dd481607e9 100644 --- a/tools/aapt2/java/JavaClassGenerator.cpp +++ b/tools/aapt2/java/JavaClassGenerator.cpp @@ -224,7 +224,16 @@ static bool operator<(const StyleableAttr& lhs, const StyleableAttr& rhs) { return cmp_ids_dynamic_after_framework(lhs_id, rhs_id); } -void JavaClassGenerator::ProcessStyleable(const ResourceNameRef& name, const ResourceId& id, +static FieldReference GetRFieldReference(const ResourceName& name, + StringPiece fallback_package_name) { + const std::string package_name = + name.package.empty() ? fallback_package_name.to_string() : name.package; + const std::string entry = JavaClassGenerator::TransformToFieldName(name.entry); + return FieldReference( + StringPrintf("%s.R.%s.%s", package_name.c_str(), to_string(name.type).data(), entry.c_str())); +} + +bool JavaClassGenerator::ProcessStyleable(const ResourceNameRef& name, const ResourceId& id, const Styleable& styleable, const StringPiece& package_name_to_generate, ClassDefinition* out_class_def, @@ -340,14 +349,29 @@ void JavaClassGenerator::ProcessStyleable(const ResourceNameRef& name, const Res // Add the ResourceIds to the array member. for (size_t i = 0; i < attr_count; i++) { - const ResourceId id = sorted_attributes[i].attr_ref->id.value_or_default(ResourceId(0)); - array_def->AddElement(id); + const StyleableAttr& attr = sorted_attributes[i]; + std::string r_txt_contents; + if (attr.symbol && attr.symbol.value().is_dynamic) { + if (!attr.attr_ref->name) { + error_ = "unable to determine R.java field name of dynamic resource"; + return false; + } + + const FieldReference field_name = + GetRFieldReference(attr.attr_ref->name.value(), package_name_to_generate); + array_def->AddElement(field_name); + r_txt_contents = field_name.ref; + } else { + const ResourceId attr_id = attr.attr_ref->id.value_or_default(ResourceId(0)); + array_def->AddElement(attr_id); + r_txt_contents = to_string(attr_id); + } if (r_txt_printer != nullptr) { if (i != 0) { r_txt_printer->Print(","); } - r_txt_printer->Print(" ").Print(id.to_string()); + r_txt_printer->Print(" ").Print(r_txt_contents); } } @@ -419,19 +443,7 @@ void JavaClassGenerator::ProcessStyleable(const ResourceNameRef& name, const Res } } - // If there is a rewrite method to generate, add the statements that rewrite package IDs - // for this styleable. - if (out_rewrite_method != nullptr) { - out_rewrite_method->AppendStatement( - StringPrintf("for (int i = 0; i < styleable.%s.length; i++) {", array_field_name.data())); - out_rewrite_method->AppendStatement( - StringPrintf(" if ((styleable.%s[i] & 0xff000000) == 0) {", array_field_name.data())); - out_rewrite_method->AppendStatement( - StringPrintf(" styleable.%s[i] = (styleable.%s[i] & 0x00ffffff) | packageIdBits;", - array_field_name.data(), array_field_name.data())); - out_rewrite_method->AppendStatement(" }"); - out_rewrite_method->AppendStatement("}"); - } + return true; } void JavaClassGenerator::ProcessResource(const ResourceNameRef& name, const ResourceId& id, @@ -448,8 +460,7 @@ void JavaClassGenerator::ProcessResource(const ResourceNameRef& name, const Reso const std::string field_name = TransformToFieldName(name.entry); if (out_class_def != nullptr) { - std::unique_ptr<ResourceMember> resource_member = - util::make_unique<ResourceMember>(field_name, real_id); + auto resource_member = util::make_unique<ResourceMember>(field_name, real_id); // Build the comments and annotations for this entry. AnnotationProcessor* processor = resource_member->GetCommentBuilder(); @@ -551,12 +562,11 @@ bool JavaClassGenerator::ProcessType(const StringPiece& package_name_to_generate if (resource_name.type == ResourceType::kStyleable) { CHECK(!entry->values.empty()); - - const Styleable* styleable = - static_cast<const Styleable*>(entry->values.front()->value.get()); - - ProcessStyleable(resource_name, id, *styleable, package_name_to_generate, out_type_class_def, - out_rewrite_method_def, r_txt_printer); + const auto styleable = reinterpret_cast<const Styleable*>(entry->values.front()->value.get()); + if (!ProcessStyleable(resource_name, id, *styleable, package_name_to_generate, + out_type_class_def, out_rewrite_method_def, r_txt_printer)) { + return false; + } } else { ProcessResource(resource_name, id, *entry, out_type_class_def, out_rewrite_method_def, r_txt_printer); @@ -626,8 +636,7 @@ bool JavaClassGenerator::Generate(const StringPiece& package_name_to_generate, if (type->type == ResourceType::kAttr) { // Also include private attributes in this same class. - const ResourceTableType* priv_type = package->FindType(ResourceType::kAttrPrivate); - if (priv_type) { + if (const ResourceTableType* priv_type = package->FindType(ResourceType::kAttrPrivate)) { if (!ProcessType(package_name_to_generate, *package, *priv_type, class_def.get(), rewrite_method.get(), r_txt_printer.get())) { return false; diff --git a/tools/aapt2/java/JavaClassGenerator.h b/tools/aapt2/java/JavaClassGenerator.h index 853120b3cb98..d9d1b39805f9 100644 --- a/tools/aapt2/java/JavaClassGenerator.h +++ b/tools/aapt2/java/JavaClassGenerator.h @@ -105,7 +105,7 @@ class JavaClassGenerator { // Writes a styleable resource to the R.java file, optionally writing out a rewrite rule for // its package ID if `out_rewrite_method` is not nullptr. // `package_name_to_generate` is the package - void ProcessStyleable(const ResourceNameRef& name, const ResourceId& id, + bool ProcessStyleable(const ResourceNameRef& name, const ResourceId& id, const Styleable& styleable, const android::StringPiece& package_name_to_generate, ClassDefinition* out_class_def, MethodDefinition* out_rewrite_method, diff --git a/tools/aapt2/java/JavaClassGenerator_test.cpp b/tools/aapt2/java/JavaClassGenerator_test.cpp index 04e20101a0dd..ec5b4151b1a6 100644 --- a/tools/aapt2/java/JavaClassGenerator_test.cpp +++ b/tools/aapt2/java/JavaClassGenerator_test.cpp @@ -581,7 +581,7 @@ TEST(JavaClassGeneratorTest, SortsDynamicAttributesAfterFrameworkAttributes) { out.Flush(); EXPECT_THAT(output, HasSubstr("public static final int[] MyStyleable={")); - EXPECT_THAT(output, HasSubstr("0x01010000, 0x00010000")); + EXPECT_THAT(output, HasSubstr("0x01010000, lib.R.attr.dynamic_attr")); EXPECT_THAT(output, HasSubstr("public static final int MyStyleable_android_framework_attr=0;")); EXPECT_THAT(output, HasSubstr("public static final int MyStyleable_dynamic_attr=1;")); } diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp index daedc2a14767..98ee63d2e5c6 100644 --- a/tools/aapt2/process/SymbolTable.cpp +++ b/tools/aapt2/process/SymbolTable.cpp @@ -370,11 +370,11 @@ std::unique_ptr<SymbolTable::Symbol> AssetManagerSymbolSource::FindByName( } else { s = util::make_unique<SymbolTable::Symbol>(); s->id = res_id; - s->is_dynamic = IsPackageDynamic(ResourceId(res_id).package_id(), real_name.package); } if (s) { s->is_public = (type_spec_flags & android::ResTable_typeSpec::SPEC_PUBLIC) != 0; + s->is_dynamic = IsPackageDynamic(ResourceId(res_id).package_id(), real_name.package); return s; } return {}; @@ -417,11 +417,11 @@ std::unique_ptr<SymbolTable::Symbol> AssetManagerSymbolSource::FindById( } else { s = util::make_unique<SymbolTable::Symbol>(); s->id = id; - s->is_dynamic = IsPackageDynamic(ResourceId(id).package_id(), name.package); } if (s) { s->is_public = (*flags & android::ResTable_typeSpec::SPEC_PUBLIC) != 0; + s->is_dynamic = IsPackageDynamic(ResourceId(id).package_id(), name.package); return s; } return {}; diff --git a/tools/aapt2/test/Fixture.cpp b/tools/aapt2/test/Fixture.cpp index 5386802dbc8e..f94f0fe1144a 100644 --- a/tools/aapt2/test/Fixture.cpp +++ b/tools/aapt2/test/Fixture.cpp @@ -18,18 +18,17 @@ #include <dirent.h> -#include "android-base/errors.h" -#include "android-base/file.h" -#include "android-base/stringprintf.h" -#include "android-base/utf8.h" -#include "androidfw/StringPiece.h" -#include "gmock/gmock.h" -#include "gtest/gtest.h" +#include <android-base/errors.h> +#include <android-base/file.h> +#include <android-base/stringprintf.h> +#include <android-base/utf8.h> +#include <androidfw/StringPiece.h> +#include <gmock/gmock.h> +#include <gtest/gtest.h> #include "cmd/Compile.h" #include "cmd/Link.h" #include "io/FileStream.h" -#include "io/Util.h" #include "util/Files.h" using testing::Eq; @@ -170,4 +169,74 @@ void CommandTestFixture::AssertLoadXml(LoadedApk* apk, const io::IData* data, } } +ManifestBuilder::ManifestBuilder(CommandTestFixture* fixture) : fixture_(fixture) { +} + +ManifestBuilder& ManifestBuilder::SetPackageName(const std::string& package_name) { + package_name_ = package_name; + return *this; +} + +ManifestBuilder& ManifestBuilder::AddContents(const std::string& contents) { + contents_ += contents + "\n"; + return *this; +} + +std::string ManifestBuilder::Build(const std::string& file_path) { + const char* manifest_template = R"( + <manifest xmlns:android="http://schemas.android.com/apk/res/android" + package="%s"> + %s + </manifest>)"; + + fixture_->WriteFile(file_path, android::base::StringPrintf( + manifest_template, package_name_.c_str(), contents_.c_str())); + return file_path; +} + +std::string ManifestBuilder::Build() { + return Build(fixture_->GetTestPath("AndroidManifest.xml")); +} + +LinkCommandBuilder::LinkCommandBuilder(CommandTestFixture* fixture) : fixture_(fixture) { +} + +LinkCommandBuilder& LinkCommandBuilder::SetManifestFile(const std::string& file) { + manifest_supplied_ = true; + args_.emplace_back("--manifest"); + args_.emplace_back(file); + return *this; +} + +LinkCommandBuilder& LinkCommandBuilder::AddFlag(const std::string& flag) { + args_.emplace_back(flag); + return *this; +} + +LinkCommandBuilder& LinkCommandBuilder::AddCompiledResDir(const std::string& dir, + IDiagnostics* diag) { + if (auto files = file::FindFiles(dir, diag)) { + for (std::string& compile_file : files.value()) { + args_.emplace_back(file::BuildPath({dir, compile_file})); + } + } + return *this; +} + +LinkCommandBuilder& LinkCommandBuilder::AddParameter(const std::string& param, + const std::string& value) { + args_.emplace_back(param); + args_.emplace_back(value); + return *this; +} + +std::vector<std::string> LinkCommandBuilder::Build(const std::string& out_apk) { + if (!manifest_supplied_) { + SetManifestFile(ManifestBuilder(fixture_).Build()); + } + args_.emplace_back("-o"); + args_.emplace_back(out_apk); + return args_; +} + } // namespace aapt
\ No newline at end of file diff --git a/tools/aapt2/test/Fixture.h b/tools/aapt2/test/Fixture.h index 457d65e30b65..f8c4889aee3b 100644 --- a/tools/aapt2/test/Fixture.h +++ b/tools/aapt2/test/Fixture.h @@ -32,7 +32,7 @@ namespace aapt { class TestDirectoryFixture : public ::testing::Test { public: TestDirectoryFixture() = default; - virtual ~TestDirectoryFixture() = default; + ~TestDirectoryFixture() override = default; // Creates the test directory or clears its contents if it contains previously created files. void SetUp() override; @@ -41,14 +41,14 @@ class TestDirectoryFixture : public ::testing::Test { void TearDown() override; // Retrieve the test directory of the fixture. - const android::StringPiece GetTestDirectory() { + android::StringPiece GetTestDirectory() { return temp_dir_; } // Retrieves the absolute path of the specified relative path in the test directory. Directories // should be separated using forward slashes ('/'), and these slashes will be translated to // backslashes when running Windows tests. - const std::string GetTestPath(const android::StringPiece& path) { + std::string GetTestPath(const android::StringPiece& path) { std::string base = temp_dir_; for (android::StringPiece part : util::Split(path, '/')) { file::AppendPath(&base, part); @@ -68,7 +68,7 @@ class TestDirectoryFixture : public ::testing::Test { class CommandTestFixture : public TestDirectoryFixture { public: CommandTestFixture() = default; - virtual ~CommandTestFixture() = default; + ~CommandTestFixture() override = default; // Wries the contents of the file to the specified path. The file is compiled and the flattened // file is written to the out directory. @@ -99,6 +99,33 @@ class CommandTestFixture : public TestDirectoryFixture { DISALLOW_COPY_AND_ASSIGN(CommandTestFixture); }; +struct ManifestBuilder { + explicit ManifestBuilder(CommandTestFixture* fixture); + ManifestBuilder& AddContents(const std::string& contents); + ManifestBuilder& SetPackageName(const std::string& package_name); + std::string Build(const std::string& file_path); + std::string Build(); + + private: + CommandTestFixture* fixture_; + std::string package_name_ = CommandTestFixture::kDefaultPackageName; + std::string contents_; +}; + +struct LinkCommandBuilder { + explicit LinkCommandBuilder(CommandTestFixture* fixture); + LinkCommandBuilder& AddCompiledResDir(const std::string& dir, IDiagnostics* diag); + LinkCommandBuilder& AddFlag(const std::string& flag); + LinkCommandBuilder& AddParameter(const std::string& param, const std::string& value); + LinkCommandBuilder& SetManifestFile(const std::string& manifest_path); + std::vector<std::string> Build(const std::string& out_apk_path); + + private: + CommandTestFixture* fixture_; + std::vector<std::string> args_; + bool manifest_supplied_ = false; +}; + } // namespace aapt #endif // AAPT_TEST_FIXTURE_H
\ No newline at end of file |