Allows features to link to other feature splits without namespacing.

Add uses-split dependencies to AppInfo. At link time, this is used
and allows features to reference other features, before
resource namespacing is implemented in Android Studio.

bug:135681292
Test: Link_test, ReferenceLinker_test, and integration tests.
Change-Id: Ifdf0067e7370552b6b9d4d6d4713d4484b6ea154
This commit is contained in:
Udam Saini
2019-06-18 16:50:34 -07:00
parent 9aeeb3e5e9
commit b228df32d9
20 changed files with 244 additions and 20 deletions

View File

@@ -18,7 +18,7 @@ android_test {
name: "FeatureSplit1",
srcs: ["**/*.java"],
sdk_version: "current",
libs: ["FeatureSplitBase"],
libs: ["FeatureSplitBase", "FeatureSplit2"],
aaptflags: [
"--package-id",
"0x80",

View File

@@ -19,6 +19,7 @@
featureSplit="feature1">
<uses-sdk android:minSdkVersion="21" />
<uses-split android:name="feature2" />
<application>
<activity android:name=".one.One" android:label="Feature One">

View File

@@ -2,4 +2,5 @@
<TextView xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/text"
android:layout_width="wrap_content"
android:layout_height="wrap_content" />
android:layout_height="wrap_content"
android:text="@string/feature2_string" />

View File

@@ -20,7 +20,8 @@
<integer name="test_integer2">200</integer>
<color name="test_color2">#00ff00</color>
<string-array name="string_array2">
<item>@string/app_title</item>
<item>@string/app_title</item>
<item>@string/feature2_string</item>
</string-array>
</resources>

View File

@@ -15,10 +15,11 @@
-->
<resources>
<string name="feature2_string">feature 2 string referenced from feature 1</string>
<integer name="test_integer3">300</integer>
<color name="test_color3">#0000ff</color>
<string-array name="string_array3">
<item>@string/app_title</item>
<item>@string/app_title</item>
</string-array>
</resources>

View File

@@ -17,6 +17,7 @@
#ifndef AAPT_APP_INFO_H
#define AAPT_APP_INFO_H
#include <set>
#include <string>
#include "util/Maybe.h"
@@ -42,6 +43,9 @@ struct AppInfo {
// The app's split name, if it is a split.
Maybe<std::string> split_name;
// The split names that this split depends on.
std::set<std::string> split_name_dependencies;
};
} // namespace aapt

View File

@@ -629,6 +629,12 @@ class CompileContext : public IAaptContext {
return 0;
}
const std::set<std::string>& GetSplitNameDependencies() override {
UNIMPLEMENTED(FATAL) << "No Split Name Dependencies be needed in compile phase";
static std::set<std::string> empty;
return empty;
}
private:
DISALLOW_COPY_AND_ASSIGN(CompileContext);

View File

@@ -243,6 +243,12 @@ class Context : public IAaptContext {
return 0u;
}
const std::set<std::string>& GetSplitNameDependencies() override {
UNIMPLEMENTED(FATAL) << "Split Name Dependencies should not be necessary";
static std::set<std::string> empty;
return empty;
}
bool verbose_ = false;
std::string package_;

View File

@@ -65,6 +65,12 @@ class DiffContext : public IAaptContext {
return 0;
}
const std::set<std::string>& GetSplitNameDependencies() override {
UNIMPLEMENTED(FATAL) << "Split Name Dependencies should not be necessary";
static std::set<std::string> empty;
return empty;
}
private:
std::string empty_;
StdErrDiagnostics diagnostics_;

View File

@@ -118,6 +118,12 @@ class DumpContext : public IAaptContext {
return 0;
}
const std::set<std::string>& GetSplitNameDependencies() override {
UNIMPLEMENTED(FATAL) << "Split Name Dependencies should not be necessary";
static std::set<std::string> empty;
return empty;
}
private:
StdErrDiagnostics diagnostics_;
bool verbose_ = false;

View File

@@ -140,6 +140,14 @@ class LinkContext : public IAaptContext {
min_sdk_version_ = minSdk;
}
const std::set<std::string>& GetSplitNameDependencies() override {
return split_name_dependencies_;
}
void SetSplitNameDependencies(const std::set<std::string>& split_name_dependencies) {
split_name_dependencies_ = split_name_dependencies;
}
private:
DISALLOW_COPY_AND_ASSIGN(LinkContext);
@@ -151,6 +159,7 @@ class LinkContext : public IAaptContext {
SymbolTable symbols_;
bool verbose_ = false;
int min_sdk_version_ = 0;
std::set<std::string> split_name_dependencies_;
};
// A custom delegate that generates compatible pre-O IDs for use with feature splits.
@@ -963,6 +972,17 @@ class Linker {
app_info.min_sdk_version = ResourceUtils::ParseSdkVersion(min_sdk->value);
}
}
for (const xml::Element* child_el : manifest_el->GetChildElements()) {
if (child_el->namespace_uri.empty() && child_el->name == "uses-split") {
if (const xml::Attribute* split_name =
child_el->FindAttribute(xml::kSchemaAndroid, "name")) {
if (!split_name->value.empty()) {
app_info.split_name_dependencies.insert(split_name->value);
}
}
}
}
return app_info;
}
@@ -1770,6 +1790,7 @@ class Linker {
context_->SetMinSdkVersion(app_info_.min_sdk_version.value_or_default(0));
context_->SetNameManglerPolicy(NameManglerPolicy{context_->GetCompilationPackage()});
context_->SetSplitNameDependencies(app_info_.split_name_dependencies);
// Override the package ID when it is "android".
if (context_->GetCompilationPackage() == "android") {

View File

@@ -14,6 +14,7 @@
* limitations under the License.
*/
#include "AppInfo.h"
#include "Link.h"
#include "LoadedApk.h"
@@ -253,4 +254,67 @@ TEST_F(LinkTest, OverrideStylesInsteadOfOverlaying) {
EXPECT_EQ(actual_style->entries[0].key.id, 0x010100d4); // android:background
}
TEST_F(LinkTest, AppInfoWithUsesSplit) {
StdErrDiagnostics diag;
const std::string base_files_dir = GetTestPath("base");
ASSERT_TRUE(CompileFile(GetTestPath("res/values/values.xml"),
R"(<resources>
<string name="bar">bar</string>
</resources>)",
base_files_dir, &diag));
const std::string base_apk = GetTestPath("base.apk");
std::vector<std::string> link_args = {
"--manifest", GetDefaultManifest("com.aapt2.app"),
"-o", base_apk,
};
ASSERT_TRUE(Link(link_args, base_files_dir, &diag));
const std::string feature_manifest = GetTestPath("feature_manifest.xml");
WriteFile(feature_manifest, android::base::StringPrintf(R"(
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="com.aapt2.app" split="feature1">
</manifest>)"));
const std::string feature_files_dir = GetTestPath("feature");
ASSERT_TRUE(CompileFile(GetTestPath("res/values/values.xml"),
R"(<resources>
<string name="foo">foo</string>
</resources>)",
feature_files_dir, &diag));
const std::string feature_apk = GetTestPath("feature.apk");
const std::string feature_package_id = "0x80";
link_args = {
"--manifest", feature_manifest,
"-I", base_apk,
"--package-id", feature_package_id,
"-o", feature_apk,
};
ASSERT_TRUE(Link(link_args, feature_files_dir, &diag));
const std::string feature2_manifest = GetTestPath("feature2_manifest.xml");
WriteFile(feature2_manifest, android::base::StringPrintf(R"(
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="com.aapt2.app" split="feature2">
<uses-split android:name="feature1"/>
</manifest>)"));
const std::string feature2_files_dir = GetTestPath("feature2");
ASSERT_TRUE(CompileFile(GetTestPath("res/values/values.xml"),
R"(<resources>
<string-array name="string_array">
<item>@string/bar</item>
<item>@string/foo</item>
</string-array>
</resources>)",
feature2_files_dir, &diag));
const std::string feature2_apk = GetTestPath("feature2.apk");
const std::string feature2_package_id = "0x81";
link_args = {
"--manifest", feature2_manifest,
"-I", base_apk,
"-I", feature_apk,
"--package-id", feature2_package_id,
"-o", feature2_apk,
};
ASSERT_TRUE(Link(link_args, feature2_files_dir, &diag));
}
} // namespace aapt

