Improve branching mode (#11)

Author: vmihalyi-graphisoft

p4-fusion/branch_set.cc

@@ -50,19 +50,15 @@ Branch::Branch(const std::string& branch, const std::string& alias)
 	}
 }
 
-std::array<std::string, 2> Branch::SplitBranchPath(const std::string& relativeDepotPath) const
+bool Branch::IsInBranch(const std::string& relativeDepotPath) const
 {
-	if (
+	return
 	    // The relative depot branch, to match this branch path, must start with the
 	    // branch path + "/".  The "StartsWith" is put at the end of the 'and' checks,
 	    // because it takes the longest.
 	    relativeDepotPath.size() > depotBranchPath.size()
 	    && relativeDepotPath[depotBranchPath.size()] == '/'
-	    && STDHelpers::StartsWith(relativeDepotPath, depotBranchPath))
-	{
-		return { gitAlias, relativeDepotPath.substr(depotBranchPath.size() + 1) };
-	}
-	return { "", "" };
+	    && STDHelpers::StartsWith(relativeDepotPath, depotBranchPath);
 }
 
 Branch createBranchFromPath(const std::string& depotBranchPath)
@@ -140,23 +136,22 @@ BranchSet::BranchSet(GitAPI& gitAPI,
 	}
 }
 
-std::array<std::string, 2> BranchSet::splitBranchPath(const std::string& relativeDepotPath) const
+const Branch* BranchSet::getBranchFor(const std::string& relativeDepotPath) const
 {
 	// Check if the relative depot path starts with any of the branches.
 	// This checks the branches in their stored order, which can mean that having a branch
 	// order like "//a/b/c" and "//a/b" will only work if the sub-branches are listed first.
 	// To do this properly, the stored branches should be scanned based on their length - longest
 	// first, but that's extra processing and code for a use case that is rare and has a manual
 	// work around (list branches in a specific order).
-	for (auto& branch : m_branches)
+	for (const Branch& branch : m_branches)
 	{
-		auto split = branch.SplitBranchPath(relativeDepotPath);
-		if (!split[0].empty() && !split[1].empty())
+		if (branch.IsInBranch(relativeDepotPath))
 		{
-			return split;
+			return &branch;
 		}
 	}
-	return { "", "" };
+	return nullptr;
 }
 
 bool BranchSet::matchesExcludes(const std::string& depotPath) const
@@ -187,19 +182,19 @@ struct branchIntegrationMap
 	std::unordered_map<std::string, int> branchIndicies;
 	int fileCount = 0;
 
-	void addMerge(const std::string& sourceBranch, const std::string& targetBranch, const FileData& rev);
-	void addTarget(const std::string& targetBranch, const FileData& rev);
+	void addMerge(const std::string& sourceBranch, const std::string& targetBranch, const std::string& depotBranchPath, const FileData& rev);
+	void addTarget(const std::string& targetBranch, const std::string& depotBranchPath, const FileData& rev);
 
 	// note: not const, because it cleans out the branchGroups.
 	std::unique_ptr<ChangedFileGroups> createChangedFileGroups() { return std::unique_ptr<ChangedFileGroups>(new ChangedFileGroups(branchGroups, fileCount)); };
 };
 
-void branchIntegrationMap::addTarget(const std::string& targetBranch, const FileData& fileData)
+void branchIntegrationMap::addTarget(const std::string& targetBranch, const std::string& depotBranchPath, const FileData& fileData)
 {
-	addMerge(EMPTY_STRING, targetBranch, fileData);
+	addMerge(EMPTY_STRING, targetBranch, depotBranchPath, fileData);
 }
 
