Fix asset compression to check name ends with arg
There exists a discrepancy between how aapt1 and aapt2 prevent the
compression of assets using the -0 flag. aapt1 checked if the file name
ended with an argument, but aapt2 is checking if the file extension
exactly matches the argument. This change makes aapt2 behave the same as
aapt1 for asset compression using the -0 flag.
Bug: 132823799
Test: aapt2_tests
Change-Id: I641b3ebce29e4407b543faea373a5ce516b70cda
diff --git a/tools/aapt2/cmd/Compile_test.cpp b/tools/aapt2/cmd/Compile_test.cpp
index 5f637bd..fb786a3 100644
--- a/tools/aapt2/cmd/Compile_test.cpp
+++ b/tools/aapt2/cmd/Compile_test.cpp
@@ -200,7 +200,7 @@
const std::string compiled_files_dir = ctf->GetTestPath("/compiled_" + file_name);
const std::string out_apk = ctf->GetTestPath("/" + file_name + ".apk");
- CHECK(ctf->WriteFile(source_file, sTranslatableXmlContent));
+ ctf->WriteFile(source_file, sTranslatableXmlContent);
CHECK(file::mkdirs(compiled_files_dir.data()));
ASSERT_EQ(CompileCommand(&diag).Execute({
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index f354bb6..4b97722 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -297,6 +297,25 @@
};
};
+template <typename T>
+uint32_t GetCompressionFlags(const StringPiece& str, T options) {
+ if (options.do_not_compress_anything) {
+ return 0;
+ }
+
+ if (options.regex_to_not_compress
+ && std::regex_search(str.to_string(), options.regex_to_not_compress.value())) {
+ return 0;
+ }
+
+ for (const std::string& extension : options.extensions_to_not_compress) {
+ if (util::EndsWith(str, extension)) {
+ return 0;
+ }
+ }
+ return ArchiveEntry::kCompress;
+}
+
class ResourceFileFlattener {
public:
ResourceFileFlattener(const ResourceFileFlattenerOptions& options, IAaptContext* context,
@@ -321,8 +340,6 @@
std::string dst_path;
};
- uint32_t GetCompressionFlags(const StringPiece& str);
-
std::vector<std::unique_ptr<xml::XmlResource>> LinkAndVersionXmlFile(ResourceTable* table,
FileOperation* file_op);
@@ -381,26 +398,6 @@
}
}
-// TODO(rtmitchell): turn this function into a variable that points to a method that retrieves the
-// compression flag
-uint32_t ResourceFileFlattener::GetCompressionFlags(const StringPiece& str) {
- if (options_.do_not_compress_anything) {
- return 0;
- }
-
- if (options_.regex_to_not_compress
- && std::regex_search(str.to_string(), options_.regex_to_not_compress.value())) {
- return 0;
- }
-
- for (const std::string& extension : options_.extensions_to_not_compress) {
- if (util::EndsWith(str, extension)) {
- return 0;
- }
- }
- return ArchiveEntry::kCompress;
-}
-
static bool IsTransitionElement(const std::string& name) {
return name == "fade" || name == "changeBounds" || name == "slide" || name == "explode" ||
name == "changeImageTransform" || name == "changeTransform" ||
@@ -640,7 +637,8 @@
}
} else {
error |= !io::CopyFileToArchive(context_, file_op.file_to_copy, file_op.dst_path,
- GetCompressionFlags(file_op.dst_path), archive_writer);
+ GetCompressionFlags(file_op.dst_path, options_),
+ archive_writer);
}
}
}
@@ -1547,16 +1545,7 @@
}
for (auto& entry : merged_assets) {
- uint32_t compression_flags = ArchiveEntry::kCompress;
- std::string extension = file::GetExtension(entry.first).to_string();
-
- if (options_.do_not_compress_anything
- || options_.extensions_to_not_compress.count(extension) > 0
- || (options_.regex_to_not_compress
- && std::regex_search(extension, options_.regex_to_not_compress.value()))) {
- compression_flags = 0u;
- }
-
+ uint32_t compression_flags = GetCompressionFlags(entry.first, options_);
if (!io::CopyFileToArchive(context_, entry.second.get(), entry.first, compression_flags,
writer)) {
return false;
diff --git a/tools/aapt2/cmd/Link.h b/tools/aapt2/cmd/Link.h
index 7c58385..5b0653e 100644
--- a/tools/aapt2/cmd/Link.h
+++ b/tools/aapt2/cmd/Link.h
@@ -248,7 +248,7 @@
"Changes the name of the target package for instrumentation. Most useful\n"
"when used in conjunction with --rename-manifest-package.",
&options_.manifest_fixer_options.rename_instrumentation_target_package);
- AddOptionalFlagList("-0", "File extensions not to compress.",
+ AddOptionalFlagList("-0", "File suffix not to compress.",
&options_.extensions_to_not_compress);
AddOptionalSwitch("--no-compress", "Do not compress any resources.",
&options_.do_not_compress_anything);
diff --git a/tools/aapt2/cmd/Link_test.cpp b/tools/aapt2/cmd/Link_test.cpp
index 9ea93f6..32ed1dd 100644
--- a/tools/aapt2/cmd/Link_test.cpp
+++ b/tools/aapt2/cmd/Link_test.cpp
@@ -43,10 +43,8 @@
// Load the binary xml tree
android::ResXMLTree tree;
std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag);
-
std::unique_ptr<io::IData> data = OpenFileAsData(apk.get(), "res/xml/test.xml");
ASSERT_THAT(data, Ne(nullptr));
-
AssertLoadXml(apk.get(), data.get(), &tree);
// Check that the raw string index has not been assigned
@@ -71,10 +69,8 @@
// Load the binary xml tree
android::ResXMLTree tree;
std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag);
-
std::unique_ptr<io::IData> data = OpenFileAsData(apk.get(), "res/xml/test.xml");
ASSERT_THAT(data, Ne(nullptr));
-
AssertLoadXml(apk.get(), data.get(), &tree);
// Check that the raw string index has been set to the correct string pool entry
@@ -83,4 +79,96 @@
EXPECT_THAT(util::GetString(tree.getStrings(), static_cast<size_t>(raw_index)), Eq("007"));
}
+TEST_F(LinkTest, NoCompressAssets) {
+ StdErrDiagnostics diag;
+ std::string content(500, 'a');
+ WriteFile(GetTestPath("assets/testtxt"), content);
+ WriteFile(GetTestPath("assets/testtxt2"), content);
+ WriteFile(GetTestPath("assets/test.txt"), content);
+ WriteFile(GetTestPath("assets/test.hello.txt"), content);
+ WriteFile(GetTestPath("assets/test.hello.xml"), content);
+
+ const std::string out_apk = GetTestPath("out.apk");
+ std::vector<std::string> link_args = {
+ "--manifest", GetDefaultManifest(),
+ "-o", out_apk,
+ "-0", ".txt",
+ "-0", "txt2",
+ "-0", ".hello.txt",
+ "-0", "hello.xml",
+ "-A", GetTestPath("assets")
+ };
+
+ ASSERT_TRUE(Link(link_args, &diag));
+
+ std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag);
+ ASSERT_THAT(apk, Ne(nullptr));
+ io::IFileCollection* zip = apk->GetFileCollection();
+ ASSERT_THAT(zip, Ne(nullptr));
+
+ auto file = zip->FindFile("assets/testtxt");
+ ASSERT_THAT(file, Ne(nullptr));
+ EXPECT_TRUE(file->WasCompressed());
+
+ file = zip->FindFile("assets/testtxt2");
+ ASSERT_THAT(file, Ne(nullptr));
+ EXPECT_FALSE(file->WasCompressed());
+
+ file = zip->FindFile("assets/test.txt");
+ ASSERT_THAT(file, Ne(nullptr));
+ EXPECT_FALSE(file->WasCompressed());
+
+ file = zip->FindFile("assets/test.hello.txt");
+ ASSERT_THAT(file, Ne(nullptr));
+ EXPECT_FALSE(file->WasCompressed());
+
+ file = zip->FindFile("assets/test.hello.xml");
+ ASSERT_THAT(file, Ne(nullptr));
+ EXPECT_FALSE(file->WasCompressed());
+}
+
+TEST_F(LinkTest, NoCompressResources) {
+ StdErrDiagnostics diag;
+ std::string content(500, 'a');
+ const std::string compiled_files_dir = GetTestPath("compiled");
+ ASSERT_TRUE(CompileFile(GetTestPath("res/raw/testtxt"), content, compiled_files_dir, &diag));
+ ASSERT_TRUE(CompileFile(GetTestPath("res/raw/test.txt"), content, compiled_files_dir, &diag));
+ ASSERT_TRUE(CompileFile(GetTestPath("res/raw/test1.hello.txt"), content, compiled_files_dir,
+ &diag));
+ ASSERT_TRUE(CompileFile(GetTestPath("res/raw/test2.goodbye.xml"), content, compiled_files_dir,
+ &diag));
+
+ const std::string out_apk = GetTestPath("out.apk");
+ std::vector<std::string> link_args = {
+ "--manifest", GetDefaultManifest(),
+ "-o", out_apk,
+ "-0", ".txt",
+ "-0", ".hello.txt",
+ "-0", "goodbye.xml",
+ };
+
+ ASSERT_TRUE(Link(link_args, compiled_files_dir, &diag));
+
+ std::unique_ptr<LoadedApk> apk = LoadedApk::LoadApkFromPath(out_apk, &diag);
+ ASSERT_THAT(apk, Ne(nullptr));
+ io::IFileCollection* zip = apk->GetFileCollection();
+ ASSERT_THAT(zip, Ne(nullptr));
+
+ auto file = zip->FindFile("res/raw/testtxt");
+ ASSERT_THAT(file, Ne(nullptr));
+ EXPECT_TRUE(file->WasCompressed());
+
+ file = zip->FindFile("res/raw/test.txt");
+ ASSERT_THAT(file, Ne(nullptr));
+ EXPECT_FALSE(file->WasCompressed());
+
+ file = zip->FindFile("res/raw/test1.hello.hello.txt");
+ ASSERT_THAT(file, Ne(nullptr));
+ EXPECT_FALSE(file->WasCompressed());
+
+ file = zip->FindFile("res/raw/test2.goodbye.goodbye.xml");
+ ASSERT_THAT(file, Ne(nullptr));
+ EXPECT_FALSE(file->WasCompressed());
+}
+
} // namespace aapt
\ No newline at end of file
diff --git a/tools/aapt2/test/Fixture.cpp b/tools/aapt2/test/Fixture.cpp
index a51b4a4..5386802 100644
--- a/tools/aapt2/test/Fixture.cpp
+++ b/tools/aapt2/test/Fixture.cpp
@@ -80,7 +80,7 @@
ClearDirectory(temp_dir_);
}
-bool TestDirectoryFixture::WriteFile(const std::string& path, const std::string& contents) {
+void TestDirectoryFixture::WriteFile(const std::string& path, const std::string& contents) {
CHECK(util::StartsWith(path, temp_dir_))
<< "Attempting to create a file outside of test temporary directory.";
@@ -91,16 +91,31 @@
file::mkdirs(dirs);
}
- return android::base::WriteStringToFile(contents, path);
+ CHECK(android::base::WriteStringToFile(contents, path));
}
bool CommandTestFixture::CompileFile(const std::string& path, const std::string& contents,
const android::StringPiece& out_dir, IDiagnostics* diag) {
- CHECK(WriteFile(path, contents));
+ WriteFile(path, contents);
CHECK(file::mkdirs(out_dir.data()));
return CompileCommand(diag).Execute({path, "-o", out_dir, "-v"}, &std::cerr) == 0;
}
+bool CommandTestFixture::Link(const std::vector<std::string>& args, IDiagnostics* diag) {
+ std::vector<android::StringPiece> link_args;
+ for(const std::string& arg : args) {
+ link_args.emplace_back(arg);
+ }
+
+ // Link against the android SDK
+ std::string android_sdk = file::BuildPath({android::base::GetExecutableDirectory(),
+ "integration-tests", "CommandTests",
+ "android-28.jar"});
+ link_args.insert(link_args.end(), {"-I", android_sdk});
+
+ return LinkCommand(diag).Execute(link_args, &std::cerr) == 0;
+}
+
bool CommandTestFixture::Link(const std::vector<std::string>& args,
const android::StringPiece& flat_dir, IDiagnostics* diag) {
std::vector<android::StringPiece> link_args;
@@ -128,10 +143,10 @@
std::string CommandTestFixture::GetDefaultManifest(const char* package_name) {
const std::string manifest_file = GetTestPath("AndroidManifest.xml");
- CHECK(WriteFile(manifest_file, android::base::StringPrintf(R"(
+ WriteFile(manifest_file, android::base::StringPrintf(R"(
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="%s">
- </manifest>)", package_name)));
+ </manifest>)", package_name));
return manifest_file;
}
diff --git a/tools/aapt2/test/Fixture.h b/tools/aapt2/test/Fixture.h
index fce2aeb..457d65e 100644
--- a/tools/aapt2/test/Fixture.h
+++ b/tools/aapt2/test/Fixture.h
@@ -58,7 +58,7 @@
// Creates a file with the specified contents, creates any intermediate directories in the
// process. The file path must be an absolute path within the test directory.
- bool WriteFile(const std::string& path, const std::string& contents);
+ void WriteFile(const std::string& path, const std::string& contents);
private:
std::string temp_dir_;
@@ -75,6 +75,9 @@
bool CompileFile(const std::string& path, const std::string& contents,
const android::StringPiece& flat_out_dir, IDiagnostics* diag);
+ // Executes the link command with the specified arguments.
+ bool Link(const std::vector<std::string>& args, IDiagnostics* diag);
+
// Executes the link command with the specified arguments. The flattened files residing in the
// flat directory will be added to the link command as file arguments.
bool Link(const std::vector<std::string>& args, const android::StringPiece& flat_dir,