View File

@@ -108,6 +108,12 @@ class OptimizeContext : public IAaptContext {
return sdk_version_;
}
const std::set<std::string>& GetSplitNameDependencies() override {
UNIMPLEMENTED(FATAL) << "Split Name Dependencies should not be necessary";
static std::set<std::string> empty;
return empty;
}
private:
DISALLOW_COPY_AND_ASSIGN(OptimizeContext);

View File

@@ -17,6 +17,7 @@
#include "link/ReferenceLinker.h"
#include "android-base/logging.h"
#include "android-base/stringprintf.h"
#include "androidfw/ResourceTypes.h"
#include "Diagnostics.h"
@@ -33,6 +34,7 @@
using ::aapt::ResourceUtils::StringBuilder;
using ::android::StringPiece;
using ::android::base::StringPrintf;
namespace aapt {
@@ -81,7 +83,7 @@ class ReferenceLinkerVisitor : public DescendingValueVisitor {
// Find the attribute in the symbol table and check if it is visible from this callsite.
const SymbolTable::Symbol* symbol = ReferenceLinker::ResolveAttributeCheckVisibility(
transformed_reference, callsite_, symbols_, &err_str);
transformed_reference, callsite_, context_, symbols_, &err_str);
if (symbol) {
// Assign our style key the correct ID. The ID may not exist.
entry.key.id = symbol->id;
@@ -203,12 +205,35 @@ bool IsSymbolVisible(const SymbolTable::Symbol& symbol, const Reference& ref,
const SymbolTable::Symbol* ReferenceLinker::ResolveSymbol(const Reference& reference,
const CallSite& callsite,
IAaptContext* context,
SymbolTable* symbols) {
if (reference.name) {
const ResourceName& name = reference.name.value();
if (name.package.empty()) {
// Use the callsite's package name if no package name was defined.
return symbols->FindByName(ResourceName(callsite.package, name.type, name.entry));
const SymbolTable::Symbol* symbol = symbols->FindByName(
ResourceName(callsite.package, name.type, name.entry));
if (symbol) {
return symbol;
}
// If the callsite package is the same as the current compilation package,
// check the feature split dependencies as well. Feature split resources
// can be referenced without a namespace, just like the base package.
// TODO: modify the package name of included splits instead of having the
// symbol table look up the resource in in every package. b/136105066
if (callsite.package == context->GetCompilationPackage()) {
const auto& split_name_dependencies = context->GetSplitNameDependencies();
for (const std::string& split_name : split_name_dependencies) {
std::string split_package =
StringPrintf("%s.%s", callsite.package.c_str(), split_name.c_str());
symbol = symbols->FindByName(ResourceName(split_package, name.type, name.entry));
if (symbol) {
return symbol;
}
}
}
return nullptr;
}
return symbols->FindByName(name);
} else if (reference.id) {
@@ -220,9 +245,10 @@ const SymbolTable::Symbol* ReferenceLinker::ResolveSymbol(const Reference& refer
const SymbolTable::Symbol* ReferenceLinker::ResolveSymbolCheckVisibility(const Reference& reference,
const CallSite& callsite,
IAaptContext* context,
SymbolTable* symbols,
std::string* out_error) {
const SymbolTable::Symbol* symbol = ResolveSymbol(reference, callsite, symbols);
const SymbolTable::Symbol* symbol = ResolveSymbol(reference, callsite, context, symbols);
if (!symbol) {
if (out_error) *out_error = "not found";
return nullptr;
@@ -236,10 +262,10 @@ const SymbolTable::Symbol* ReferenceLinker::ResolveSymbolCheckVisibility(const R
}
const SymbolTable::Symbol* ReferenceLinker::ResolveAttributeCheckVisibility(
const Reference& reference, const CallSite& callsite, SymbolTable* symbols,
std::string* out_error) {
const Reference& reference, const CallSite& callsite, IAaptContext* context,
SymbolTable* symbols, std::string* out_error) {
const SymbolTable::Symbol* symbol =
ResolveSymbolCheckVisibility(reference, callsite, symbols, out_error);
ResolveSymbolCheckVisibility(reference, callsite, context, symbols, out_error);
if (!symbol) {
return nullptr;
}
@@ -253,10 +279,11 @@ const SymbolTable::Symbol* ReferenceLinker::ResolveAttributeCheckVisibility(
Maybe<xml::AaptAttribute> ReferenceLinker::CompileXmlAttribute(const Reference& reference,
const CallSite& callsite,
IAaptContext* context,
SymbolTable* symbols,
std::string* out_error) {
const SymbolTable::Symbol* symbol =
ResolveAttributeCheckVisibility(reference, callsite, symbols, out_error);
ResolveAttributeCheckVisibility(reference, callsite, context, symbols, out_error);
if (!symbol) {
return {};
}
@@ -335,7 +362,7 @@ bool ReferenceLinker::LinkReference(const CallSite& callsite, Reference* referen
std::string err_str;
const SymbolTable::Symbol* s =
ResolveSymbolCheckVisibility(transformed_reference, callsite, symbols, &err_str);
ResolveSymbolCheckVisibility(transformed_reference, callsite, context, symbols, &err_str);
if (s) {
// The ID may not exist. This is fine because of the possibility of building
// against libraries without assigned IDs.

View File

@@ -39,13 +39,16 @@ class ReferenceLinker : public IResourceTableConsumer {
// package if the reference has no package name defined (implicit).
// Returns nullptr if the symbol was not found.
static const SymbolTable::Symbol* ResolveSymbol(const Reference& reference,
const CallSite& callsite, SymbolTable* symbols);
const CallSite& callsite,
IAaptContext* context,
SymbolTable* symbols);
// Performs name mangling and looks up the resource in the symbol table. If the symbol is not
// visible by the reference at the callsite, nullptr is returned.
// `out_error` holds the error message.
static const SymbolTable::Symbol* ResolveSymbolCheckVisibility(const Reference& reference,
const CallSite& callsite,
IAaptContext* context,
SymbolTable* symbols,
std::string* out_error);
@@ -53,6 +56,7 @@ class ReferenceLinker : public IResourceTableConsumer {
// That is, the return value will have a non-null value for ISymbolTable::Symbol::attribute.
static const SymbolTable::Symbol* ResolveAttributeCheckVisibility(const Reference& reference,
const CallSite& callsite,
IAaptContext* context,
SymbolTable* symbols,
std::string* out_error);
@@ -60,6 +64,7 @@ class ReferenceLinker : public IResourceTableConsumer {
// If resolution fails, outError holds the error message.
static Maybe<xml::AaptAttribute> CompileXmlAttribute(const Reference& reference,
const CallSite& callsite,
IAaptContext* context,
SymbolTable* symbols,
std::string* out_error);

View File

@@ -266,8 +266,13 @@ TEST(ReferenceLinkerTest, AppsWithSamePackageButDifferentIdAreVisibleNonPublic)
std::string error;
const CallSite call_site{"com.app.test"};
std::unique_ptr<IAaptContext> context =
test::ContextBuilder()
.SetCompilationPackage("com.app.test")
.SetPackageId(0x7f)
.Build();
const SymbolTable::Symbol* symbol = ReferenceLinker::ResolveSymbolCheckVisibility(
*test::BuildReference("com.app.test:string/foo"), call_site, &table, &error);
*test::BuildReference("com.app.test:string/foo"), call_site, context.get(), &table, &error);
ASSERT_THAT(symbol, NotNull());
EXPECT_TRUE(error.empty());
}
@@ -281,17 +286,23 @@ TEST(ReferenceLinkerTest, AppsWithDifferentPackageCanNotUseEachOthersAttribute)
.AddPublicSymbol("com.app.test:attr/public_foo", ResourceId(0x7f010001),
test::AttributeBuilder().Build())
.Build());
std::unique_ptr<IAaptContext> context =
test::ContextBuilder()
.SetCompilationPackage("com.app.ext")
.SetPackageId(0x7f)
.Build();
std::string error;
const CallSite call_site{"com.app.ext"};
EXPECT_FALSE(ReferenceLinker::CompileXmlAttribute(
*test::BuildReference("com.app.test:attr/foo"), call_site, &table, &error));
*test::BuildReference("com.app.test:attr/foo"), call_site, context.get(), &table, &error));
EXPECT_FALSE(error.empty());
error = "";
ASSERT_TRUE(ReferenceLinker::CompileXmlAttribute(
*test::BuildReference("com.app.test:attr/public_foo"), call_site, &table, &error));
*test::BuildReference("com.app.test:attr/public_foo"), call_site, context.get(), &table,
&error));
EXPECT_TRUE(error.empty());
}
@@ -302,20 +313,62 @@ TEST(ReferenceLinkerTest, ReferenceWithNoPackageUsesCallSitePackage) {
.AddSymbol("com.app.test:string/foo", ResourceId(0x7f010000))
.AddSymbol("com.app.lib:string/foo", ResourceId(0x7f010001))
.Build());
std::unique_ptr<IAaptContext> context =
test::ContextBuilder()
.SetCompilationPackage("com.app.test")
.SetPackageId(0x7f)
.Build();
const SymbolTable::Symbol* s = ReferenceLinker::ResolveSymbol(*test::BuildReference("string/foo"),
CallSite{"com.app.test"}, &table);
CallSite{"com.app.test"},
context.get(), &table);
ASSERT_THAT(s, NotNull());
EXPECT_THAT(s->id, Eq(make_value<ResourceId>(0x7f010000)));
s = ReferenceLinker::ResolveSymbol(*test::BuildReference("string/foo"), CallSite{"com.app.lib"},
&table);
context.get(), &table);
ASSERT_THAT(s, NotNull());
EXPECT_THAT(s->id, Eq(make_value<ResourceId>(0x7f010001)));
EXPECT_THAT(ReferenceLinker::ResolveSymbol(*test::BuildReference("string/foo"),
CallSite{"com.app.bad"}, &table),
CallSite{"com.app.bad"}, context.get(), &table),
IsNull());
}
TEST(ReferenceLinkerTest, ReferenceSymbolFromOtherSplit) {
NameMangler mangler(NameManglerPolicy{"com.app.test"});
SymbolTable table(&mangler);
table.AppendSource(test::StaticSymbolSourceBuilder()
.AddSymbol("com.app.test.feature:string/bar", ResourceId(0x80010000))
.Build());
std::set<std::string> split_name_dependencies;
split_name_dependencies.insert("feature");
std::unique_ptr<IAaptContext> context =
test::ContextBuilder()
.SetCompilationPackage("com.app.test")
.SetPackageId(0x81)
.SetSplitNameDependencies(split_name_dependencies)
.Build();
const SymbolTable::Symbol* s = ReferenceLinker::ResolveSymbol(*test::BuildReference("string/bar"),
CallSite{"com.app.test"},
context.get(), &table);
ASSERT_THAT(s, NotNull());
EXPECT_THAT(s->id, Eq(make_value<ResourceId>(0x80010000)));
s = ReferenceLinker::ResolveSymbol(*test::BuildReference("string/foo"), CallSite{"com.app.lib"},
context.get(), &table);
EXPECT_THAT(s, IsNull());
context =
test::ContextBuilder()
.SetCompilationPackage("com.app.test")
.SetPackageId(0x81)
.Build();
s = ReferenceLinker::ResolveSymbol(*test::BuildReference("string/bar"),CallSite{"com.app.test"},
context.get(), &table);
EXPECT_THAT(s, IsNull());
}
} // namespace aapt

View File

@@ -99,7 +99,7 @@ class XmlVisitor : public xml::PackageAwareVisitor {
std::string err_str;
attr.compiled_attribute =
ReferenceLinker::CompileXmlAttribute(attr_ref, callsite_, symbols_, &err_str);
ReferenceLinker::CompileXmlAttribute(attr_ref, callsite_, context_, symbols_, &err_str);
if (!attr.compiled_attribute) {
DiagMessage error_msg(source);

View File

@@ -101,6 +101,10 @@ class ContextWrapper : public IAaptContext {
util::make_unique<SourcePathDiagnostics>(Source{source}, context_->GetDiagnostics());
}
const std::set<std::string>& GetSplitNameDependencies() override {
return context_->GetSplitNameDependencies();
}
private:
IAaptContext* context_;
std::unique_ptr<SourcePathDiagnostics> source_diag_;

View File

@@ -19,6 +19,7 @@
#include <iostream>
#include <list>
#include <set>
#include <sstream>
#include "Diagnostics.h"
@@ -50,6 +51,7 @@ struct IAaptContext {
virtual NameMangler* GetNameMangler() = 0;
virtual bool IsVerbose() = 0;
virtual int GetMinSdkVersion() = 0;
virtual const std::set<std::string>& GetSplitNameDependencies() = 0;
};
struct IResourceTableConsumer {

View File

@@ -81,6 +81,10 @@ class Context : public IAaptContext {
return min_sdk_version_;
}
const std::set<std::string>& GetSplitNameDependencies() override {
return split_name_dependencies_;
}
private:
DISALLOW_COPY_AND_ASSIGN(Context);
@@ -93,6 +97,7 @@ class Context : public IAaptContext {
NameMangler name_mangler_;
SymbolTable symbols_;
int min_sdk_version_;
std::set<std::string> split_name_dependencies_;
};
class ContextBuilder {
@@ -127,6 +132,11 @@ class ContextBuilder {
return *this;
}
ContextBuilder& SetSplitNameDependencies(const std::set<std::string>& split_name_dependencies) {
context_->split_name_dependencies_ = split_name_dependencies;
return *this;
}
std::unique_ptr<Context> Build() { return std::move(context_); }
private: