Merge pull request #96 from salesforce/build-fixes

Fix the existing repo update code path

Author: twarit-waikar

README.md

@@ -138,7 +138,8 @@ Because of the extra effort the script performs, expect it to take orders of mag
 ## Build
 
 0. Pre-requisites
-  * Install openssl (both 1.1.1 and 3 work)
+  * Install zlib - `brew install zlib`
+  * Install openssl (try 1.1.1, 3.x.x doesn't work with p4 API libs) - `brew install openssl@1.1`
   * Install CMake 3.16+.
   * Install g++ 11.2.0 (older versions compatible with C++11 are also supported).
   * Clone this repository or [get a release distribution](https://github.com/salesforce/p4-fusion/releases).

generate_cache.sh

@@ -9,13 +9,13 @@ cmakeArgs=(
     -DBUILD_SHARED_LIBS=OFF
     -DBUILD_CLAR=OFF
     -DBUILD_EXAMPLES=OFF
-    -DUSE_BUNDLED_ZLIB=ON
+    -DUSE_BUNDLED_ZLIB=OFF
     -DREGEX_BACKEND=builtin
     -DTHREADSAFE=ON
     -DUSE_SSH=OFF
     -DUSE_HTTPS=OFF
     -DUSE_THREADS=ON
-    -DOPENSSL_ROOT_DIR=/usr/local/ssl
+    -DOPENSSL_ROOT_DIR=/opt/homebrew/Cellar/openssl@1.1/1.1.1w
     -DCMAKE_C_COMPILER=/usr/bin/gcc
     -DCMAKE_CXX_COMPILER=/usr/bin/g++
 )

p4-fusion/commands/changes_result.cc

@@ -4,7 +4,6 @@
  * SPDX-License-Identifier: BSD-3-Clause
  * For full license text, see the LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
-#include <algorithm>
 #include "changes_result.h"
 
 void ChangesResult::OutputStat(StrDict* varList)
@@ -15,8 +14,3 @@ void ChangesResult::OutputStat(StrDict* varList)
 	    varList->GetVar("user")->Text(),
 	    varList->GetVar("time")->Atoi64());
 }
-
-void ChangesResult::reverse()
-{
-	std::reverse(m_Changes.begin(), m_Changes.end());
-}

p4-fusion/commands/changes_result.h

@@ -7,7 +7,9 @@
 #pragma once
 
 #include <vector>
+
 #include "common.h"
+
 #include "change_list.h"
 #include "result.h"
 
@@ -18,7 +20,6 @@ class ChangesResult : public Result
 
 public:
 	std::vector<ChangeList>& GetChanges() { return m_Changes; }
-	void reverse();
 
 	void OutputStat(StrDict* varList) override;
 };

p4-fusion/git_api.cc

