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

Commit 6e286968 authored by Felipe Leme's avatar Felipe Leme Committed by Josh Gao
Browse files

DO NOT MERGE: Minor improvements on bugreport generation. 

- Skipped artificial 100/100 message, since pulling will take care of
  the final 100% progress.
- Consolidated unsupported lines in just one message.
- Let user know the bugreport can still be recovered when it could not
  be copied to the destination directory.

BUG: 30451114

Change-Id: Icfce9c1e8e7ed407719728b9874679ac40b21eab
(cherry picked from commit 80a65d03)
(cherry picked from commit 5f6eacae)
parent daf4628f

adb/bugreport.cpp

+46 −29
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@ 
 */



#include <string>

#include <vector>



#include <android-base/strings.h>
@@ -31,10 +32,12 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface 
  public:

    BugreportStandardStreamsCallback(const std::string& dest_file, bool show_progress, Bugreport* br)

        : br_(br),

          src_file_(),

          dest_file_(dest_file),

          line_message_(android::base::StringPrintf("generating %s", dest_file_.c_str())),

          invalid_lines_(),

          show_progress_(show_progress),

          status_(-1),

          status_(0),

          line_() {

    }
@@ -54,9 +57,39 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface 
        OnStream(nullptr, stderr, buffer, length);

    }

    int Done(int unused_) {

        // Process remaining line, if any...

        // Process remaining line, if any.

        ProcessLine(line_);

        // ..then return.



        // Warn about invalid lines, if any.

        if (!invalid_lines_.empty()) {

            fprintf(stderr,

                    "WARNING: bugreportz generated %zu line(s) with unknown commands, "

                    "device might not support zipped bugreports:\n",

                    invalid_lines_.size());

            for (const auto& line : invalid_lines_) {

                fprintf(stderr, "\t%s\n", line.c_str());

            }

            fprintf(stderr,

                    "If the zipped bugreport was not generated, try 'adb bugreport' instead.\n");

        }



        // Pull the generated bug report.

        if (status_ == 0) {

            if (src_file_.empty()) {

                fprintf(stderr, "bugreportz did not return a '%s' or '%s' line\n", BUGZ_OK_PREFIX,

                        BUGZ_FAIL_PREFIX);

                return -1;

            }

            std::vector<const char*> srcs{src_file_.c_str()};

            status_ = br_->DoSyncPull(srcs, dest_file_.c_str(), true, line_message_.c_str()) ? 0 : 1;

            if (status_ != 0) {

                fprintf(stderr,

                        "Bug report finished but could not be copied to '%s'.\n"

                        "Try to run 'adb pull %s <directory>'\n"

                        "to copy it to a directory that can be written.\n",

                        dest_file_.c_str(), src_file_.c_str());

            }

        }

        return status_;

    }
@@ -65,17 +98,7 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface 
        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(line_message_, 100, 100);

            }



            const char* zip_file = &line[strlen(BUGZ_OK_PREFIX)];

            std::vector<const char*> srcs{zip_file};

            status_ = br_->DoSyncPull(srcs, dest_file_.c_str(), true, line_message_.c_str()) ? 0 : 1;

            if (status_ != 0) {

                fprintf(stderr, "Could not copy file '%s' to '%s'\n", zip_file, dest_file_.c_str());

            }

            src_file_ = &line[strlen(BUGZ_OK_PREFIX)];

        } 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);
@@ -91,17 +114,15 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface 
            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());

            invalid_lines_.push_back(line);

        }

    }



    Bugreport* br_;

    std::string src_file_;

    const std::string dest_file_;

    const std::string line_message_;

    std::vector<std::string> invalid_lines_;

    bool show_progress_;

    int status_;
