Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 81dfca0e authored by Ryan Mitchell's avatar Ryan Mitchell
Browse files

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
parent 1dbbe113

tools/aapt2/cmd/Compile_test.cpp

+1 −1
Original line number Diff line number Diff line
@@ -200,7 +200,7 @@ static void AssertTranslations(CommandTestFixture *ctf, std::string file_name, 
  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({

tools/aapt2/cmd/Link.cpp

+22 −33
Original line number Diff line number Diff line
@@ -297,6 +297,25 @@ struct R { 
  };

};



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 @@ class ResourceFileFlattener { 
    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 @@ ResourceFileFlattener::ResourceFileFlattener(const ResourceFileFlattenerOptions& 
  }

}



// 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 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv 
          }

        } 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 @@ class Linker { 
    }



    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;

tools/aapt2/cmd/Link.h

+1 −1
Original line number Diff line number Diff line
@@ -248,7 +248,7 @@ class LinkCommand : public Command { 
        "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);

tools/aapt2/cmd/Link_test.cpp

+92 −4
Original line number Diff line number Diff line
@@ -43,10 +43,8 @@ TEST_F(LinkTest, RemoveRawXmlStrings) { 
  // 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 @@ TEST_F(LinkTest, KeepRawXmlStrings) { 
  // 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 @@ TEST_F(LinkTest, KeepRawXmlStrings) { 
  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

tools/aapt2/test/Fixture.cpp

+20 −5
Original line number Diff line number Diff line
@@ -80,7 +80,7 @@ void TestDirectoryFixture::TearDown() { 
  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 @@ bool TestDirectoryFixture::WriteFile(const std::string& path, const std::string& 
    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 @@ bool CommandTestFixture::Link(const std::vector<std::string>& args, 


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;

}