@@ -263,7 +263,7 @@ std::string GitAPI::Commit(
     const std::string& user,
     const std::string& email,
     const int& timezone,
-    const std::string& desc,
+    std::string desc,
     const int64_t& timestamp,
     const std::string& mergeFromStream)
 {
@@ -278,6 +278,11 @@ std::string GitAPI::Commit(
 	git_signature* author = nullptr;
 	GIT2(git_signature_new(&author, user.c_str(), email.c_str(), timestamp, timezone));
 
+	STDHelpers::StripSurrounding(desc, '\n');
+	STDHelpers::StripSurrounding(desc, '\r');
+	STDHelpers::StripSurrounding(desc, ' ');
+	STDHelpers::StripSurrounding(desc, '\t');
+
 	// -3 to remove the trailing "..."
 	std::string commitMsg = cl + " - " + desc + "\n[p4-fusion: depot-paths = \"" + depotPath.substr(0, depotPath.size() - 3) + "\": change = " + cl + "]";
 

p4-fusion/git_api.h

@@ -1,54 +1,54 @@
-/*
- * Copyright (c) 2022 Salesforce, Inc.
- * All rights reserved.
- * SPDX-License-Identifier: BSD-3-Clause
- * For full license text, see the LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
- */
-#pragma once
-
-#include <string>
-#include <vector>
-#include <utility>
-
-#include "common.h"
-#include "git2/oid.h"
-
-struct git_repository;
-
-class GitAPI
-{
-	git_repository* m_Repo = nullptr;
-	git_index* m_Index = nullptr;
-	git_oid m_FirstCommitOid;
-
-	std::string m_CurrentBranch = "";
-
-public:
-	GitAPI(bool fsyncEnable);
-	~GitAPI();
-
-	bool InitializeRepository(const std::string& srcPath);
-	void OpenRepository(const std::string& repoPath);
-
-	bool IsHEADExists();
-	bool IsRepositoryClonedFrom(const std::string& depotPath);
-	std::string DetectLatestCL();
-
-	git_oid CreateBlob(const std::vector<char>& data);
-
-	void CreateIndex();
-	void SetActiveBranch(const std::string& branchName);
-	void AddFileToIndex(const std::string& relativePath, const std::vector<char>& contents, const bool plusx);
-	void RemoveFileFromIndex(const std::string& relativePath);
-
-	std::string Commit(
-	    const std::string& depotPath,
-	    const std::string& cl,
-	    const std::string& user,
-	    const std::string& email,
-	    const int& timezone,
-	    const std::string& desc,
-	    const int64_t& timestamp,
-	    const std::string& mergeFromStream);
-	void CloseIndex();
-};
+/*
+ * Copyright (c) 2022 Salesforce, Inc.
+ * All rights reserved.
+ * SPDX-License-Identifier: BSD-3-Clause
+ * For full license text, see the LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+#pragma once
+
+#include <string>
+#include <vector>
+#include <utility>
+
+#include "common.h"
+#include "git2/oid.h"
+
+struct git_repository;
+
+class GitAPI
+{
+	git_repository* m_Repo = nullptr;
+	git_index* m_Index = nullptr;
+	git_oid m_FirstCommitOid;
+
+	std::string m_CurrentBranch = "";
+
+public:
+	GitAPI(bool fsyncEnable);
+	~GitAPI();
+
+	bool InitializeRepository(const std::string& srcPath);
+	void OpenRepository(const std::string& repoPath);
+
+	bool IsHEADExists();
+	bool IsRepositoryClonedFrom(const std::string& depotPath);
+	std::string DetectLatestCL();
+
+	git_oid CreateBlob(const std::vector<char>& data);
+
+	void CreateIndex();
+	void SetActiveBranch(const std::string& branchName);
+	void AddFileToIndex(const std::string& relativePath, const std::vector<char>& contents, const bool plusx);
+	void RemoveFileFromIndex(const std::string& relativePath);
+
+	std::string Commit(
+	    const std::string& depotPath,
+	    const std::string& cl,
+	    const std::string& user,
+	    const std::string& email,
+	    const int& timezone,
+	    std::string desc,
+	    const int64_t& timestamp,
+	    const std::string& mergeFromStream);
+	void CloseIndex();
+};

p4-fusion/main.cc

@@ -313,29 +313,30 @@ int Main(int argc, char** argv)
 	ThreadPool::GetSingleton()->Initialize(networkThreads);
 	SUCCESS("Created " << ThreadPool::GetSingleton()->GetThreadCount() << " threads in thread pool");
 
-	int startupDownloadsCount = 0;
-
+	
 	// Go in the chronological order
 	size_t lastDownloadedCL = 0;
 	for (size_t currentCL = 0; currentCL < changes.size() && currentCL < lookAhead; currentCL++)
 	{
 		ChangeList& cl = changes.at(currentCL);
-
+		
 		// Start gathering changed files with `p4 describe` or `p4 filelog`
 		cl.PrepareDownload(branchSet);
-
+		
 		lastDownloadedCL = currentCL;
 	}
-
+	
 	// This is intentionally put in a separate loop.
 	// We want to submit `p4 describe` commands before sending any of the `p4 print` commands.
 	// Gives ~15% perf boost.
+	int startupDownloadsCount = 0;
 	for (size_t currentCL = 0; currentCL <= lastDownloadedCL; currentCL++)
 	{
 		ChangeList& cl = changes.at(currentCL);
 
 		// Start running `p4 print` on changed files when the describe is finished
 		cl.StartDownload(printBatch);
+		startupDownloadsCount++;
 	}
 
 	SUCCESS("Queued first " << startupDownloadsCount << " CLs up until CL " << changes.at(lastDownloadedCL).number << " for downloading");

p4-fusion/p4_api.cc

@@ -185,28 +185,29 @@ std::unique_ptr<TestResult> P4API::TestConnection(const int retries)
 std::unique_ptr<ChangesResult> P4API::ShortChanges(const std::string& path)
 {
 	std::unique_ptr<ChangesResult> changes = Run<ChangesResult>("changes", {
-	                                                          "-s", "submitted", // Only include submitted CLs
-	                                                          path // Depot path to get CLs from
-	                                                      });
-	changes->reverse();
+															  "-r", // Get CLs from earliest to latest
+															  "-s", "submitted", // Only include submitted CLs
+															  path // Depot path to get CLs from
+														  });
 	return changes;
 }
 
 std::unique_ptr<ChangesResult> P4API::Changes(const std::string& path)
 {
 	MTR_SCOPE("P4", __func__);
 	return Run<ChangesResult>("changes", {
-	                                         "-l", // Get full descriptions instead of sending cut-short ones
-	                                         "-s", "submitted", // Only include submitted CLs
-	                                         path // Depot path to get CLs from
-	                                     });
+											 "-l", // Get full descriptions instead of sending cut-short ones
+											 "-s", "submitted", // Only include submitted CLs
+											 path // Depot path to get CLs from
+										 });
 }
 
 std::unique_ptr<ChangesResult> P4API::Changes(const std::string& path, const std::string& from, int32_t maxCount)
 {
 	std::vector<std::string> args = {
 		"-l", // Get full descriptions instead of sending cut-short ones
 		"-s", "submitted", // Only include submitted CLs
+		"-r" // Send CLs in chronological order
 	};
 
 	// This needs to be declared outside the if scope below to
@@ -233,55 +234,54 @@ std::unique_ptr<ChangesResult> P4API::Changes(const std::string& path, const std
 	args.push_back(path + pathAddition);
 
 	std::unique_ptr<ChangesResult> result = Run<ChangesResult>("changes", args);
-	result->reverse();
 	return result;
 }
 
 std::unique_ptr<ChangesResult> P4API::ChangesFromTo(const std::string& path, const std::string& from, const std::string& to)
 {
 	std::string pathArg = path + "@" + from + "," + to;
 	return Run<ChangesResult>("changes", {
-	                                         "-s", "submitted", // Only include submitted CLs
-	                                         pathArg // Depot path to get CLs from
-	                                     });
+											 "-s", "submitted", // Only include submitted CLs
+											 pathArg // Depot path to get CLs from
+										 });
 }
 
 std::unique_ptr<ChangesResult> P4API::LatestChange(const std::string& path)
 {
 	MTR_SCOPE("P4", __func__);
 	return Run<ChangesResult>("changes", {
-	                                         "-s", "submitted", // Only include submitted CLs,
-	                                         "-m", "1", // Get top-most change
-	                                         path // Depot path to get CLs from
-	                                     });
+											 "-s", "submitted", // Only include submitted CLs,
+											 "-m", "1", // Get top-most change
+											 path // Depot path to get CLs from
+										 });
 }
 
 std::unique_ptr<ChangesResult> P4API::OldestChange(const std::string& path)
 {
 	std::unique_ptr<ChangesResult> changes = Run<ChangesResult>("changes", {
-	                                                          "-s", "submitted", // Only include submitted CLs,
-	                                                          "-m", "1", // Get top-most change
-	                                                          path // Depot path to get CLs from
-	                                                      });
-	changes->reverse();
+															  "-r", // List from earliest to latest
+															  "-s", "submitted", // Only include submitted CLs,
+															  "-m", "1", // Get top-most change
+															  path // Depot path to get CLs from
+														  });
 	return changes;
 }
 
 std::unique_ptr<DescribeResult> P4API::Describe(const std::string& cl)
 {
 	MTR_SCOPE("P4", __func__);
 	return Run<DescribeResult>("describe", { "-s", // Omit the diffs
-	                                           cl });
+											   cl });
 }
 
 std::unique_ptr<FileLogResult> P4API::FileLog(const std::string& changelist)
 {
 	return Run<FileLogResult>("filelog", {
-	                                         "-c", // restrict output to a single changelist
-	                                         changelist,
-	                                         "-m1", // don't get the full history, just the first entry.
-	                                         "//..." // rather than require the path to be passed in, just list all files.
-	                                     });
+											 "-c", // restrict output to a single changelist
+											 changelist,
+											 "-m1", // don't get the full history, just the first entry.
+											 "//..." // rather than require the path to be passed in, just list all files.
+										 });
 }
 
 std::unique_ptr<SizesResult> P4API::Size(const std::string& file)
@@ -298,16 +298,16 @@ std::unique_ptr<SyncResult> P4API::GetFilesToSyncAtCL(const std::string& path, c
 {
 	std::string clCommand = "@" + cl;
 	return Run<SyncResult>("sync", {
-	                                   "-n", // Only preview the files to sync. Don't send file contents...yet
-	                                   clCommand,
-	                               });
+									   "-n", // Only preview the files to sync. Don't send file contents...yet
+									   clCommand,
+								   });
 }
 
 std::unique_ptr<PrintResult> P4API::PrintFile(const std::string& filePathRevision)
 {
 	return Run<PrintResult>("print", {
-	                                     filePathRevision,
-	                                 });
+										 filePathRevision,
+									 });
 }
 
 std::unique_ptr<PrintResult> P4API::PrintFiles(const std::vector<std::string>& fileRevisions)
@@ -325,15 +325,15 @@ std::unique_ptr<PrintResult> P4API::PrintFiles(const std::vector<std::string>& f
 std::unique_ptr<Result> P4API::Sync(const std::string& path)
 {
 	return Run<Result>("sync", {
-	                               path // Sync a particular depot path
-	                           });
+								   path // Sync a particular depot path
+							   });
 }
 
 std::unique_ptr<UsersResult> P4API::Users()
 {
 	return Run<UsersResult>("users", {
-	                                     "-a" // Include service accounts
-	                                 });
+										 "-a" // Include service accounts
+									 });
 }
 
 std::unique_ptr<InfoResult> P4API::Info()