Skip to content

Commit

Permalink
Respect ART_APEX_DATA for default artifact dir
Browse files Browse the repository at this point in the history
This change does not change the default artifact directory. It just
makes odrefresh not define the default directory by itself, but relies
on GetArtApexData from libartbase instead (which has the same default).

Bug: 206468124
Test: Run odrefresh in the VM. No longer seeing files in the
      hard-coded directory.
Test: atest art_standalone_odrefresh_tests
Test: atest art_standalone_libartbase_tests
Change-Id: I45bcc145dbccb3b953f359f6c10fa186f251600c
  • Loading branch information
victorhsieh authored and Treehugger Robot committed Dec 2, 2021
1 parent 80f93a8 commit fb00761
Show file tree
Hide file tree
Showing 9 changed files with 38 additions and 29 deletions.
2 changes: 1 addition & 1 deletion libartbase/base/file_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ bool GetDalvikCacheFilename(const char* location,
return GetLocationEncodedFilename(location, cache_location, filename, error_msg);
}

static std::string GetApexDataDalvikCacheDirectory(InstructionSet isa) {
std::string GetApexDataDalvikCacheDirectory(InstructionSet isa) {
if (isa != InstructionSet::kNone) {
return GetDalvikCacheDirectory(GetArtApexData(), GetInstructionSetString(isa));
}
Expand Down
4 changes: 4 additions & 0 deletions libartbase/base/file_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ void GetDalvikCache(const char* subdir, bool create_if_absent, std::string* dalv
bool GetDalvikCacheFilename(const char* location, const char* cache_location,
std::string* filename, std::string* error_msg);

// Returns the absolute dalvik-cache path. The path may include the instruction set sub-directory
// if specified.
std::string GetApexDataDalvikCacheDirectory(InstructionSet isa);

// Gets the oat location in the ART APEX data directory for a DEX file installed anywhere other
// than in an APEX. Returns the oat filename if `location` is valid, empty string otherwise.
std::string GetApexDataOatFilename(std::string_view location, InstructionSet isa);
Expand Down
8 changes: 8 additions & 0 deletions libartbase/base/file_utils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,14 @@ TEST_F(FileUtilsTest, ReplaceFileExtension) {
EXPECT_EQ("/.directory/file.vdex", ReplaceFileExtension("/.directory/file", "vdex"));
}

TEST_F(FileUtilsTest, ArtApexDataPath) {
ScopedUnsetEnvironmentVariable no_env("ART_APEX_DATA");
EXPECT_EQ(kArtApexDataDefaultPath, GetArtApexData());

setenv("ART_APEX_DATA", "/path/from/env", /* overwrite */ 1);
EXPECT_EQ("/path/from/env", GetArtApexData());
}

TEST_F(FileUtilsTest, GetApexDataOatFilename) {
ScopedUnsetEnvironmentVariable android_root("ANDROID_ROOT");
ScopedUnsetEnvironmentVariable i18n_root("ANDROID_I18N_ROOT");
Expand Down
22 changes: 10 additions & 12 deletions odrefresh/include/odrefresh/odrefresh.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,36 +22,34 @@
namespace art {
namespace odrefresh {

// Default directory to which artifacts are written. (Overridable via the --dalvik-cache command
// line argument.)
static constexpr const char* kOdrefreshArtifactDirectory =
"/data/misc/apexdata/com.android.art/dalvik-cache";

//
// Exit codes from the odrefresh process (in addition to standard exit codes in sysexits.h).
//
// NB if odrefresh crashes, then the caller should not sign any artifacts and should remove any
// unsigned artifacts under `kOdrefreshArtifactDirectory`.
// unsigned artifacts under the output artifact directory.
//
// The output artifact directory is `$ART_APEX_DATA/dalvik-cache` by default, and is overridable via
// the --dalvik-cache command argument.
//
enum ExitCode : int {
// No compilation required, all artifacts look good or there is insufficient space to compile.
// For ART APEX in the system image, there may be no artifacts present under
// `kOdrefreshArtifactDirectory`.
// For ART APEX in the system image, there may be no artifacts present under the output artifact
// directory.
kOkay = EX_OK,

// Compilation required (only returned for --check). Re-run program with --compile on the
// command-line to generate + new artifacts under `kOdrefreshArtifactDirectory`.
// command-line to generate + new artifacts under the output artifact directory.
kCompilationRequired = EX__MAX + 1,

// New artifacts successfully generated under `kOdrefreshArtifactDirectory`.
// New artifacts successfully generated under the output artifact directory.
kCompilationSuccess = EX__MAX + 2,

// Compilation failed. Any artifacts under `kOdrefreshArtifactDirectory` are valid and should not
// Compilation failed. Any artifacts under the output artifact directory are valid and should not
// be removed. This may happen, for example, if compilation of boot extensions succeeds, but the
// compilation of the system_server jars fails due to lack of storage space.
kCompilationFailed = EX__MAX + 3,

// Removal of existing artifacts (or files under `kOdrefreshArtifactDirectory`) failed. Artifacts
// Removal of existing artifacts (or files under the output artifact directory) failed. Artifacts
// should be treated as invalid and should be removed if possible.
kCleanupFailed = EX__MAX + 4,

Expand Down
16 changes: 10 additions & 6 deletions odrefresh/odr_artifacts_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@
namespace art {
namespace odrefresh {

static constexpr const char* kOdrefreshArtifactDirectory = "/test/dir";

TEST(OdrArtifactsTest, ForBootImageExtension) {
ScopedUnsetEnvironmentVariable no_env("ART_APEX_DATA");
setenv("ART_APEX_DATA", kOdrefreshArtifactDirectory, /* overwrite */ 1);

const std::string image_location = GetApexDataBootImage("/system/framework/framework.jar");
EXPECT_TRUE(StartsWith(image_location, GetArtApexData()));
Expand All @@ -35,16 +38,17 @@ TEST(OdrArtifactsTest, ForBootImageExtension) {
GetSystemImageFilename(image_location.c_str(), InstructionSet::kArm64);

const auto artifacts = OdrArtifacts::ForBootImageExtension(image_filename);
CHECK_EQ(std::string(kOdrefreshArtifactDirectory) + "/arm64/boot-framework.art",
CHECK_EQ(std::string(kOdrefreshArtifactDirectory) + "/dalvik-cache/arm64/boot-framework.art",
artifacts.ImagePath());
CHECK_EQ(std::string(kOdrefreshArtifactDirectory) + "/arm64/boot-framework.oat",
CHECK_EQ(std::string(kOdrefreshArtifactDirectory) + "/dalvik-cache/arm64/boot-framework.oat",
artifacts.OatPath());
CHECK_EQ(std::string(kOdrefreshArtifactDirectory) + "/arm64/boot-framework.vdex",
CHECK_EQ(std::string(kOdrefreshArtifactDirectory) + "/dalvik-cache/arm64/boot-framework.vdex",
artifacts.VdexPath());
}

TEST(OdrArtifactsTest, ForSystemServer) {
ScopedUnsetEnvironmentVariable no_env("ART_APEX_DATA");
setenv("ART_APEX_DATA", kOdrefreshArtifactDirectory, /* overwrite */ 1);

const std::string image_location = GetApexDataImage("/system/framework/services.jar");
EXPECT_TRUE(StartsWith(image_location, GetArtApexData()));
Expand All @@ -53,13 +57,13 @@ TEST(OdrArtifactsTest, ForSystemServer) {
GetSystemImageFilename(image_location.c_str(), InstructionSet::kX86);
const auto artifacts = OdrArtifacts::ForSystemServer(image_filename);
CHECK_EQ(
std::string(kOdrefreshArtifactDirectory) + "/x86/system@[email protected]@classes.art",
std::string(kOdrefreshArtifactDirectory) + "/dalvik-cache/x86/system@[email protected]@classes.art",
artifacts.ImagePath());
CHECK_EQ(
std::string(kOdrefreshArtifactDirectory) + "/x86/system@[email protected]@classes.odex",
std::string(kOdrefreshArtifactDirectory) + "/dalvik-cache/x86/system@[email protected]@classes.odex",
artifacts.OatPath());
CHECK_EQ(
std::string(kOdrefreshArtifactDirectory) + "/x86/system@[email protected]@classes.vdex",
std::string(kOdrefreshArtifactDirectory) + "/dalvik-cache/x86/system@[email protected]@classes.vdex",
artifacts.VdexPath());
}

Expand Down
4 changes: 3 additions & 1 deletion odrefresh/odr_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@

#include "android-base/file.h"
#include "arch/instruction_set.h"
#include "base/file_utils.h"
#include "base/globals.h"
#include "log/log.h"
#include "odr_common.h"
#include "odrefresh/odrefresh.h"

namespace art {
Expand Down Expand Up @@ -79,7 +81,7 @@ class OdrConfig final {
: dry_run_(false),
isa_(InstructionSet::kNone),
program_name_(android::base::Basename(program_name)),
artifact_dir_(kOdrefreshArtifactDirectory) {
artifact_dir_(GetApexDataDalvikCacheDirectory(InstructionSet::kNone)) {
}

const std::string& GetApexInfoListFile() const { return apex_info_list_file_; }
Expand Down
2 changes: 1 addition & 1 deletion odrefresh/odrefresh.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class OnDeviceRefresh final {
// Gets the `ApexInfo` for active APEXes.
std::optional<std::vector<com::android::apex::ApexInfo>> GetApexInfoList() const;

// Reads the ART APEX cache information (if any) found in `kOdrefreshArtifactDirectory`.
// Reads the ART APEX cache information (if any) found in the output artifact directory.
std::optional<com::android::art::CacheInfo> ReadCacheInfo() const;

// Write ART APEX cache information to `kOnDeviceRefreshOdrefreshArtifactDirectory`.
Expand Down
2 changes: 1 addition & 1 deletion odrefresh/odrefresh_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ int InitializeTargetConfig(int argc, char** argv, OdrConfig* config) {
} else if (ArgumentMatches(arg, "--dalvik-cache=", &value)) {
art::OverrideDalvikCacheSubDirectory(value);
config->SetArtifactDirectory(Concatenate(
{android::base::Dirname(art::odrefresh::kOdrefreshArtifactDirectory), "/", value}));
{GetApexDataDalvikCacheDirectory(art::InstructionSet::kNone), "/", value}));
} else if (ArgumentMatches(arg, "--max-execution-seconds=", &value)) {
int seconds;
if (!android::base::ParseInt(value, &seconds)) {
Expand Down
7 changes: 0 additions & 7 deletions odrefresh/odrefresh_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -223,13 +223,6 @@ class OdRefreshTest : public CommonArtTest {
std::string boot_profile_file_;
};

TEST_F(OdRefreshTest, OdrefreshArtifactDirectory) {
// odrefresh.h defines kOdrefreshArtifactDirectory for external callers of odrefresh. This is
// where compilation artifacts end up.
ScopedUnsetEnvironmentVariable no_env("ART_APEX_DATA");
EXPECT_EQ(kOdrefreshArtifactDirectory, GetArtApexData() + "/dalvik-cache");
}

TEST_F(OdRefreshTest, AllSystemServerJars) {
auto [odrefresh, mock_odr_dexopt] = CreateOdRefresh();

Expand Down

0 comments on commit fb00761

Please sign in to comment.