@@ -132,18 +153,15 @@ int Bugreport::DoIt(TransportType transport_type, const char* serial, int argc, 
    std::string bugz_stderr;

    DefaultStandardStreamsCallback version_callback(nullptr, &bugz_stderr);

    int status = SendShellCommand(transport_type, serial, "bugreportz -v", false, &version_callback);

    std::string bugz_version = android::base::Trim(bugz_stderr);



    if (status != 0) {

    if (status != 0 || bugz_version.empty()) {

        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.",

                "Failed to get bugreportz version: 'bugreportz -v' returned '%s' (code %d).\n"

                "If the device runs Android M or below, try 'adb bugreport' instead.\n",

                bugz_stderr.c_str(), status);

        return status;

        return status != 0 ? status : -1;

    }

    std::string bugz_version = android::base::Trim(bugz_stderr);



    bool show_progress = true;

    std::string bugz_command = "bugreportz -p";
@@ -153,8 +171,7 @@ int Bugreport::DoIt(TransportType transport_type, const char* serial, int argc, 
        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");

                "until it completes.\n");

        show_progress = false;

        bugz_command = "bugreportz";

    }

adb/bugreport_test.cpp

+11 −4
Original line number Diff line number Diff line
@@ -189,14 +189,14 @@ TEST_F(BugreportTest, Ok) { 
    ExpectProgress(10, 100);

    ExpectProgress(50, 100);

    ExpectProgress(99, 100);

    ExpectProgress(100, 100);

    // clang-format off

    EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))

        // NOTE: DoAll accepts at most 10 arguments, and we have reached that limit...

        .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")),

            WithArg<4>(WriteOnStdout("\nDUDE:SWEET\n\nBLA\n\nBLA\nBLA\n\n")),

            // Multiple progress lines in one write

            WithArg<4>(WriteOnStdout("PROGRESS:10/100\nPROGRESS:50/100\n")),

            // Progress line in multiple writes
@@ -207,6 +207,7 @@ TEST_F(BugreportTest, Ok) { 
            WithArg<4>(WriteOnStdout("OK:/device/bugreport")),

            WithArg<4>(WriteOnStdout(".zip")),

            WithArg<4>(ReturnCallbackDone())));

    // clang-format on

    EXPECT_CALL(br_, DoSyncPull(ElementsAre(StrEq("/device/bugreport.zip")), StrEq("file.zip"),

                                true, HasSubstr("file.zip")))

        .WillOnce(Return(true));
@@ -218,7 +219,6 @@ TEST_F(BugreportTest, Ok) { 
// Tests 'adb bugreport file' when it succeeds

TEST_F(BugreportTest, OkNoExtension) {

    SetBugreportzVersion("1.1");

    ExpectProgress(100, 100);

    EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))

        .WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip\n")),

                        WithArg<4>(ReturnCallbackDone())));
@@ -281,6 +281,14 @@ TEST_F(BugreportTest, BugreportzVersionFailed) { 
    ASSERT_EQ(666, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));

}



// Tests 'adb bugreport file.zip' when the bugreportz -v returns status 0 but with no output.

TEST_F(BugreportTest, BugreportzVersionEmpty) {

    SetBugreportzVersion("");



    const char* args[1024] = {"bugreport", "file.zip"};

    ASSERT_EQ(-1, br_.DoIt(kTransportLocal, "HannibalLecter", 2, args));

}



// Tests 'adb bugreport file.zip' when the main bugreportz command failed

TEST_F(BugreportTest, BugreportzFailed) {

    SetBugreportzVersion("1.1");
@@ -294,7 +302,6 @@ TEST_F(BugreportTest, BugreportzFailed) { 
// Tests 'adb bugreport file.zip' when the bugreport could not be pulled

TEST_F(BugreportTest, PullFails) {

    SetBugreportzVersion("1.1");

    ExpectProgress(100, 100);

    EXPECT_CALL(br_, SendShellCommand(kTransportLocal, "HannibalLecter", "bugreportz -p", false, _))

        .WillOnce(DoAll(WithArg<4>(WriteOnStdout("OK:/device/bugreport.zip")),

                        WithArg<4>(ReturnCallbackDone())));