Small clean ups for issues raised in reviews of fastdeploy
Removed support for -f shortcut flag as this conflicts with some package manager flags
Renamed use_localagent global to match conventions
Fixed case where tmp files were created unnecessarily
Removed dead code (delete_host_file)
Cleaned up multiple layers of error handling by using fatal() as soon as error conditions occur
Fix: 113631900
Test: mm
Test: adb install -r --fastdeploy --nostreaming --force-agent --local-agent ~/example_apks/example.apk
Test: adb install -r --fastdeploy --no-streaming --force-agent --local-agent ~/example_apks/example.apk
Test: observe that fast deploy works as usual
Test: adb install -r -f --nostreaming --force-agent --local-agent ~/example_apks/example.apk
Test: adb install -r -f --no-streaming --force-agent --local-agent ~/example_apks/example.apk
Test: observe that fast deploy is no longer invoked by -f
Change-Id: Ic719df1003ac319e48b32f7f377f6f91d17f6a6f
(cherry picked from commit 0584689beaff604ceeccaf706dc368213d07b977)
diff --git a/adb/client/adb_install.cpp b/adb/client/adb_install.cpp
index c0d09fd..c00d955 100644
--- a/adb/client/adb_install.cpp
+++ b/adb/client/adb_install.cpp
@@ -177,7 +177,9 @@
}
cleanup_streamed_apk:
- delete_device_patch_file(file);
+ if (use_fastdeploy == true) {
+ delete_device_patch_file(file);
+ }
return result;
} else {
struct stat sb;
@@ -258,10 +260,10 @@
std::string apk_dest =
android::base::StringPrintf(where, android::base::Basename(argv[last_apk]).c_str());
- TemporaryFile metadataTmpFile;
- TemporaryFile patchTmpFile;
-
if (use_fastdeploy == true) {
+ TemporaryFile metadataTmpFile;
+ TemporaryFile patchTmpFile;
+
FILE* metadataFile = fopen(metadataTmpFile.path, "wb");
int metadata_len = extract_metadata(apk_file[0], metadataFile);
fclose(metadataFile);
@@ -294,7 +296,9 @@
result = pm_command(argc, argv);
cleanup_apk:
- delete_device_patch_file(apk_file[0]);
+ if (use_fastdeploy == true) {
+ delete_device_patch_file(apk_file[0]);
+ }
delete_device_file(apk_dest);
return result;
}
@@ -328,9 +332,6 @@
} else if (!strcmp(argv[i], "--no-fastdeploy")) {
processedArgIndicies.push_back(i);
use_fastdeploy = false;
- } else if (!strcmp(argv[i], "-f")) {
- processedArgIndicies.push_back(i);
- use_fastdeploy = true;
} else if (!strcmp(argv[i], "--force-agent")) {
processedArgIndicies.push_back(i);
agent_update_strategy = FastDeploy_AgentUpdateAlways;
@@ -383,12 +384,7 @@
if (use_fastdeploy == true) {
fastdeploy_set_local_agent(use_localagent);
-
- bool agent_up_to_date = update_agent(agent_update_strategy);
- if (agent_up_to_date == false) {
- printf("Failed to update agent, exiting\n");
- return 1;
- }
+ update_agent(agent_update_strategy);
}
switch (installMode) {
@@ -533,19 +529,3 @@
std::string cmd = "rm -f " + escape_arg(filename);
return send_shell_command(cmd);
}
-
-int delete_host_file(const std::string& filename) {
-#ifdef _WIN32
- BOOL delete_return = DeleteFileA(filename.c_str());
- if (delete_return != 0) {
- return 0;
- } else {
- DWORD last_error = GetLastError();
- printf("Error [%ld] deleting: %s\n", last_error, filename.c_str());
- return delete_return;
- }
-#else
- std::string cmd = "rm -f " + escape_arg(filename);
- return system(cmd.c_str());
-#endif
-}
diff --git a/adb/client/commandline.cpp b/adb/client/commandline.cpp
index a5cfd7f..6e143c1 100644
--- a/adb/client/commandline.cpp
+++ b/adb/client/commandline.cpp
@@ -158,7 +158,7 @@
" --instant: cause the app to be installed as an ephemeral install app\n"
" --no-streaming: always push APK to device and invoke Package Manager as separate steps\n"
" --streaming: force streaming APK directly into Package Manager\n"
- " -f/--fastdeploy: use fast deploy (only valid with -r)\n"
+ " --fastdeploy: use fast deploy (only valid with -r)\n"
" --no-fastdeploy: prevent use of fast deploy (only valid with -r)\n"
" --force-agent: force update of deployment agent when using fast deploy\n"
" --date-check-agent: update deployment agent when local version is newer and using fast deploy\n"
diff --git a/adb/client/fastdeploy.cpp b/adb/client/fastdeploy.cpp
index 2914836..d3f35c8 100644
--- a/adb/client/fastdeploy.cpp
+++ b/adb/client/fastdeploy.cpp
@@ -30,7 +30,7 @@
static constexpr const char* kDeviceAgentPath = "/data/local/tmp/";
-static bool use_localagent = false;
+static bool g_use_localagent = false;
long get_agent_version() {
std::vector<char> versionOutputBuffer;
@@ -61,48 +61,36 @@
return api_level;
}
-void fastdeploy_set_local_agent(bool set_use_localagent) {
- use_localagent = set_use_localagent;
+void fastdeploy_set_local_agent(bool use_localagent) {
+ g_use_localagent = use_localagent;
}
// local_path - must start with a '/' and be relative to $ANDROID_PRODUCT_OUT
-static bool get_agent_component_host_path(const char* local_path, const char* sdk_path,
- std::string* output_path) {
+static std::string get_agent_component_host_path(const char* local_path, const char* sdk_path) {
std::string adb_dir = android::base::GetExecutableDirectory();
if (adb_dir.empty()) {
- return false;
+ fatal("Could not determine location of adb!");
}
- if (use_localagent) {
+ if (g_use_localagent) {
const char* product_out = getenv("ANDROID_PRODUCT_OUT");
if (product_out == nullptr) {
- return false;
+ fatal("Could not locate %s because $ANDROID_PRODUCT_OUT is not defined", local_path);
}
- *output_path = android::base::StringPrintf("%s%s", product_out, local_path);
- return true;
+ return android::base::StringPrintf("%s%s", product_out, local_path);
} else {
- *output_path = adb_dir + sdk_path;
- return true;
+ return adb_dir + sdk_path;
}
- return false;
}
static bool deploy_agent(bool checkTimeStamps) {
std::vector<const char*> srcs;
- std::string agent_jar_path;
- if (get_agent_component_host_path("/system/framework/deployagent.jar", "/deployagent.jar",
- &agent_jar_path)) {
- srcs.push_back(agent_jar_path.c_str());
- } else {
- return false;
- }
-
- std::string agent_sh_path;
- if (get_agent_component_host_path("/system/bin/deployagent", "/deployagent", &agent_sh_path)) {
- srcs.push_back(agent_sh_path.c_str());
- } else {
- return false;
- }
+ std::string jar_path =
+ get_agent_component_host_path("/system/framework/deployagent.jar", "/deployagent.jar");
+ std::string script_path =
+ get_agent_component_host_path("/system/bin/deployagent", "/deployagent");
+ srcs.push_back(jar_path.c_str());
+ srcs.push_back(script_path.c_str());
if (do_sync_push(srcs, kDeviceAgentPath, checkTimeStamps)) {
// on windows the shell script might have lost execute permission
@@ -111,24 +99,24 @@
std::string chmodCommand =
android::base::StringPrintf(kChmodCommandPattern, kDeviceAgentPath);
int ret = send_shell_command(chmodCommand);
- return (ret == 0);
+ if (ret != 0) {
+ fatal("Error executing %s returncode: %d", chmodCommand.c_str(), ret);
+ }
} else {
- return false;
+ fatal("Error pushing agent files to device");
}
+
+ return true;
}
-bool update_agent(FastDeploy_AgentUpdateStrategy agentUpdateStrategy) {
+void update_agent(FastDeploy_AgentUpdateStrategy agentUpdateStrategy) {
long agent_version = get_agent_version();
switch (agentUpdateStrategy) {
case FastDeploy_AgentUpdateAlways:
- if (deploy_agent(false) == false) {
- return false;
- }
+ deploy_agent(false);
break;
case FastDeploy_AgentUpdateNewerTimeStamp:
- if (deploy_agent(true) == false) {
- return false;
- }
+ deploy_agent(true);
break;
case FastDeploy_AgentUpdateDifferentVersion:
if (agent_version != kRequiredAgentVersion) {
@@ -138,19 +126,20 @@
printf("Device agent version is (%ld), (%ld) is required, re-deploying\n",
agent_version, kRequiredAgentVersion);
}
- if (deploy_agent(false) == false) {
- return false;
- }
+ deploy_agent(false);
}
break;
}
agent_version = get_agent_version();
- return (agent_version == kRequiredAgentVersion);
+ if (agent_version != kRequiredAgentVersion) {
+ fatal("After update agent version remains incorrect! Expected %ld but version is %ld",
+ kRequiredAgentVersion, agent_version);
+ }
}
static std::string get_aapt2_path() {
- if (use_localagent) {
+ if (g_use_localagent) {
// This should never happen on a Windows machine
const char* host_out = getenv("ANDROID_HOST_OUT");
if (host_out == nullptr) {
@@ -186,26 +175,24 @@
}
// output is required to point to a valid output string (non-null)
-static bool get_packagename_from_apk(const char* apkPath, std::string* output) {
+static std::string get_packagename_from_apk(const char* apkPath) {
const char* kAapt2DumpNameCommandPattern = R"(%s dump packagename "%s")";
std::string aapt2_path_string = get_aapt2_path();
std::string getPackagenameCommand = android::base::StringPrintf(
kAapt2DumpNameCommandPattern, aapt2_path_string.c_str(), apkPath);
- if (system_capture(getPackagenameCommand.c_str(), *output) == 0) {
- // strip any line end characters from the output
- *output = android::base::Trim(*output);
- return true;
+ std::string package_name;
+ int exit_code = system_capture(getPackagenameCommand.c_str(), package_name);
+ if (exit_code != 0) {
+ fatal("Error executing '%s' exitcode: %d", getPackagenameCommand.c_str(), exit_code);
}
- return false;
+
+ // strip any line end characters from the output
+ return android::base::Trim(package_name);
}
int extract_metadata(const char* apkPath, FILE* outputFp) {
- std::string packageName;
- if (get_packagename_from_apk(apkPath, &packageName) == false) {
- return -1;
- }
-
+ std::string packageName = get_packagename_from_apk(apkPath);
const char* kAgentExtractCommandPattern = "/data/local/tmp/deployagent extract %s";
std::string extractCommand =
android::base::StringPrintf(kAgentExtractCommandPattern, packageName.c_str());
@@ -223,7 +210,7 @@
}
static std::string get_patch_generator_command() {
- if (use_localagent) {
+ if (g_use_localagent) {
// This should never happen on a Windows machine
const char* host_out = getenv("ANDROID_HOST_OUT");
if (host_out == nullptr) {
@@ -250,11 +237,7 @@
}
std::string get_patch_path(const char* apkPath) {
- std::string packageName;
- if (get_packagename_from_apk(apkPath, &packageName) == false) {
- return "";
- }
-
+ std::string packageName = get_packagename_from_apk(apkPath);
std::string patchDevicePath =
android::base::StringPrintf("%s%s.patch", kDeviceAgentPath, packageName.c_str());
return patchDevicePath;
@@ -262,12 +245,7 @@
int apply_patch_on_device(const char* apkPath, const char* patchPath, const char* outputPath) {
const std::string kAgentApplyCommandPattern = "/data/local/tmp/deployagent apply %s %s -o %s";
-
- std::string packageName;
- if (get_packagename_from_apk(apkPath, &packageName) == false) {
- return -1;
- }
-
+ std::string packageName = get_packagename_from_apk(apkPath);
std::string patchDevicePath = get_patch_path(apkPath);
std::vector<const char*> srcs = {patchPath};
@@ -286,12 +264,7 @@
int install_patch(const char* apkPath, const char* patchPath, int argc, const char** argv) {
const std::string kAgentApplyCommandPattern = "/data/local/tmp/deployagent apply %s %s -pm %s";
-
- std::string packageName;
- if (get_packagename_from_apk(apkPath, &packageName) == false) {
- return -1;
- }
-
+ std::string packageName = get_packagename_from_apk(apkPath);
std::vector<const char*> srcs;
std::string patchDevicePath =
android::base::StringPrintf("%s%s.patch", kDeviceAgentPath, packageName.c_str());
diff --git a/adb/client/fastdeploy.h b/adb/client/fastdeploy.h
index 80f3875..e5e7663 100644
--- a/adb/client/fastdeploy.h
+++ b/adb/client/fastdeploy.h
@@ -26,7 +26,7 @@
void fastdeploy_set_local_agent(bool use_localagent);
int get_device_api_level();
-bool update_agent(FastDeploy_AgentUpdateStrategy agentUpdateStrategy);
+void update_agent(FastDeploy_AgentUpdateStrategy agentUpdateStrategy);
int extract_metadata(const char* apkPath, FILE* outputFp);
int create_patch(const char* apkPath, const char* metadataPath, const char* patchPath);
int apply_patch_on_device(const char* apkPath, const char* patchPath, const char* outputPath);