diff --git a/src/jamidht/conversation.cpp b/src/jamidht/conversation.cpp index 62659d8f7fe68f54a149089a0bac3833b83e15ce..995e638c9e4413725b33e4720f97e64b8ea1d850 100644 --- a/src/jamidht/conversation.cpp +++ b/src/jamidht/conversation.cpp @@ -404,10 +404,9 @@ Conversation::Impl::convCommitToMap(const ConversationCommit& commit) const if (!extension.empty()) message["fileId"] += "." + extension; } - auto linearizedParent = repository_->linearizedParent(commit.id); message["id"] = commit.id; message["parents"] = parents; - message["linearizedParent"] = linearizedParent ? *linearizedParent : ""; + message["linearizedParent"] = commit.linearized_parent; message["author"] = authorId; message["type"] = type; message["timestamp"] = std::to_string(commit.timestamp); diff --git a/src/jamidht/conversationrepository.cpp b/src/jamidht/conversationrepository.cpp index d6235829c871fa3c99e6cbbca1c8e2575267d050..46282ab7ed7456272c3b09cb3476e973cb6c5dfd 100644 --- a/src/jamidht/conversationrepository.cpp +++ b/src/jamidht/conversationrepository.cpp @@ -119,7 +119,6 @@ public: bool logIfNotFound = false, bool fastLog = false, const std::string& authorUri = "") const; - std::optional<std::string> linearizedParent(const std::string& commitId) const; GitObject fileAtTree(const std::string& path, const GitTree& tree) const; // NOTE! GitDiff needs to be deteleted before repo @@ -1533,20 +1532,21 @@ ConversationRepository::Impl::log(const std::string& from, const std::string& authorUri) const { std::vector<ConversationCommit> commits {}; + git_oid oid, oidFrom, oidMerge; - git_oid oid; + // Note: Start from head to get all merge possibilities and correct linearized parent. auto repo = repository(); - if (!repo) + if (!repo or git_reference_name_to_id(&oid, repo.get(), "HEAD") < 0) { + JAMI_ERR("Cannot get reference for HEAD"); return commits; - if (from.empty()) { - if (git_reference_name_to_id(&oid, repo.get(), "HEAD") < 0) { - JAMI_ERR("Cannot get reference for HEAD"); - return commits; - } - } else { - if (git_oid_fromstr(&oid, from.c_str()) < 0) { - JAMI_ERR("Cannot get reference for commit %s", from.c_str()); - return commits; + } + + if (from != "" && git_oid_fromstr(&oidFrom, from.c_str()) == 0) { + auto isMergeBase = git_merge_base(&oidMerge, repo.get(), &oid, &oidFrom) == 0 + && git_oid_equal(&oidMerge, &oidFrom); + if (!isMergeBase) { + // We're logging a non merged branch, so, take this one instead of HEAD + oid = oidFrom; } } @@ -1559,23 +1559,30 @@ ConversationRepository::Impl::log(const std::string& from, JAMI_DBG("Couldn't init revwalker for conversation %s", id_.c_str()); return commits; } + GitRevWalker walker {walker_ptr, git_revwalk_free}; git_revwalk_sorting(walker.get(), GIT_SORT_TOPOLOGICAL | GIT_SORT_TIME); + auto startLogging = from == ""; for (auto idx = 0u; !git_revwalk_next(&oid, walker.get()); ++idx) { - if (n != 0 && idx == n) { - break; - } git_commit* commit_ptr = nullptr; std::string id = git_oid_tostr_s(&oid); + if (!commits.empty()) { + // Set linearized parent + commits.rbegin()->linearized_parent = id; + } if (git_commit_lookup(&commit_ptr, repo.get(), &oid) < 0) { JAMI_WARN("Failed to look up commit %s", id.c_str()); break; } GitCommit commit {commit_ptr, git_commit_free}; - if (id == to) { + if ((n != 0 && commits.size() == n) || (id == to)) break; - } + + if (!startLogging && from != "" && from == id) + startLogging = true; + if (!startLogging) + continue; const git_signature* sig = git_commit_author(commit.get()); GitAuthor author; @@ -1625,49 +1632,9 @@ ConversationRepository::Impl::log(const std::string& from, git_buf_dispose(&signed_data); cc->timestamp = git_commit_time(commit.get()); } - return commits; } -std::optional<std::string> -ConversationRepository::Impl::linearizedParent(const std::string& commitId) const -{ - git_oid oid; - auto repo = repository(); - if (!repo or git_reference_name_to_id(&oid, repo.get(), "HEAD") < 0) { - JAMI_ERR("Cannot get reference for HEAD"); - return std::nullopt; - } - - git_revwalk* walker_ptr = nullptr; - if (git_revwalk_new(&walker_ptr, repo.get()) < 0 || git_revwalk_push(walker_ptr, &oid) < 0) { - GitRevWalker walker {walker_ptr, git_revwalk_free}; - // This fail can be ok in the case we check if a commit exists before pulling (so can fail - // there). only log if the fail is unwanted. - return std::nullopt; - } - GitRevWalker walker {walker_ptr, git_revwalk_free}; - git_revwalk_sorting(walker.get(), GIT_SORT_TOPOLOGICAL | GIT_SORT_TIME); - - auto ret = false; - for (auto idx = 0u; !git_revwalk_next(&oid, walker.get()); ++idx) { - git_commit* commit_ptr = nullptr; - std::string id = git_oid_tostr_s(&oid); - if (git_commit_lookup(&commit_ptr, repo.get(), &oid) < 0) { - JAMI_WARN("Failed to look up commit %s", id.c_str()); - break; - } - GitCommit commit {commit_ptr, git_commit_free}; - - if (ret) - return id; - if (id == commitId) - ret = true; - } - - return std::nullopt; -} - GitObject ConversationRepository::Impl::fileAtTree(const std::string& path, const GitTree& tree) const { @@ -2927,12 +2894,6 @@ ConversationRepository::validClone() const return pimpl_->validCommits(logN("", 0)); } -std::optional<std::string> -ConversationRepository::linearizedParent(const std::string& commitId) const -{ - return pimpl_->linearizedParent(commitId); -} - void ConversationRepository::removeBranchWith(const std::string& remoteDevice) { diff --git a/src/jamidht/conversationrepository.h b/src/jamidht/conversationrepository.h index b4a73a230c2b1722040d932cf0497b88e864b479..ed26caa82a8e7d2cb6d30234149b70934b4e26ef 100644 --- a/src/jamidht/conversationrepository.h +++ b/src/jamidht/conversationrepository.h @@ -69,6 +69,7 @@ struct ConversationCommit std::vector<uint8_t> signed_content {}; std::vector<uint8_t> signature {}; std::string commit_msg {}; + std::string linearized_parent {}; int64_t timestamp {0}; }; diff --git a/test/unitTest/conversation/conversation.cpp b/test/unitTest/conversation/conversation.cpp index a6214b6328d3837c6472c9153ae15db11b28ba8c..b407317e6e8a072e7df2a0f56b7baeb29b849b97 100644 --- a/test/unitTest/conversation/conversation.cpp +++ b/test/unitTest/conversation/conversation.cpp @@ -1064,14 +1064,11 @@ ConversationTest::testPingPongMessages() [&](const std::string& accountId, const std::string& /* conversationId */, std::map<std::string, std::string> /*message*/) { - if (accountId == bobId) { + if (accountId == bobId) messageBobReceived += 1; - } - if (accountId == aliceId) { + if (accountId == aliceId) messageAliceReceived += 1; - } - if (messageAliceReceived == messageBobReceived) - cv.notify_one(); + cv.notify_one(); })); confHandlers.insert( DRing::exportable_callback<DRing::ConversationSignal::ConversationRequestReceived>( diff --git a/test/unitTest/conversationRepository/conversationRepository.cpp b/test/unitTest/conversationRepository/conversationRepository.cpp index 2d8fb93c29d979628a38a7794dabb0ffe029947e..b688cbe0c125fd65c0796ccd1457b625ba839f3b 100644 --- a/test/unitTest/conversationRepository/conversationRepository.cpp +++ b/test/unitTest/conversationRepository/conversationRepository.cpp @@ -426,6 +426,7 @@ ConversationRepositoryTest::testFetch() // Clone repository auto id1 = repository->commitMessage("Commit 1"); + auto cloned = ConversationRepository::cloneConversation(bobAccount->weak(), aliceDeviceId.toString(), repository->id());