Merge "Use R field directly for shared lib styleable attrs" into sc-dev

This commit is contained in:
Ryan Mitchell
2021-03-02 20:02:24 +00:00
committed by Android (Google) Code Review
10 changed files with 257 additions and 51 deletions

View File

@@ -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");

View File

@@ -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

View File

@@ -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);
}

View File

@@ -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 {

View File

@@ -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;

View File

@@ -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,

View File

@@ -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;"));
}

View File

@@ -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 {};

View File

@@ -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

View File

@@ -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