From 0db1394c12257fd8ba4d75d93b7a88fd259ce759 Mon Sep 17 00:00:00 2001 From: Twarit Waikar Date: Thu, 10 Mar 2022 15:06:44 +0530 Subject: [PATCH 1/3] git: Add tests for GitAPI --- p4-fusion/commands/info_result.cc | 12 +---- p4-fusion/main.cc | 1 + p4-fusion/p4_api.h | 1 - p4-fusion/thread_pool.cc | 2 - p4-fusion/utils/time_helpers.cc | 15 ++++++ p4-fusion/utils/time_helpers.h | 9 ++++ tests/CMakeLists.txt | 6 +++ tests/main.cc | 81 +++---------------------------- tests/tests.common.h | 39 +++++++++++++++ tests/tests.git.h | 48 ++++++++++++++++++ tests/tests.utils.h | 64 ++++++++++++++++++++++++ 11 files changed, 191 insertions(+), 87 deletions(-) create mode 100644 p4-fusion/utils/time_helpers.cc create mode 100644 p4-fusion/utils/time_helpers.h create mode 100644 tests/tests.common.h create mode 100644 tests/tests.git.h create mode 100644 tests/tests.utils.h diff --git a/p4-fusion/commands/info_result.cc b/p4-fusion/commands/info_result.cc index 292e5cd6..fa161bf8 100644 --- a/p4-fusion/commands/info_result.cc +++ b/p4-fusion/commands/info_result.cc @@ -5,19 +5,11 @@ * For full license text, see the LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause */ #include "info_result.h" +#include "utils/time_helpers.h" void InfoResult::OutputStat(StrDict* varList) { std::string serverDate = varList->GetVar("serverDate")->Text(); - // ... serverDate 2021/09/06 04:49:28 -0700 PDT - // ^ ^ - // 0 20 - std::string timezone = serverDate.substr(20, 5); - - int hours = std::stoi(timezone.substr(1, 2)); - int minutes = std::stoi(timezone.substr(3, 2)); - int sign = timezone[0] == '-' ? -1 : +1; - - m_TimezoneMinutes = sign * (hours * 60 + minutes); + m_TimezoneMinutes = Time::GetTimezoneMinutes(serverDate); } diff --git a/p4-fusion/main.cc b/p4-fusion/main.cc index 5a843abb..9743e6fd 100644 --- a/p4-fusion/main.cc +++ b/p4-fusion/main.cc @@ -181,6 +181,7 @@ int Main(int argc, char** argv) PRINT("Creating " << networkThreads << " network threads"); ThreadPool::GetSingleton()->Initialize(networkThreads); + SUCCESS("Created " << ThreadPool::GetSingleton()->GetThreadCount() << " threads in thread pool"); int startupDownloadsCount = 0; long long lastDownloadCL = -1; diff --git a/p4-fusion/p4_api.h b/p4-fusion/p4_api.h index d479cf1c..e6001ae2 100644 --- a/p4-fusion/p4_api.h +++ b/p4-fusion/p4_api.h @@ -24,7 +24,6 @@ class P4API { - ClientApi m_ClientAPI; MapApi m_ClientMapping; int m_Usage; diff --git a/p4-fusion/thread_pool.cc b/p4-fusion/thread_pool.cc index 7bc88fce..ed0b0aa4 100644 --- a/p4-fusion/thread_pool.cc +++ b/p4-fusion/thread_pool.cc @@ -135,8 +135,6 @@ void ThreadPool::Initialize(int size) } })); } - - SUCCESS("Created " << size << " threads in thread pool"); } ThreadPool::~ThreadPool() diff --git a/p4-fusion/utils/time_helpers.cc b/p4-fusion/utils/time_helpers.cc new file mode 100644 index 00000000..8285f872 --- /dev/null +++ b/p4-fusion/utils/time_helpers.cc @@ -0,0 +1,15 @@ +#include "time_helpers.h" + +int Time::GetTimezoneMinutes(const std::string& timezoneStr) +{ + // string -> ... serverDate 2021/09/06 04:49:28 -0700 PDT + // ^ ^ + // index -> 0 20 + std::string timezone = timezoneStr.substr(20, 5); + + int hours = std::stoi(timezone.substr(1, 2)); + int minutes = std::stoi(timezone.substr(3, 2)); + int sign = timezone[0] == '-' ? -1 : +1; + + return sign * (hours * 60 + minutes); +} diff --git a/p4-fusion/utils/time_helpers.h b/p4-fusion/utils/time_helpers.h new file mode 100644 index 00000000..d76b79f9 --- /dev/null +++ b/p4-fusion/utils/time_helpers.h @@ -0,0 +1,9 @@ +#pragma once + +#include + +class Time +{ +public: + static int GetTimezoneMinutes(const std::string& timezoneStr); +}; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f72dd5d0..4572121b 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -2,6 +2,8 @@ add_executable(p4-fusion-test main.cc ../p4-fusion/utils/std_helpers.cc + ../p4-fusion/utils/time_helpers.cc + ../p4-fusion/git_api.cc ) target_include_directories(p4-fusion-test PRIVATE @@ -10,3 +12,7 @@ target_include_directories(p4-fusion-test PRIVATE ../vendor/libgit2/include/ ../vendor/minitrace/ ) + +target_link_libraries(p4-fusion-test PRIVATE + git2 +) diff --git a/tests/main.cc b/tests/main.cc index ea58868d..4eb3d1b4 100644 --- a/tests/main.cc +++ b/tests/main.cc @@ -6,82 +6,15 @@ */ #include -#include "common.h" -#include "utils/std_helpers.h" - -#define TEST_START() \ - static int32_t failedTestCaseCount = 0; \ - SUCCESS("Running p4-fusion tests..."); - -#define TEST(value, expected) \ - if (value == expected) \ - { \ - PRINT(#value << " == " << #expected); \ - } \ - else \ - { \ - failedTestCaseCount++; \ - ERR(#value << " == " << #expected); \ - } - -#define TEST_END() \ - if (failedTestCaseCount == 0) \ - { \ - SUCCESS("All test cases passed successfully"); \ - } \ - else \ - { \ - ERR(failedTestCaseCount << " tests are failing"); \ - } - -#define TEST_EXIT_CODE() failedTestCaseCount +#include "tests.common.h" +#include "tests.utils.h" +#include "tests.git.h" int main() { - TEST_START(); - - TEST(STDHelpers::Contains("abc", "a"), true); - TEST(STDHelpers::Contains("abc", "abcd"), false); - TEST(STDHelpers::Contains("abc", ""), true); - TEST(STDHelpers::Contains("", "abcd"), false); - TEST(STDHelpers::Contains("", ""), true); - TEST(STDHelpers::Contains("//depot/path/.git/config", "/.git/"), true); - TEST(STDHelpers::Contains("//depot/path/.git/config", ".git"), true); - TEST(STDHelpers::Contains("//depot/path/.git", "/.git"), true); - TEST(STDHelpers::Contains("//depot/path/.git", "/.git/"), false); - TEST(STDHelpers::Contains("//depot/path/.git", ".git"), true); - TEST(STDHelpers::Contains("//.git", ".git"), true); - TEST(STDHelpers::Contains("//", "/.git/"), false); - TEST(STDHelpers::Contains("//", ".git"), false); - - TEST(STDHelpers::StartsWith("abc", "a"), true); - TEST(STDHelpers::StartsWith("abc", "b"), false); - TEST(STDHelpers::StartsWith("ab", "abc"), false); - TEST(STDHelpers::StartsWith("ab", "abc"), false); - TEST(STDHelpers::StartsWith("//depot/path/", "//"), true); - TEST(STDHelpers::StartsWith("//depot/path/", "//depot"), true); - TEST(STDHelpers::StartsWith("//depot/path/", "//_depot"), false); - - TEST(STDHelpers::EndsWith("abc", "abc"), true); - TEST(STDHelpers::EndsWith("abc", "ab"), false); - TEST(STDHelpers::EndsWith("abc", "a"), false); - TEST(STDHelpers::EndsWith("abc", "abcd"), false); - TEST(STDHelpers::EndsWith("abc", "z"), false); - TEST(STDHelpers::EndsWith("//...", "/..."), true); - TEST(STDHelpers::EndsWith("//", "/..."), false); - TEST(STDHelpers::EndsWith("", "/..."), false); - TEST(STDHelpers::EndsWith("", ""), true); - TEST(STDHelpers::EndsWith("", "/..."), false); - TEST(STDHelpers::EndsWith("//depot/path/", "/..."), false); - TEST(STDHelpers::EndsWith("//depot/path", "/..."), false); - TEST(STDHelpers::EndsWith("//depot/path/...", "/..."), true); - TEST(STDHelpers::EndsWith("//depot/path", ".git"), false); - TEST(STDHelpers::EndsWith("//depot/path/.git/config", ".git"), false); - TEST(STDHelpers::EndsWith("//depot/path/.git", "/.git"), true); - TEST(STDHelpers::EndsWith("//depot/path/.git", "/.git/"), false); - TEST(STDHelpers::EndsWith("//depot/path/.git", ".git"), true); - - TEST_END(); + TEST_REPORT("Utils", TestUtils()); + TEST_REPORT("GitAPI", TestGitAPI()); - return TEST_EXIT_CODE(); + SUCCESS("All test cases passed"); + return 0; } diff --git a/tests/tests.common.h b/tests/tests.common.h new file mode 100644 index 00000000..903ef74e --- /dev/null +++ b/tests/tests.common.h @@ -0,0 +1,39 @@ +#include "common.h" + +#define TEST_START() \ + static int32_t failedTestCaseCount = 0; + +#define TEST(value, expected) \ + if (value == expected) \ + { \ + PRINT(#value << " == " << #expected); \ + } \ + else \ + { \ + failedTestCaseCount++; \ + ERR(#value << " == " << value << " but != " \ + << #expected << " (expected)"); \ + } + +#define TEST_END() \ + if (failedTestCaseCount != 0) \ + { \ + ERR(failedTestCaseCount << " tests failed so far"); \ + } + +#define TEST_EXIT_CODE() failedTestCaseCount + +#define TEST_REPORT(testName, functionCall) \ + do \ + { \ + auto result = functionCall; \ + if (result == 0) \ + { \ + SUCCESS(testName << " tests succeeded"); \ + } \ + else \ + { \ + ERR(testName << " failed with exit code " << result); \ + return result; \ + } \ + } while (false) diff --git a/tests/tests.git.h b/tests/tests.git.h new file mode 100644 index 00000000..745d431c --- /dev/null +++ b/tests/tests.git.h @@ -0,0 +1,48 @@ +#pragma once + +#include "tests.common.h" +#include "git_api.h" + +int TestGitAPI() +{ + TEST_START(); + + GitAPI git(false); + + TEST(git.InitializeRepository("/tmp/test-repo"), true); + git.CreateIndex(); + git.AddFileToIndex("//a/b/c/foo.txt", { 'x', 'y', 'z' }); + git.Commit( + "//a/b/c/...", + "12345678", + "test.user", + "test@user", + 0, + "Test description", + 10000000); + TEST(git.IsHEADExists(), true); + TEST(git.IsRepositoryClonedFrom("//a/b/c/..."), true); + TEST(git.IsRepositoryClonedFrom("//a/b/c/d/..."), false); + TEST(git.IsRepositoryClonedFrom("//x/y/z/..."), false); + TEST(git.DetectLatestCL(), "12345678"); + + git.RemoveFileFromIndex("//a/b/c/foo.txt"); + git.Commit( + "//a/b/c/...", + "12345679", + "test.user.2", + "test2@user", + 0, + "Test description", + 20000000); + TEST(git.IsHEADExists(), true); + TEST(git.IsRepositoryClonedFrom("//a/b/c/..."), true); + TEST(git.IsRepositoryClonedFrom("//a/b/c/d/..."), false); + TEST(git.IsRepositoryClonedFrom("//x/y/z/..."), false); + TEST(git.DetectLatestCL(), "12345679"); + + git.CloseIndex(); + + TEST_END(); + return TEST_EXIT_CODE(); +} diff --git a/tests/tests.utils.h b/tests/tests.utils.h new file mode 100644 index 00000000..ac67e0b2 --- /dev/null +++ b/tests/tests.utils.h @@ -0,0 +1,64 @@ +#pragma once + +#include "tests.common.h" +#include "utils/std_helpers.h" +#include "utils/time_helpers.h" + +int TestUtils() +{ + TEST_START(); + + TEST(STDHelpers::Contains("abc", "a"), true); + TEST(STDHelpers::Contains("abc", "abcd"), false); + TEST(STDHelpers::Contains("abc", ""), true); + TEST(STDHelpers::Contains("", "abcd"), false); + TEST(STDHelpers::Contains("", ""), true); + TEST(STDHelpers::Contains("//depot/path/.git/config", "/.git/"), true); + TEST(STDHelpers::Contains("//depot/path/.git/config", ".git"), true); + TEST(STDHelpers::Contains("//depot/path/.git", "/.git"), true); + TEST(STDHelpers::Contains("//depot/path/.git", "/.git/"), false); + TEST(STDHelpers::Contains("//depot/path/.git", ".git"), true); + TEST(STDHelpers::Contains("//.git", ".git"), true); + TEST(STDHelpers::Contains("//", "/.git/"), false); + TEST(STDHelpers::Contains("//", ".git"), false); + + TEST(STDHelpers::StartsWith("abc", "a"), true); + TEST(STDHelpers::StartsWith("abc", "b"), false); + TEST(STDHelpers::StartsWith("ab", "abc"), false); + TEST(STDHelpers::StartsWith("ab", "abc"), false); + TEST(STDHelpers::StartsWith("//depot/path/", "//"), true); + TEST(STDHelpers::StartsWith("//depot/path/", "//depot"), true); + TEST(STDHelpers::StartsWith("//depot/path/", "//_depot"), false); + + TEST(STDHelpers::EndsWith("abc", "abc"), true); + TEST(STDHelpers::EndsWith("abc", "ab"), false); + TEST(STDHelpers::EndsWith("abc", "a"), false); + TEST(STDHelpers::EndsWith("abc", "abcd"), false); + TEST(STDHelpers::EndsWith("abc", "z"), false); + TEST(STDHelpers::EndsWith("//...", "/..."), true); + TEST(STDHelpers::EndsWith("//", "/..."), false); + TEST(STDHelpers::EndsWith("", "/..."), false); + TEST(STDHelpers::EndsWith("", ""), true); + TEST(STDHelpers::EndsWith("", "/..."), false); + TEST(STDHelpers::EndsWith("//depot/path/", "/..."), false); + TEST(STDHelpers::EndsWith("//depot/path", "/..."), false); + TEST(STDHelpers::EndsWith("//depot/path/...", "/..."), true); + TEST(STDHelpers::EndsWith("//depot/path", ".git"), false); + TEST(STDHelpers::EndsWith("//depot/path/.git/config", ".git"), false); + TEST(STDHelpers::EndsWith("//depot/path/.git", "/.git"), true); + TEST(STDHelpers::EndsWith("//depot/path/.git", "/.git/"), false); + TEST(STDHelpers::EndsWith("//depot/path/.git", ".git"), true); + + TEST(Time::GetTimezoneMinutes("2022/03/09 22:59:04 -0800 PST"), -480); + TEST(Time::GetTimezoneMinutes("2022/03/09 22:59:04 +0800 PST"), +480); + TEST(Time::GetTimezoneMinutes("2022/03/09 22:59:04 -1800 PST"), -1080); + TEST(Time::GetTimezoneMinutes("2022/03/09 22:59:04 +1800 PST"), +1080); + TEST(Time::GetTimezoneMinutes("2022/03/09 22:59:04 +0530 IST"), +330); + TEST(Time::GetTimezoneMinutes("2022/03/09 22:59:04 -1234 XXX"), -754); + TEST(Time::GetTimezoneMinutes("2022/03/09 22:59:04 +1234 XXX"), +754); + TEST(Time::GetTimezoneMinutes("2022/03/09 22:59:04 +0000 GMT"), +0); + TEST(Time::GetTimezoneMinutes("2022/03/09 22:59:04 -0000 GMT"), +0); + + TEST_END(); + return TEST_EXIT_CODE(); +} From fa0bbf3ef8ddf5d2398ffcac30332712b28734c8 Mon Sep 17 00:00:00 2001 From: Twarit Waikar Date: Wed, 16 Mar 2022 21:10:02 +0530 Subject: [PATCH 2/3] all: Improve error handling all over the codebase --- p4-fusion/commands/change_list.cc | 63 ++++++++++++++++++---------- p4-fusion/commands/change_list.h | 24 +++++++---- p4-fusion/commands/changes_result.cc | 18 +++----- p4-fusion/commands/info_result.cc | 17 +++++++- p4-fusion/commands/info_result.h | 2 + p4-fusion/commands/users_result.cc | 13 +++++- p4-fusion/main.cc | 34 +++++++-------- p4-fusion/p4_api.cc | 23 ++++++---- p4-fusion/p4_api.h | 6 ++- tests/tests.utils.h | 1 + 10 files changed, 128 insertions(+), 73 deletions(-) diff --git a/p4-fusion/commands/change_list.cc b/p4-fusion/commands/change_list.cc index 3072cfe7..991e3e34 100644 --- a/p4-fusion/commands/change_list.cc +++ b/p4-fusion/commands/change_list.cc @@ -13,6 +13,14 @@ #include "thread_pool.h" +ChangeList::ChangeList(const std::string& clNumber, const std::string& clDescription, const std::string& userID, const int64_t& clTimestamp) + : number(clNumber) + , user(userID) + , description(clDescription) + , timestamp(clTimestamp) +{ +} + void ChangeList::PrepareDownload() { ChangeList& cl = *this; @@ -25,7 +33,7 @@ void ChangeList::PrepareDownload() std::unique_lock lock((*(cl.canDownloadMutex))); *cl.canDownload = true; } - cl.canDownloadCV->notify_one(); + cl.canDownloadCV->notify_all(); }); } @@ -71,32 +79,41 @@ void ChangeList::StartDownload(const std::string& depotPath, const int& printBat cl.commitCV->notify_all(); } - // Empty the batches - if (printBatchFiles->size() > printBatch || i == cl.changedFiles.size() - 1) + // Clear the batches if it fits + if (printBatchFiles->size() == printBatch) { - if (printBatchFiles->empty()) - { - continue; - } - - ThreadPool::GetSingleton()->AddJob([&cl, printBatchFiles, printBatchFileData](P4API* p4) - { - const PrintResult& printData = p4->PrintFiles(*printBatchFiles); - - for (int i = 0; i < printBatchFiles->size(); i++) - { - printBatchFileData->at(i)->contents = std::move(printData.GetPrintData().at(i).contents); - } - - (*cl.filesDownloaded) += printBatchFiles->size(); - - cl.commitCV->notify_all(); - }); + cl.Flush(printBatchFiles, printBatchFileData); + // We let go of the refs held by us and create new ones to queue the next batch printBatchFiles = std::make_shared>(); printBatchFileData = std::make_shared>(); + // Now only the thread job has access to the older batch } } + + // Flush any remaining files that were smaller in number than the total batch size + if (!printBatchFiles->empty()) + { + cl.Flush(printBatchFiles, printBatchFileData); + } + }); +} + +void ChangeList::Flush(std::shared_ptr> printBatchFiles, std::shared_ptr> printBatchFileData) +{ + // Share ownership of this batch with the thread job + ThreadPool::GetSingleton()->AddJob([this, printBatchFiles, printBatchFileData](P4API* p4) + { + const PrintResult& printData = p4->PrintFiles(*printBatchFiles); + + for (int i = 0; i < printBatchFiles->size(); i++) + { + printBatchFileData->at(i)->contents = std::move(printData.GetPrintData().at(i).contents); + } + + (*filesDownloaded) += printBatchFiles->size(); + + commitCV->notify_all(); }); } @@ -113,7 +130,11 @@ void ChangeList::Clear() user.clear(); description.clear(); changedFiles.clear(); + filesDownloaded.reset(); + canDownload.reset(); + canDownloadMutex.reset(); + canDownloadCV.reset(); commitMutex.reset(); commitCV.reset(); } diff --git a/p4-fusion/commands/change_list.h b/p4-fusion/commands/change_list.h index 394b2431..4e3d0776 100644 --- a/p4-fusion/commands/change_list.h +++ b/p4-fusion/commands/change_list.h @@ -19,17 +19,27 @@ struct ChangeList std::string number; std::string user; std::string description; - long long timestamp; + int64_t timestamp = 0; std::vector changedFiles; - std::shared_ptr> filesDownloaded; - std::shared_ptr> canDownload; - std::shared_ptr canDownloadMutex; - std::shared_ptr canDownloadCV; - std::shared_ptr commitMutex; - std::shared_ptr commitCV; + + std::shared_ptr> filesDownloaded = std::make_shared>(-1); + std::shared_ptr> canDownload = std::make_shared>(false); + std::shared_ptr canDownloadMutex = std::make_shared(); + std::shared_ptr canDownloadCV = std::make_shared(); + std::shared_ptr commitMutex = std::make_shared(); + std::shared_ptr commitCV = std::make_shared(); + + ChangeList(const std::string& number, const std::string& description, const std::string& user, const int64_t& timestamp); + + ChangeList(const ChangeList& other) = default; + ChangeList& operator=(const ChangeList&) = delete; + ChangeList(ChangeList&&) = default; + ChangeList& operator=(ChangeList&&) = default; + ~ChangeList() = default; void PrepareDownload(); void StartDownload(const std::string& depotPath, const int& printBatch, const bool includeBinaries); + void Flush(std::shared_ptr> printBatchFiles, std::shared_ptr> printBatchFileData); void WaitForDownload(); void Clear(); }; diff --git a/p4-fusion/commands/changes_result.cc b/p4-fusion/commands/changes_result.cc index 9663bdcf..2deb2873 100644 --- a/p4-fusion/commands/changes_result.cc +++ b/p4-fusion/commands/changes_result.cc @@ -8,17 +8,9 @@ void ChangesResult::OutputStat(StrDict* varList) { - m_Changes.push_back({}); - ChangeList& data = m_Changes.back(); - - data.number = varList->GetVar("change")->Text(); - data.description = varList->GetVar("desc")->Text(); - data.user = varList->GetVar("user")->Text(); - data.timestamp = varList->GetVar("time")->Atoi64(); - data.filesDownloaded = std::make_shared>(-1); - data.canDownload = std::make_shared>(false); - data.canDownloadMutex = std::make_shared(); - data.canDownloadCV = std::make_shared(); - data.commitMutex = std::make_shared(); - data.commitCV = std::make_shared(); + m_Changes.emplace_back( + varList->GetVar("change")->Text(), + varList->GetVar("desc")->Text(), + varList->GetVar("user")->Text(), + varList->GetVar("time")->Atoi64()); } diff --git a/p4-fusion/commands/info_result.cc b/p4-fusion/commands/info_result.cc index fa161bf8..7e3f7cd4 100644 --- a/p4-fusion/commands/info_result.cc +++ b/p4-fusion/commands/info_result.cc @@ -5,11 +5,24 @@ * For full license text, see the LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause */ #include "info_result.h" -#include "utils/time_helpers.h" + +InfoResult::InfoResult() + : m_TimezoneMinutes(0) +{ +} void InfoResult::OutputStat(StrDict* varList) { std::string serverDate = varList->GetVar("serverDate")->Text(); - m_TimezoneMinutes = Time::GetTimezoneMinutes(serverDate); + // ... serverDate 2021/09/06 04:49:28 -0700 PDT + // ^ ^ + // 0 20 + std::string timezone = serverDate.substr(20, 5); + + int hours = std::stoi(timezone.substr(1, 2)); + int minutes = std::stoi(timezone.substr(3, 2)); + int sign = timezone[0] == '-' ? -1 : +1; + + m_TimezoneMinutes = sign * (hours * 60 + minutes); } diff --git a/p4-fusion/commands/info_result.h b/p4-fusion/commands/info_result.h index 71a32b38..b49aa0dd 100644 --- a/p4-fusion/commands/info_result.h +++ b/p4-fusion/commands/info_result.h @@ -14,6 +14,8 @@ class InfoResult : public Result int m_TimezoneMinutes; public: + InfoResult(); + int GetServerTimezoneMinutes() const { return m_TimezoneMinutes; }; void OutputStat(StrDict* varList) override; diff --git a/p4-fusion/commands/users_result.cc b/p4-fusion/commands/users_result.cc index fa7a0621..e009e77a 100644 --- a/p4-fusion/commands/users_result.cc +++ b/p4-fusion/commands/users_result.cc @@ -8,10 +8,19 @@ void UsersResult::OutputStat(StrDict* varList) { - UserID userID = varList->GetVar("User")->Text(); + StrPtr* userIDPtr = varList->GetVar("User"); + StrPtr* emailPtr = varList->GetVar("Email"); + + if (!userIDPtr || !emailPtr) + { + ERR("UserID or email not found for a Perforce user"); + return; + } + + UserID userID = userIDPtr->Text(); UserData userData; - userData.email = varList->GetVar("Email")->Text(); + userData.email = emailPtr->Text(); StrPtr* fullNamePtr = varList->GetVar("FullName"); if (fullNamePtr) diff --git a/p4-fusion/main.cc b/p4-fusion/main.cc index 9743e6fd..bdd54562 100644 --- a/p4-fusion/main.cc +++ b/p4-fusion/main.cc @@ -184,37 +184,37 @@ int Main(int argc, char** argv) SUCCESS("Created " << ThreadPool::GetSingleton()->GetThreadCount() << " threads in thread pool"); int startupDownloadsCount = 0; - long long lastDownloadCL = -1; // Go in the chronological order - for (size_t i = 0; i < changes.size(); i++) + size_t lastDownloadedCL = 0; + for (size_t currentCL = 0; currentCL < changes.size() && currentCL < lookAhead; currentCL++) { - if (startupDownloadsCount == lookAhead) - { - break; - } + ChangeList& cl = changes.at(currentCL); - // Start running `p4 print` on changed files - ChangeList& cl = changes.at(i); + // Start gathering changed files with `p4 describe` cl.PrepareDownload(); + + // Start running `p4 print` on changed files when the describe is finished cl.StartDownload(depotPath, printBatch, includeBinaries); startupDownloadsCount++; - lastDownloadCL = i; + + lastDownloadedCL = currentCL; } - SUCCESS("Queued first " << startupDownloadsCount << " CLs for downloading"); + SUCCESS("Queued first " << startupDownloadsCount << " CLs up until CL " << changes.at(lastDownloadedCL).number << " for downloading"); int timezoneMinutes = p4.Info().GetServerTimezoneMinutes(); + SUCCESS("Perforce server timezone is " << timezoneMinutes << " minutes"); // Map usernames to emails - const UsersResult& usersResult = p4.Users(); - const std::unordered_map& users = usersResult.GetUserEmails(); - + const std::unordered_map users = std::move(p4.Users().GetUserEmails()); SUCCESS("Received userbase details from the Perforce server"); // Commit procedure start Timer commitTimer; + PRINT("Last CL to start downloading is CL " << changes.at(lastDownloadedCL).number); + git.CreateIndex(); for (size_t i = 0; i < changes.size(); i++) { @@ -271,15 +271,15 @@ int Main(int argc, char** argv) SUCCESS( "CL " << cl.number << " --> Commit " << commitSHA - << " with " << cl.changedFiles.size() << " files (" << i << "/" << changes.size() << "|" << lastDownloadCL - (long long)i << "). " + << " with " << cl.changedFiles.size() << " files (" << i + 1 << "/" << changes.size() << "|" << lastDownloadedCL - (long long)i << "). " << "Elapsed " << commitTimer.GetTimeS() / 60.0f << " mins. " << ((commitTimer.GetTimeS() / 60.0f) / (float)(i + 1)) * (changes.size() - i - 1) << " mins left."); // Start downloading the CL chronologically after the last CL that was previously downloaded, if there's still some left - if (lastDownloadCL + 1 < changes.size()) + if (lastDownloadedCL + 1 < changes.size()) { - lastDownloadCL++; - ChangeList& downloadCL = changes.at(lastDownloadCL); + lastDownloadedCL++; + ChangeList& downloadCL = changes.at(lastDownloadedCL); downloadCL.PrepareDownload(); downloadCL.StartDownload(depotPath, printBatch, includeBinaries); } diff --git a/p4-fusion/p4_api.cc b/p4-fusion/p4_api.cc index f7de5593..8179de0b 100644 --- a/p4-fusion/p4_api.cc +++ b/p4-fusion/p4_api.cc @@ -17,14 +17,20 @@ std::string P4API::P4USER; std::string P4API::P4CLIENT; int P4API::CommandRetries = 1; int P4API::CommandRefreshThreshold = 1; -std::mutex P4API::ReinitializationMutex; +std::mutex P4API::InitializationMutex; P4API::P4API() { - if (!Initialize()) { - ERR("Could not initialize P4API"); - return; + // Helix Core C++ API seems to crash while making connections parallely. + // Although, this function is not currently accessed in parallel, it can + // be during retries. + std::unique_lock lock(InitializationMutex); + if (!Initialize()) + { + ERR("Could not initialize P4API"); + return; + } } AddClientSpecView(ClientSpec.mapping); @@ -43,6 +49,7 @@ bool P4API::Initialize() m_ClientAPI.SetClient(P4CLIENT.c_str()); m_ClientAPI.SetProtocol("tag", ""); m_ClientAPI.Init(&e); + if (!CheckErrors(e, msg)) { ERR("Could not initialize Helix Core C/C++ API"); @@ -54,11 +61,14 @@ bool P4API::Initialize() bool P4API::Deinitialize() { + std::unique_lock lock(InitializationMutex); + Error e; StrBuf msg; m_ClientAPI.Final(&e); CheckErrors(e, msg); + return true; } @@ -66,11 +76,6 @@ bool P4API::Reinitialize() { MTR_SCOPE("P4", __func__); - // Helix Core C++ API seems to crash while making connections parallely. - // The Initialize() function is immune to this because it is never run in - // parrallel, while Reinitialize() function can get in a situation where it is - // called in parallel. - std::unique_lock lock(ReinitializationMutex); bool status = Deinitialize() && Initialize(); return status; } diff --git a/p4-fusion/p4_api.h b/p4-fusion/p4_api.h index e6001ae2..c4d836f0 100644 --- a/p4-fusion/p4_api.h +++ b/p4-fusion/p4_api.h @@ -43,7 +43,9 @@ class P4API static ClientResult::ClientSpecData ClientSpec; static int CommandRetries; static int CommandRefreshThreshold; - static std::mutex ReinitializationMutex; + + // Helix Core C++ API seems to crash while making connections parallely. + static std::mutex InitializationMutex; static bool InitializeLibraries(); static bool ShutdownLibraries(); @@ -72,7 +74,7 @@ class P4API Result Sync(const std::string& path); SyncResult GetFilesToSyncAtCL(const std::string& path, const std::string& cl); PrintResult PrintFile(const std::string& filePathRevision); - PrintResult PrintFiles(const std::vector& files); + PrintResult PrintFiles(const std::vector& fileRevisions); void UpdateClientSpec(); ClientResult Client(); UsersResult Users(); diff --git a/tests/tests.utils.h b/tests/tests.utils.h index ac67e0b2..8ab4292a 100644 --- a/tests/tests.utils.h +++ b/tests/tests.utils.h @@ -49,6 +49,7 @@ int TestUtils() TEST(STDHelpers::EndsWith("//depot/path/.git", "/.git/"), false); TEST(STDHelpers::EndsWith("//depot/path/.git", ".git"), true); + TEST(Time::GetTimezoneMinutes("2022/03/15 09:56:15 -0400 EDT"), -240); TEST(Time::GetTimezoneMinutes("2022/03/09 22:59:04 -0800 PST"), -480); TEST(Time::GetTimezoneMinutes("2022/03/09 22:59:04 +0800 PST"), +480); TEST(Time::GetTimezoneMinutes("2022/03/09 22:59:04 -1800 PST"), -1080); From b3e1692cab718cca0fe249b035d3134f75b32b3c Mon Sep 17 00:00:00 2001 From: Twarit Waikar Date: Fri, 25 Mar 2022 12:19:19 +0530 Subject: [PATCH 3/3] build: Add note about running mixed build configurations --- README.md | 2 +- p4-fusion/commands/info_result.cc | 13 ++----------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 800d7bf3..3a61b34a 100644 --- a/README.md +++ b/README.md @@ -91,7 +91,7 @@ This tool uses C++11 and thus it should work with much older GCC versions. We ha ./generate_cache.sh Debug ``` -Replace `Debug` with `Release` for a speed-optimized binary. Debug will run marginally slower (considering the tool is mostly bottlenecked by network I/O) but will contain debug symbols and allows a better debugging experience while working with a debugger. +Replace `Debug` with `Release` or `RelWithDebInfo` or `MinSizeRel` for a differently optimized binary. Debug will run marginally slower (considering the tool is mostly bottlenecked by network I/O) but will contain debug symbols and allows a better debugging experience while working with a debugger. By default tracing is disabled in p4-fusion. It can be enabled by including `p` in the second argument while generating the CMake cache. If tracing is enabled, p4-fusion generates trace JSON files in the cloning directory. These files can be opened in the `about:tracing` window in Chromium web browsers to view the tracing data. diff --git a/p4-fusion/commands/info_result.cc b/p4-fusion/commands/info_result.cc index 7e3f7cd4..7e0cbadf 100644 --- a/p4-fusion/commands/info_result.cc +++ b/p4-fusion/commands/info_result.cc @@ -5,6 +5,7 @@ * For full license text, see the LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause */ #include "info_result.h" +#include "utils/time_helpers.h" InfoResult::InfoResult() : m_TimezoneMinutes(0) @@ -14,15 +15,5 @@ InfoResult::InfoResult() void InfoResult::OutputStat(StrDict* varList) { std::string serverDate = varList->GetVar("serverDate")->Text(); - - // ... serverDate 2021/09/06 04:49:28 -0700 PDT - // ^ ^ - // 0 20 - std::string timezone = serverDate.substr(20, 5); - - int hours = std::stoi(timezone.substr(1, 2)); - int minutes = std::stoi(timezone.substr(3, 2)); - int sign = timezone[0] == '-' ? -1 : +1; - - m_TimezoneMinutes = sign * (hours * 60 + minutes); + m_TimezoneMinutes = Time::GetTimezoneMinutes(serverDate); }