-void branchIntegrationMap::addMerge(const std::string& sourceBranch, const std::string& targetBranch, const FileData& fileData)
+void branchIntegrationMap::addMerge(const std::string& sourceBranch, const std::string& targetBranch, const std::string& depotBranchPath, const FileData& fileData)
 {
 	// Need to store this in the integration map, using "src/tgt" as the
 	// key.  Because stream names can't have a '/' in them, this creates a unique key.
@@ -214,6 +209,7 @@ void branchIntegrationMap::addMerge(const std::string& sourceBranch, const std::
 		BranchedFileGroup& bfg = branchGroups[index];
 		bfg.sourceBranch = sourceBranch;
 		bfg.targetBranch = targetBranch;
+		bfg.depotBranchPath = depotBranchPath;
 		bfg.hasSource = !sourceBranch.empty();
 		bfg.files.push_back(fileData);
 	}
@@ -327,50 +323,46 @@ std::unique_ptr<ChangedFileGroups> BranchSet::ParseAffectedFiles(const std::vect
 		if (HasMergeableBranch())
 		{
 			// [0] == branch name, [1] == relative path in the branch.
-			std::array<std::string, 2> branchPath = splitBranchPath(relativeDepotPath);
-			if (
-			    branchPath[0].empty()
-			    || branchPath[1].empty())
+			const Branch* branch = getBranchFor(relativeDepotPath);
+			if (branch == nullptr)
 			{
 				// not a valid branch file.  skip it.
 				continue;
 			}
 
 			// It's a valid destination to a branch.
 			// Make sure the relative path is set.
-			fileData.SetRelativeDepotPath(branchPath[1]);
+			const std::string branchFilePath = relativeDepotPath.substr(branch->depotBranchPath.size() + 1);
+			fileData.SetRelativeDepotPath(branchFilePath);
 
 			bool needsHandling = true;
 			if (fileData.IsIntegrated())
 			{
 				// Only add the integration if the source is from a branch we care about.
 				// [0] == branch name, [1] == relative path in the branch.
-				std::array<std::string, 2> fromBranchPath = splitBranchPath(stripBasePath(fileData.GetFromDepotFile()));
-				if (
-				    !fromBranchPath[0].empty()
-				    && !fromBranchPath[1].empty()
-
+				const Branch* fromBranch = getBranchFor(stripBasePath(fileData.GetFromDepotFile()));
+				if (fromBranch != nullptr
 				    // Can't have source and target be pointing to the same branch; that's not
 				    // a branch operation in the Git sense.
-				    && fromBranchPath[0] != branchPath[0])
+				    && fromBranch->gitAlias != branch->gitAlias)
 				{
 					// This is a valid integrate from a known source to a known target branch.
-					branchMap.addMerge(fromBranchPath[0], branchPath[0], fileData);
+					branchMap.addMerge(fromBranch->gitAlias, branch->gitAlias, branch->depotBranchPath, fileData);
 					needsHandling = false;
 				}
 			}
 			if (needsHandling)
 			{
 				// Either not a valid integrate, or a normal operation.
-				branchMap.addTarget(branchPath[0], fileData);
+				branchMap.addTarget(branch->gitAlias, branch->depotBranchPath, fileData);
 			}
 		}
 		else
 		{
 			// It's a non-branching setup.
 			// Make sure the relative path is set.
 			fileData.SetRelativeDepotPath(relativeDepotPath);
-			branchMap.addTarget(EMPTY_STRING, fileData);
+			branchMap.addTarget(EMPTY_STRING, EMPTY_STRING, fileData);
 		}
 	}
 	return branchMap.createChangedFileGroups();

p4-fusion/branch_set.h

@@ -28,6 +28,7 @@ struct BranchedFileGroup
 	// These branch names will be the Git branch names.
 	std::string sourceBranch;
 	std::string targetBranch;
+	std::string depotBranchPath;
 	bool hasSource;
 	std::vector<FileData> files;
 };
@@ -58,9 +59,7 @@ struct Branch
 
 	Branch(const std::string& branch, const std::string& alias);
 
-	// splitBranchPath If the relativeDepotPath matches, returns {branch alias, branch file path}.
-	//   Otherwise, returns {"", ""}
-	std::array<std::string, 2> SplitBranchPath(const std::string& relativeDepotPath) const;
+	bool IsInBranch(const std::string& relativeDepotPath) const;
 };
 
 // A singular view on the branches and a base view (acts as a filter to trim down affected files).
@@ -84,9 +83,7 @@ struct BranchSet
 	// stripBasePath remove the base path from the depot path, or "" if not in the base path.
 	std::string stripBasePath(const std::string& depotPath) const;
 
-	// splitBranchPath extract the branch name and path under the branch (no leading '/' on the path)
-	//    relativeDepotPath - already stripped from running stripBasePath.
-	std::array<std::string, 2> splitBranchPath(const std::string& relativeDepotPath) const;
+	const Branch* getBranchFor(const std::string& relativeDepotPath) const;
 
 	bool matchesExcludes(const std::string& depotPath) const;
 
@@ -107,6 +104,8 @@ struct BranchSet
 
 	int Count() const { return m_branches.size(); };
 
+	const std::vector<Branch>& GetBranches() const { return m_branches; };
+
 	// ParseAffectedFiles create collections of merges and commits.
 	// Breaks up the files into those that are within the view, with each item in the
 	// list is its own target Git branch.

p4-fusion/git_api.cc

@@ -62,7 +62,7 @@ GitAPI::~GitAPI()
 	git_libgit2_shutdown();
 }
 
-bool GitAPI::IsRepositoryClonedFrom(const std::string& depotPath)
+std::string GitAPI::GetDepotPathFromLastCommit() const
 {
 	git_oid oid;
 	GIT2(git_reference_name_to_id(&oid, m_Repo, "HEAD"));
@@ -73,11 +73,11 @@ bool GitAPI::IsRepositoryClonedFrom(const std::string& depotPath)
 	std::string message = git_commit_message(headCommit);
 	size_t depotPathStart = message.find("depot-paths = \"") + 15;
 	size_t depotPathEnd = message.find("\": change") - 1;
-	std::string repoDepotPath = message.substr(depotPathStart, depotPathEnd - depotPathStart + 1) + "...";
+	std::string repoDepotPath = message.substr(depotPathStart, depotPathEnd - depotPathStart + 1);
 
 	git_commit_free(headCommit);
 
-	return repoDepotPath == depotPath;
+	return repoDepotPath;
 }
 
 void GitAPI::OpenRepository(const std::string& repoPath)
