diff --git a/adb/Android.mk b/adb/Android.mk index 27b8a0006..695cbe08c 100644 --- a/adb/Android.mk +++ b/adb/Android.mk @@ -183,6 +183,7 @@ LOCAL_SRC_FILES := \ adb_client.cpp \ bugreport.cpp \ bugreport_test.cpp \ + line_printer.cpp \ services.cpp \ shell_service_protocol.cpp \ shell_service_protocol_test.cpp \ diff --git a/adb/bugreport.cpp b/adb/bugreport.cpp index e267f0f87..8a3eb0825 100644 --- a/adb/bugreport.cpp +++ b/adb/bugreport.cpp @@ -23,55 +23,92 @@ static constexpr char BUGZ_OK_PREFIX[] = "OK:"; static constexpr char BUGZ_FAIL_PREFIX[] = "FAIL:"; +static constexpr char BUGZ_PROGRESS_PREFIX[] = "PROGRESS:"; +static constexpr char BUGZ_PROGRESS_SEPARATOR[] = "/"; // Custom callback used to handle the output of zipped bugreports. class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface { public: - BugreportStandardStreamsCallback(const std::string& dest_file, Bugreport* br) - : br_(br), dest_file_(dest_file), stdout_str_() { + BugreportStandardStreamsCallback(const std::string& dest_file, bool show_progress, Bugreport* br) + : br_(br), dest_file_(dest_file), show_progress_(show_progress), status_(-1), line_() { } void OnStdout(const char* buffer, int length) { - std::string output; - OnStream(&output, stdout, buffer, length); - stdout_str_.append(output); + for (int i = 0; i < length; i++) { + char c = buffer[i]; + if (c == '\n') { + ProcessLine(line_); + line_.clear(); + } else { + line_.append(1, c); + } + } } void OnStderr(const char* buffer, int length) { OnStream(nullptr, stderr, buffer, length); } - int Done(int unused_) { - int status = -1; - std::string output = android::base::Trim(stdout_str_); - if (android::base::StartsWith(output, BUGZ_OK_PREFIX)) { - const char* zip_file = &output[strlen(BUGZ_OK_PREFIX)]; - std::vector srcs{zip_file}; - status = br_->DoSyncPull(srcs, dest_file_.c_str(), true, dest_file_.c_str()) ? 0 : 1; - if (status != 0) { - fprintf(stderr, "Could not copy file '%s' to '%s'\n", zip_file, dest_file_.c_str()); - } - } else if (android::base::StartsWith(output, BUGZ_FAIL_PREFIX)) { - const char* error_message = &output[strlen(BUGZ_FAIL_PREFIX)]; - fprintf(stderr, "Device failed to take a zipped bugreport: %s\n", error_message); - } else { - fprintf(stderr, - "Unexpected string (%s) returned by bugreportz, " - "device probably does not support it\n", - output.c_str()); - } - - return status; + // Process remaining line, if any... + ProcessLine(line_); + // ..then return. + return status_; } private: + void ProcessLine(const std::string& line) { + if (line.empty()) return; + + if (android::base::StartsWith(line, BUGZ_OK_PREFIX)) { + if (show_progress_) { + // Make sure pull message doesn't conflict with generation message. + br_->UpdateProgress(dest_file_, 100, 100, true); + } + + const char* zip_file = &line[strlen(BUGZ_OK_PREFIX)]; + std::vector srcs{zip_file}; + status_ = br_->DoSyncPull(srcs, dest_file_.c_str(), true, dest_file_.c_str()) ? 0 : 1; + if (status_ != 0) { + fprintf(stderr, "Could not copy file '%s' to '%s'\n", zip_file, dest_file_.c_str()); + } + } else if (android::base::StartsWith(line, BUGZ_FAIL_PREFIX)) { + const char* error_message = &line[strlen(BUGZ_FAIL_PREFIX)]; + fprintf(stderr, "Device failed to take a zipped bugreport: %s\n", error_message); + status_ = -1; + } else if (show_progress_ && android::base::StartsWith(line, BUGZ_PROGRESS_PREFIX)) { + // progress_line should have the following format: + // + // BUGZ_PROGRESS_PREFIX:PROGRESS/TOTAL + // + size_t idx1 = line.rfind(BUGZ_PROGRESS_PREFIX) + strlen(BUGZ_PROGRESS_PREFIX); + size_t idx2 = line.rfind(BUGZ_PROGRESS_SEPARATOR); + int progress = std::stoi(line.substr(idx1, (idx2 - idx1))); + int total = std::stoi(line.substr(idx2 + 1)); + br_->UpdateProgress(dest_file_, progress, total); + } else { + fprintf(stderr, + "WARNING: unexpected line (%s) returned by bugreportz, " + "device probably does not support zipped bugreports.\n" + "Try 'adb bugreport' instead.", + line.c_str()); + } + } + Bugreport* br_; const std::string dest_file_; - std::string stdout_str_; + bool show_progress_; + int status_; + + // Temporary buffer containing the characters read since the last newline + // (\n). + std::string line_; DISALLOW_COPY_AND_ASSIGN(BugreportStandardStreamsCallback); }; +// Implemented in commandline.cpp +int usage(); + int Bugreport::DoIt(TransportType transport_type, const char* serial, int argc, const char** argv) { if (argc == 1) return SendShellCommand(transport_type, serial, "bugreport", false); if (argc != 2) return usage(); @@ -84,12 +121,50 @@ int Bugreport::DoIt(TransportType transport_type, const char* serial, int argc, // TODO: use a case-insensitive comparison (like EndsWithIgnoreCase dest_file += ".zip"; } - fprintf(stderr, - "Bugreport is in progress and it could take minutes to complete.\n" - "Please be patient and do not cancel or disconnect your device until " - "it completes.\n"); - BugreportStandardStreamsCallback bugz_callback(dest_file, this); - return SendShellCommand(transport_type, serial, "bugreportz", false, &bugz_callback); + + // Gets bugreportz version. + std::string bugz_stderr; + DefaultStandardStreamsCallback version_callback(nullptr, &bugz_stderr); + int status = SendShellCommand(transport_type, serial, "bugreportz -v", false, &version_callback); + + if (status != 0) { + fprintf(stderr, + "Failed to get bugreportz version: 'bugreport -v' returned '%s' " + "(code %d)." + "\nIf the device does not support it, try running 'adb bugreport' " + "to get a " + "flat-file bugreport.", + bugz_stderr.c_str(), status); + return status; + } + std::string bugz_version = android::base::Trim(bugz_stderr); + + bool show_progress = true; + std::string bugz_command = "bugreportz -p"; + if (bugz_version == "1.0") { + // 1.0 does not support progress notifications, so print a disclaimer + // message instead. + fprintf(stderr, + "Bugreport is in progress and it could take minutes to complete.\n" + "Please be patient and do not cancel or disconnect your device " + "until it completes." + "\n"); + show_progress = false; + bugz_command = "bugreportz"; + } + BugreportStandardStreamsCallback bugz_callback(dest_file, show_progress, this); + return SendShellCommand(transport_type, serial, bugz_command, false, &bugz_callback); +} + +void Bugreport::UpdateProgress(const std::string& file_name, int progress, int total, + bool keep_info_line) { + int progress_percentage = (progress * 100 / total); + line_printer_.Print(android::base::StringPrintf("[%3d%%] generating %s", progress_percentage, + file_name.c_str()), + LinePrinter::INFO); + if (keep_info_line) { + line_printer_.KeepInfoLine(); + } } int Bugreport::SendShellCommand(TransportType transport_type, const char* serial, diff --git a/adb/bugreport.h b/adb/bugreport.h index 1c39a9f23..fd11a4a2b 100644 --- a/adb/bugreport.h +++ b/adb/bugreport.h @@ -21,12 +21,13 @@ #include "adb.h" #include "commandline.h" +#include "line_printer.h" class Bugreport { friend class BugreportStandardStreamsCallback; public: - Bugreport() { + Bugreport() : line_printer_() { } int DoIt(TransportType transport_type, const char* serial, int argc, const char** argv); @@ -42,6 +43,9 @@ class Bugreport { const char* name); private: + virtual void UpdateProgress(const std::string& file_name, int progress, int total, + bool keep_info_line = false); + LinePrinter line_printer_; DISALLOW_COPY_AND_ASSIGN(Bugreport); }; diff --git a/adb/bugreport_test.cpp b/adb/bugreport_test.cpp index 15c6f83f2..e695b2431 100644 --- a/adb/bugreport_test.cpp +++ b/adb/bugreport_test.cpp @@ -51,37 +51,63 @@ int send_shell_command(TransportType transport_type, const char* serial, const s return -42; } +enum StreamType { + kStreamStdout, + kStreamStderr, +}; + // gmock black magic to provide a WithArg<4>(WriteOnStdout(output)) matcher typedef void OnStandardStreamsCallbackFunction(StandardStreamsCallbackInterface*); class OnStandardStreamsCallbackAction : public ActionInterface { public: - explicit OnStandardStreamsCallbackAction(const std::string& output) : output_(output) { + explicit OnStandardStreamsCallbackAction(StreamType type, const std::string& output) + : type_(type), output_(output) { } virtual Result Perform(const ArgumentTuple& args) { - ::std::tr1::get<0>(args)->OnStdout(output_.c_str(), output_.size()); + if (type_ == kStreamStdout) { + ::std::tr1::get<0>(args)->OnStdout(output_.c_str(), output_.size()); + } + if (type_ == kStreamStderr) { + ::std::tr1::get<0>(args)->OnStderr(output_.c_str(), output_.size()); + } } private: + StreamType type_; std::string output_; }; +// Matcher used to emulated StandardStreamsCallbackInterface.OnStdout(buffer, +// length) Action WriteOnStdout(const std::string& output) { - return MakeAction(new OnStandardStreamsCallbackAction(output)); + return MakeAction(new OnStandardStreamsCallbackAction(kStreamStdout, output)); +} + +// Matcher used to emulated StandardStreamsCallbackInterface.OnStderr(buffer, +// length) +Action WriteOnStderr(const std::string& output) { + return MakeAction(new OnStandardStreamsCallbackAction(kStreamStderr, output)); } typedef int CallbackDoneFunction(StandardStreamsCallbackInterface*); class CallbackDoneAction : public ActionInterface { public: + explicit CallbackDoneAction(int status) : status_(status) { + } virtual Result Perform(const ArgumentTuple& args) { - int status = ::std::tr1::get<0>(args)->Done(123); // Value passed does not matter + int status = ::std::tr1::get<0>(args)->Done(status_); return status; } + + private: + int status_; }; -Action ReturnCallbackDone() { - return MakeAction(new CallbackDoneAction()); +// Matcher used to emulated StandardStreamsCallbackInterface.Done(status) +Action ReturnCallbackDone(int status = -1337) { + return MakeAction(new CallbackDoneAction(status)); } class BugreportMock : public Bugreport { @@ -91,10 +117,22 @@ class BugreportMock : public Bugreport { bool disable_shell_protocol, StandardStreamsCallbackInterface* callback)); MOCK_METHOD4(DoSyncPull, bool(const std::vector& srcs, const char* dst, bool copy_attrs, const char* name)); + MOCK_METHOD4(UpdateProgress, void(const std::string&, int, int, bool)); }; class BugreportTest : public ::testing::Test { public: + void SetBugreportzVersion(const std::string& version) { + EXPECT_CALL(br_, + SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -v", false, _)) + .WillOnce(DoAll(WithArg<4>(WriteOnStderr(version.c_str())), + WithArg<4>(ReturnCallbackDone(0)))); + } + + void ExpectProgress(int progress, int total, bool keep_info_line = false) { + EXPECT_CALL(br_, UpdateProgress(StrEq("file.zip"), progress, total, keep_info_line)); + } + BugreportMock br_; }; @@ -113,8 +151,10 @@ TEST_F(BugreportTest, FlatFileFormat) { ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 1, args)); } -// Tests 'adb bugreport file.zip' when it succeeds -TEST_F(BugreportTest, Ok) { +// Tests 'adb bugreport file.zip' when it succeeds and device does not support +// progress. +TEST_F(BugreportTest, OkLegacy) { + SetBugreportzVersion("1.0"); EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _)) .WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip")), WithArg<4>(ReturnCallbackDone()))); @@ -126,22 +166,10 @@ TEST_F(BugreportTest, Ok) { ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); } -// Tests 'adb bugreport file' when it succeeds -TEST_F(BugreportTest, OkNoExtension) { - EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _)) - .WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip")), - WithArg<4>(ReturnCallbackDone()))); - EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/bugreport.zip")), StrEq("file.zip"), - true, StrEq("file.zip"))) - .WillOnce(Return(true)); - - const char* args[1024] = {"bugreport", "file"}; - ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); -} - // Tests 'adb bugreport file.zip' when it succeeds but response was sent in -// multiple buffer writers. -TEST_F(BugreportTest, OkSplitBuffer) { +// multiple buffer writers and without progress updates. +TEST_F(BugreportTest, OkLegacySplitBuffer) { + SetBugreportzVersion("1.0"); EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _)) .WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device")), WithArg<4>(WriteOnStdout("/bugreport.zip")), @@ -154,35 +182,87 @@ TEST_F(BugreportTest, OkSplitBuffer) { ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); } +// Tests 'adb bugreport file.zip' when it succeeds and displays progress. +TEST_F(BugreportTest, Ok) { + SetBugreportzVersion("1.1"); + ExpectProgress(1, 100); + ExpectProgress(10, 100); + ExpectProgress(50, 100); + ExpectProgress(99, 100); + ExpectProgress(100, 100, true); + // clang-format off + EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _)) + .WillOnce(DoAll( + // Progress line in one write + WithArg<4>(WriteOnStdout("PROGRESS:1/100\n")), + // Add some bogus lines + WithArg<4>(WriteOnStdout("\nDUDE:SWEET\n\n")), + // Multiple progress lines in one write + WithArg<4>(WriteOnStdout("PROGRESS:10/100\nPROGRESS:50/100\n")), + // Progress line in multiple writes + WithArg<4>(WriteOnStdout("PROG")), + WithArg<4>(WriteOnStdout("RESS:99")), + WithArg<4>(WriteOnStdout("/100\n")), + // Split last message as well, just in case + WithArg<4>(WriteOnStdout("OK:/device/bugreport")), + WithArg<4>(WriteOnStdout(".zip")), + WithArg<4>(ReturnCallbackDone()))); + EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/bugreport.zip")), StrEq("file.zip"), + true, StrEq("file.zip"))) + .WillOnce(Return(true)); + + const char* args[1024] = {"bugreport", "file.zip"}; + ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); +} + +// Tests 'adb bugreport file' when it succeeds +TEST_F(BugreportTest, OkNoExtension) { + SetBugreportzVersion("1.1"); + ExpectProgress(100, 100, true); + EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _)) + .WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip\n")), + WithArg<4>(ReturnCallbackDone()))); + EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/bugreport.zip")), StrEq("file.zip"), + true, StrEq("file.zip"))) + .WillOnce(Return(true)); + + const char* args[1024] = {"bugreport", "file"}; + ASSERT_EQ(0, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); +} + // Tests 'adb bugreport file.zip' when the bugreport itself failed TEST_F(BugreportTest, BugreportzReturnedFail) { - EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _)) - .WillOnce(DoAll(WithArg<4>(WriteOnStdout("FAIL:D'OH!")), WithArg<4>(ReturnCallbackDone()))); + SetBugreportzVersion("1.1"); + EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _)) + .WillOnce( + DoAll(WithArg<4>(WriteOnStdout("FAIL:D'OH!\n")), WithArg<4>(ReturnCallbackDone()))); CaptureStderr(); const char* args[1024] = {"bugreport", "file.zip"}; ASSERT_EQ(-1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); - ASSERT_THAT(GetCapturedStderr(), HasSubstr("D'OH")); + ASSERT_THAT(GetCapturedStderr(), HasSubstr("D'OH!")); } // Tests 'adb bugreport file.zip' when the bugreport itself failed but response // was sent in // multiple buffer writes TEST_F(BugreportTest, BugreportzReturnedFailSplitBuffer) { - EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _)) - .WillOnce(DoAll(WithArg<4>(WriteOnStdout("FAIL")), WithArg<4>(WriteOnStdout(":D'OH!")), + SetBugreportzVersion("1.1"); + EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _)) + .WillOnce(DoAll(WithArg<4>(WriteOnStdout("FAIL")), WithArg<4>(WriteOnStdout(":D'OH!\n")), WithArg<4>(ReturnCallbackDone()))); CaptureStderr(); const char* args[1024] = {"bugreport", "file.zip"}; ASSERT_EQ(-1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); - ASSERT_THAT(GetCapturedStderr(), HasSubstr("D'OH")); + ASSERT_THAT(GetCapturedStderr(), HasSubstr("D'OH!")); } // Tests 'adb bugreport file.zip' when the bugreportz returned an unsupported // response. TEST_F(BugreportTest, BugreportzReturnedUnsupported) { - EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _)) + SetBugreportzVersion("1.1"); + EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _)) .WillOnce(DoAll(WithArg<4>(WriteOnStdout("bugreportz? What am I, a zombie?")), WithArg<4>(ReturnCallbackDone()))); @@ -192,9 +272,19 @@ TEST_F(BugreportTest, BugreportzReturnedUnsupported) { ASSERT_THAT(GetCapturedStderr(), HasSubstr("bugreportz? What am I, a zombie?")); } -// Tests 'adb bugreport file.zip' when the bugreportz command fails +// Tests 'adb bugreport file.zip' when the bugreportz -v command failed +TEST_F(BugreportTest, BugreportzVersionFailed) { + EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -v", false, _)) + .WillOnce(Return(666)); + + const char* args[1024] = {"bugreport", "file.zip"}; + ASSERT_EQ(666, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args)); +} + +// Tests 'adb bugreport file.zip' when the main bugreportz command failed TEST_F(BugreportTest, BugreportzFailed) { - EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _)) + SetBugreportzVersion("1.1"); + EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _)) .WillOnce(Return(666)); const char* args[1024] = {"bugreport", "file.zip"}; @@ -203,7 +293,9 @@ TEST_F(BugreportTest, BugreportzFailed) { // Tests 'adb bugreport file.zip' when the bugreport could not be pulled TEST_F(BugreportTest, PullFails) { - EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz", false, _)) + SetBugreportzVersion("1.1"); + ExpectProgress(100, 100, true); + EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _)) .WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip")), WithArg<4>(ReturnCallbackDone()))); EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/bugreport.zip")), StrEq("file.zip"),