@@ -363,8 +363,7 @@ std::string GitAPI::Commit(
 	git_signature* author = nullptr;
 	GIT2(git_signature_new(&author, user.c_str(), email.c_str(), timestamp, timezone));
 
-	// -3 to remove the trailing "..."
-	std::string commitMsg = desc + "\n[p4-fusion: depot-paths = \"" + depotPath.substr(0, depotPath.size() - 3) + "\": change = " + cl + "]";
+	std::string commitMsg = desc + "\n[p4-fusion: depot-paths = \"" + depotPath + "\": change = " + cl + "]";
 
 	// Find the parent commits.
 	// Order is very important.

p4-fusion/git_api.h

@@ -36,7 +36,7 @@ class GitAPI
 	void OpenRepository(const std::string& repoPath);
 
 	bool IsHEADExists();
-	bool IsRepositoryClonedFrom(const std::string& depotPath);
+	std::string GetDepotPathFromLastCommit() const;
 	std::string DetectLatestCL();
 
 	git_oid CreateBlob(const std::vector<char>& data);

p4-fusion/main.cc

@@ -350,10 +350,32 @@ int Main(int argc, char** argv)
 	std::string resumeFromCL;
 	if (git.IsHEADExists())
 	{
-		if (!git.IsRepositoryClonedFrom(depotPath))
+		if (branchSet.HasMergeableBranch())
 		{
-			ERR("Git repository at " << srcPath << " was not initially cloned with depotPath = " << depotPath << ". Exiting.");
-			return 1;
+			bool foundMatchingBranch = false;
+			for (const Branch& branch : branchSet.GetBranches())
+			{
+				const std::string branchDepotPath = depotPath.substr(0, depotPath.size() - 3) + branch.depotBranchPath;
+
+				if (git.GetDepotPathFromLastCommit() == branchDepotPath)
+				{
+					foundMatchingBranch = true;
+					break;
+				}
+			}
+			if (!foundMatchingBranch)
+			{
+				ERR("Git repository at " << srcPath << " was not initially cloned with any provided branches. Exiting.");
+				return 1;
+			}
+		}
+		else
+		{
+			if (git.GetDepotPathFromLastCommit() + "..." != depotPath)
+			{
+				ERR("Git repository at " << srcPath << " was not initially cloned with depotPath = \"" << depotPath << "\". Exiting.");
+				return 1;
+			}
 		}
 
 		resumeFromCL = git.DetectLatestCL();
@@ -362,7 +384,25 @@ int Main(int argc, char** argv)
 
 	PRINT("Requesting changelists to convert from the Perforce server");
 
-	std::vector<ChangeList> changes = std::move(p4.Changes(depotPath, resumeFromCL, maxChanges).GetChanges());
+	std::vector<ChangeList> changes;
+	if (branchSet.HasMergeableBranch())
+	{
+		for (const Branch& branch : branchSet.GetBranches())
+		{
+			const std::string branchDepotPath = depotPath.substr(0, depotPath.size() - 3) + branch.depotBranchPath + "/...";
+
+			std::vector<ChangeList> branchChanges = std::move(p4.Changes(branchDepotPath, resumeFromCL, maxChanges).GetChanges());
+			for (ChangeList& cl : branchChanges)
+			{
+				changes.push_back(std::move(cl));
+			}
+		}
+		std::sort(changes.begin(), changes.end());
+	}
+	else
+	{
+		changes = std::move(p4.Changes(depotPath, resumeFromCL, maxChanges).GetChanges());
+	}
 
 	if (streamMappings)
 	{
@@ -382,6 +422,17 @@ int Main(int argc, char** argv)
 		}
 	}
 
+	// Check if all changelists appear only once
+	std::set<std::string> uniqueChangeListNumbers;
+	for (const ChangeList& cl : changes) {
+		uniqueChangeListNumbers.insert(cl.number);
+	}
+	if (uniqueChangeListNumbers.size() != changes.size())
+	{
+		ERR("Changelists appear more than once. Exiting.");
+		return 1;
+	}
+
 	// Return early if we have no work to do
 	if (changes.empty())
 	{
@@ -494,7 +545,17 @@ int Main(int argc, char** argv)
 				mergeFrom = branchGroup.sourceBranch;
 			}
 
-			std::string commitSHA = git.Commit(depotPath,
+			std::string depotPathString = branchSet.HasMergeableBranch() ? branchGroup.depotBranchPath : depotPath;
+			if (STDHelpers::EndsWith(depotPathString, "/..."))
+			{
+				depotPathString = depotPathString.substr(0, depotPathString.size() - 3);
+			}
+			if (!STDHelpers::StartsWith(depotPathString, "//"))
+			{
+				depotPathString = "//" + depotPathString;
+			}
+
+			std::string commitSHA = git.Commit(depotPathString,
 			    cl.number,
 			    fullName,
 			    email,