From 0eba0699fde7d9c0f13ddab1ad1e582881eaf7c8 Mon Sep 17 00:00:00 2001 From: Ian Hattendorf Date: Thu, 16 May 2019 14:25:52 -0700 Subject: [PATCH 1/4] Implement faster file history walk Co-authored-by: Tyler Ang-Wanek --- generate/input/libgit2-supplement.json | 4 +- .../manual/revwalk/file_history_walk.cc | 522 +++++++++++------- 2 files changed, 338 insertions(+), 188 deletions(-) diff --git a/generate/input/libgit2-supplement.json b/generate/input/libgit2-supplement.json index 42d9260b2..1b3e66a21 100644 --- a/generate/input/libgit2-supplement.json +++ b/generate/input/libgit2-supplement.json @@ -434,11 +434,11 @@ }, { "name": "max_count", - "type": "int" + "type": "unsigned int" }, { "name": "out", - "type": "std::vector< std::pair > *> *" + "type": "std::vector *" }, { "name": "walk", diff --git a/generate/templates/manual/revwalk/file_history_walk.cc b/generate/templates/manual/revwalk/file_history_walk.cc index c70b6856c..555149e13 100644 --- a/generate/templates/manual/revwalk/file_history_walk.cc +++ b/generate/templates/manual/revwalk/file_history_walk.cc @@ -1,3 +1,182 @@ +// Note: commit is not owned by this class (must be freed elsewhere) +class FileHistoryEvent { +public: + FileHistoryEvent( + git_delta_t inputType, + bool inputExistsInCurrentTree, + bool inputIsMergeCommit, + git_commit *inputCommit, + const char *inputFrom, + const char *inputTo + ): + type(inputType), + existsInCurrentTree(inputExistsInCurrentTree), + isMergeCommit(inputIsMergeCommit), + from(inputFrom == NULL ? NULL : strdup(inputFrom)), + to(inputTo == NULL ? NULL : strdup(inputTo)), + commit(inputCommit) + { + if (inputCommit != NULL) { + const int error = git_commit_dup(&commit, inputCommit); + assert(error == GIT_OK); + } + } + + ~FileHistoryEvent() { + if (commit != NULL) { + git_commit_free(commit); + } + } + + v8::Local toJavascript() { + v8::Local historyEntry = Nan::New(); + v8::Local owners = Nan::New(1); + Nan::Set( + owners, + Nan::New(owners->Length()), + GitRepository::New( + git_commit_owner(commit), + true + )->ToObject() + ); + Nan::Set(historyEntry, Nan::New("commit").ToLocalChecked(), GitCommit::New(commit, true, owners)); + commit = NULL; + Nan::Set(historyEntry, Nan::New("status").ToLocalChecked(), Nan::New(type)); + Nan::Set(historyEntry, Nan::New("isMergeCommit").ToLocalChecked(), Nan::New(isMergeCommit)); + if (type == GIT_DELTA_RENAMED) { + if (existsInCurrentTree) { + Nan::Set(historyEntry, Nan::New("oldName").ToLocalChecked(), Nan::New(from).ToLocalChecked()); + } else { + Nan::Set(historyEntry, Nan::New("newName").ToLocalChecked(), Nan::New(to).ToLocalChecked()); + } + } + return historyEntry; + } + + static int buildHistoryEvent( + FileHistoryEvent **fileHistoryEvent, + git_repository *repo, + git_commit *currentCommit, + git_tree *currentTree, + git_tree *parentTree, + const char *filePath + ) { + int errorCode; + git_tree_entry *currentEntry; + if (git_tree_entry_bypath(¤tEntry, currentTree, filePath) != GIT_OK) { + currentEntry = NULL; + } + git_tree_entry *parentEntry; + if (git_tree_entry_bypath(&parentEntry, parentTree, filePath) != GIT_OK) { + parentEntry = NULL; + } + + if (!currentEntry && !parentEntry) { + *fileHistoryEvent = new FileHistoryEvent(GIT_DELTA_UNMODIFIED, false, false, currentCommit, NULL, NULL); + return GIT_OK; + } + + // The filePath was added + if (currentEntry && !parentEntry) { + git_diff *diff; + if ((errorCode = git_diff_tree_to_tree(&diff, repo, parentTree, currentTree, NULL)) != GIT_OK) { + git_tree_entry_free(currentEntry); + return errorCode; + } + if ((errorCode = git_diff_find_similar(diff, NULL)) != GIT_OK) { + git_diff_free(diff); + git_tree_entry_free(currentEntry); + return errorCode; + } + const size_t numDeltas = git_diff_num_deltas(diff); + for (size_t i = 0; i < numDeltas; ++i) { + const git_diff_delta *delta = git_diff_get_delta(diff, i); + if (delta->new_file.path != NULL && std::strcmp(delta->new_file.path, filePath) == 0) { + if (delta->status == GIT_DELTA_RENAMED + || (delta->old_file.path != NULL && std::strcmp(delta->old_file.path, filePath) != 0)) { + *fileHistoryEvent = new FileHistoryEvent( + GIT_DELTA_RENAMED, + true, + false, + currentCommit, + delta->old_file.path, + NULL + ); + git_diff_free(diff); + git_tree_entry_free(currentEntry); + return GIT_OK; + } + break; + } + } + git_diff_free(diff); + git_tree_entry_free(currentEntry); + + *fileHistoryEvent = new FileHistoryEvent(GIT_DELTA_ADDED, true, false, currentCommit, NULL, NULL); + return GIT_OK; + } + + // The filePath was deleted + if (!currentEntry && parentEntry) { + git_diff *diff; + if ((errorCode = git_diff_tree_to_tree(&diff, repo, parentTree, currentTree, NULL)) != GIT_OK) { + git_tree_entry_free(parentEntry); + return errorCode; + } + if ((errorCode = git_diff_find_similar(diff, NULL)) != GIT_OK) { + git_diff_free(diff); + git_tree_entry_free(parentEntry); + return errorCode; + } + const size_t numDeltas = git_diff_num_deltas(diff); + for (size_t i = 0; i < numDeltas; ++i) { + const git_diff_delta *delta = git_diff_get_delta(diff, i); + if (delta->old_file.path != NULL && std::strcmp(delta->old_file.path, filePath) == 0) { + if (delta->status == GIT_DELTA_RENAMED + || (delta->new_file.path != NULL && std::strcmp(delta->new_file.path, filePath) != 0)) { + *fileHistoryEvent = new FileHistoryEvent( + GIT_DELTA_RENAMED, + false, + false, + currentCommit, + NULL, + delta->new_file.path + ); + git_diff_free(diff); + git_tree_entry_free(parentEntry); + return GIT_OK; + } + break; + } + } + git_diff_free(diff); + git_tree_entry_free(parentEntry); + + *fileHistoryEvent = new FileHistoryEvent(GIT_DELTA_DELETED, false, false, currentCommit, NULL, NULL); + return GIT_OK; + } + + if (git_oid_cmp(git_tree_entry_id(currentEntry), git_tree_entry_id(parentEntry)) != 0 + || git_tree_entry_filemode(currentEntry) != git_tree_entry_filemode(parentEntry) + ) { + git_tree_entry_free(parentEntry); + git_tree_entry_free(currentEntry); + *fileHistoryEvent = new FileHistoryEvent(GIT_DELTA_MODIFIED, true, false, currentCommit, NULL, NULL); + return GIT_OK; + } + + *fileHistoryEvent = new FileHistoryEvent(GIT_DELTA_UNMODIFIED, true, false, currentCommit, NULL, NULL); + git_tree_entry_free(parentEntry); + git_tree_entry_free(currentEntry); + return GIT_OK; + } + + git_delta_t type; + bool existsInCurrentTree, isMergeCommit; + const char *from, *to; + git_commit *commit; +}; + NAN_METHOD(GitRevwalk::FileHistoryWalk) { if (info.Length() == 0 || !info[0]->IsString()) { @@ -19,7 +198,7 @@ NAN_METHOD(GitRevwalk::FileHistoryWalk) String::Utf8Value from_js_file_path(info[0]->ToString()); baton->file_path = strdup(*from_js_file_path); baton->max_count = Nan::To(info[1]).FromJust(); - baton->out = new std::vector< std::pair > *>; + baton->out = new std::vector; baton->out->reserve(baton->max_count); baton->walk = Nan::ObjectWrap::Unwrap(info.This())->GetValue(); @@ -34,251 +213,222 @@ NAN_METHOD(GitRevwalk::FileHistoryWalk) void GitRevwalk::FileHistoryWalkWorker::Execute() { git_repository *repo = git_revwalk_repository(baton->walk); - git_oid *nextOid = (git_oid *)malloc(sizeof(git_oid)); + git_oid currentOid; git_error_clear(); for ( - unsigned int i = 0; - i < baton->max_count && (baton->error_code = git_revwalk_next(nextOid, baton->walk)) == GIT_OK; - ++i + unsigned int revwalkIterations = 0; + revwalkIterations < baton->max_count && (baton->error_code = git_revwalk_next(¤tOid, baton->walk)) == GIT_OK; + ++revwalkIterations ) { - // check if this commit has the file - git_commit *nextCommit; - - if ((baton->error_code = git_commit_lookup(&nextCommit, repo, nextOid)) != GIT_OK) { + git_commit *currentCommit; + if ((baton->error_code = git_commit_lookup(¤tCommit, repo, ¤tOid)) != GIT_OK) { break; } - git_tree *thisTree, *parentTree; - if ((baton->error_code = git_commit_tree(&thisTree, nextCommit)) != GIT_OK) { - git_commit_free(nextCommit); + git_tree *currentTree; + if ((baton->error_code = git_commit_tree(¤tTree, currentCommit)) != GIT_OK) { + git_commit_free(currentCommit); break; } - git_diff *diffs; - git_diff_options opts = GIT_DIFF_OPTIONS_INIT; - char *file_path = strdup(baton->file_path); - opts.pathspec.strings = &file_path; - opts.pathspec.count = 1; - git_commit *parent; - unsigned int parents = git_commit_parentcount(nextCommit); - if (parents > 1) { - git_commit_free(nextCommit); - continue; - } else if (parents == 1) { - if ((baton->error_code = git_commit_parent(&parent, nextCommit, 0)) != GIT_OK) { - git_commit_free(nextCommit); - break; + const unsigned int parentCount = git_commit_parentcount(currentCommit); + if (parentCount == 0) { + git_tree_entry* entry; + if (git_tree_entry_bypath(&entry, currentTree, baton->file_path) == GIT_OK) { + baton->out->push_back(new FileHistoryEvent(GIT_DELTA_ADDED, false, false, currentCommit, NULL, NULL)); + git_tree_entry_free(entry); } - if ( - (baton->error_code = git_commit_tree(&parentTree, parent)) != GIT_OK || - (baton->error_code = git_diff_tree_to_tree(&diffs, repo, parentTree, thisTree, &opts)) != GIT_OK - ) { - git_commit_free(nextCommit); - git_commit_free(parent); + git_commit_free(currentCommit); + git_tree_free(currentTree); + continue; + } + + if (parentCount == 1) { + git_commit *parentCommit; + if ((baton->error_code = git_commit_parent(&parentCommit, currentCommit, 0)) != GIT_OK) { + git_commit_free(currentCommit); + git_tree_free(currentTree); break; } - } else { - if ((baton->error_code = git_diff_tree_to_tree(&diffs, repo, NULL, thisTree, &opts)) != GIT_OK) { - git_commit_free(nextCommit); + + git_tree *parentTree; + if ((baton->error_code = git_commit_tree(&parentTree, parentCommit)) != GIT_OK) { + git_commit_free(currentCommit); + git_commit_free(parentCommit); + git_tree_free(currentTree); break; } - } - free(file_path); - opts.pathspec.strings = NULL; - opts.pathspec.count = 0; - bool flag = false; - bool doRenamedPass = false; - unsigned int numDeltas = git_diff_num_deltas(diffs); - for (unsigned int j = 0; j < numDeltas; ++j) { - git_patch *nextPatch; - baton->error_code = git_patch_from_diff(&nextPatch, diffs, j); - - if (baton->error_code < GIT_OK) { + FileHistoryEvent *fileHistoryEvent; + if ((baton->error_code = FileHistoryEvent::buildHistoryEvent( + &fileHistoryEvent, + repo, + currentCommit, + currentTree, + parentTree, + baton->file_path + )) != GIT_OK) { + git_commit_free(currentCommit); + git_commit_free(parentCommit); + git_tree_free(currentTree); + git_tree_free(parentTree); break; } - if (nextPatch == NULL) { - continue; + if (fileHistoryEvent->type != GIT_DELTA_UNMODIFIED) { + baton->out->push_back(fileHistoryEvent); } - const git_diff_delta *delta = git_patch_get_delta(nextPatch); - bool isEqualOldFile = !strncmp(delta->old_file.path, baton->file_path, strlen(baton->file_path)); - bool isEqualNewFile = !strncmp(delta->new_file.path, baton->file_path, strlen(baton->file_path)); + git_commit_free(currentCommit); + git_commit_free(parentCommit); + git_tree_free(currentTree); + git_tree_free(parentTree); + continue; + } - if (isEqualNewFile) { - if (delta->status == GIT_DELTA_ADDED || delta->status == GIT_DELTA_DELETED) { - doRenamedPass = true; - break; - } - std::pair > *historyEntry; - if (!isEqualOldFile) { - historyEntry = new std::pair >( - nextCommit, - std::pair(strdup(delta->old_file.path), delta->status) - ); - } else { - historyEntry = new std::pair >( - nextCommit, - std::pair(strdup(delta->new_file.path), delta->status) - ); - } - baton->out->push_back(historyEntry); - flag = true; + std::pair firstMatchingParentIndex(false, 0); + bool fileExistsInCurrent = false, fileExistsInSomeParent = false; + for (unsigned int parentIndex = 0; parentIndex < parentCount; ++parentIndex) { + git_commit *parentCommit; + if ((baton->error_code = git_commit_parent(&parentCommit, currentCommit, parentIndex)) != GIT_OK) { + break; } - git_patch_free(nextPatch); - - if (flag) { + git_tree *parentTree; + if ((baton->error_code = git_commit_tree(&parentTree, parentCommit)) != GIT_OK) { + git_commit_free(parentCommit); break; } - } - if (doRenamedPass) { - git_diff_free(diffs); + FileHistoryEvent *fileHistoryEvent; + if ((baton->error_code = FileHistoryEvent::buildHistoryEvent( + &fileHistoryEvent, + repo, + currentCommit, + currentTree, + parentTree, + baton->file_path + )) != GIT_OK) { + git_tree_free(parentTree); + git_commit_free(parentCommit); + break; + } - if (parents == 1) { - if ((baton->error_code = git_diff_tree_to_tree(&diffs, repo, parentTree, thisTree, NULL)) != GIT_OK) { - git_commit_free(nextCommit); + switch (fileHistoryEvent->type) { + case GIT_DELTA_ADDED: { + fileExistsInCurrent = true; break; } - if ((baton->error_code = git_diff_find_similar(diffs, NULL)) != GIT_OK) { - git_commit_free(nextCommit); - break; - } - } else { - if ((baton->error_code = git_diff_tree_to_tree(&diffs, repo, NULL, thisTree, NULL)) != GIT_OK) { - git_commit_free(nextCommit); + case GIT_DELTA_MODIFIED: { + fileExistsInCurrent = true; + fileExistsInSomeParent = true; break; } - if((baton->error_code = git_diff_find_similar(diffs, NULL)) != GIT_OK) { - git_commit_free(nextCommit); + case GIT_DELTA_DELETED: { + fileExistsInSomeParent = true; break; } - } - - flag = false; - numDeltas = git_diff_num_deltas(diffs); - for (unsigned int j = 0; j < numDeltas; ++j) { - git_patch *nextPatch; - baton->error_code = git_patch_from_diff(&nextPatch, diffs, j); - - if (baton->error_code < GIT_OK) { + case GIT_DELTA_RENAMED: { + if (fileHistoryEvent->existsInCurrentTree) { + fileExistsInCurrent = true; + } else { + fileExistsInSomeParent = true; + } break; } - - if (nextPatch == NULL) { - continue; - } - - const git_diff_delta *delta = git_patch_get_delta(nextPatch); - bool isEqualOldFile = !strncmp(delta->old_file.path, baton->file_path, strlen(baton->file_path)); - bool isEqualNewFile = !strncmp(delta->new_file.path, baton->file_path, strlen(baton->file_path)); - int oldLen = strlen(delta->old_file.path); - int newLen = strlen(delta->new_file.path); - char *outPair = new char[oldLen + newLen + 2]; - strcpy(outPair, delta->new_file.path); - outPair[newLen] = '\n'; - outPair[newLen + 1] = '\0'; - strcat(outPair, delta->old_file.path); - - if (isEqualNewFile) { - std::pair > *historyEntry; - if (!isEqualOldFile || delta->status == GIT_DELTA_RENAMED) { - historyEntry = new std::pair >( - nextCommit, - std::pair(strdup(outPair), delta->status) - ); - } else { - historyEntry = new std::pair >( - nextCommit, - std::pair(strdup(delta->new_file.path), delta->status) - ); + case GIT_DELTA_UNMODIFIED: { + if (fileHistoryEvent->existsInCurrentTree) { + fileExistsInCurrent = true; + fileExistsInSomeParent = true; } - baton->out->push_back(historyEntry); - flag = true; - } else if (isEqualOldFile) { - std::pair > *historyEntry; - historyEntry = new std::pair >( - nextCommit, - std::pair(strdup(outPair), delta->status) - ); - baton->out->push_back(historyEntry); - flag = true; + firstMatchingParentIndex = std::make_pair(true, parentIndex); + break; } - - delete[] outPair; - - git_patch_free(nextPatch); - - if (flag) { + default: { break; } } - } - git_diff_free(diffs); + delete fileHistoryEvent; - if (!flag && nextCommit != NULL) { - git_commit_free(nextCommit); + if (firstMatchingParentIndex.first) { + git_commit_free(parentCommit); + git_tree_free(parentTree); + break; + } } if (baton->error_code != GIT_OK) { + git_tree_free(currentTree); + git_commit_free(currentCommit); break; } - } - free(nextOid); + if (!firstMatchingParentIndex.first) { + assert(fileExistsInCurrent || fileExistsInSomeParent); + git_delta_t mergeType = GIT_DELTA_UNREADABLE; // It will never result in this case because of the assertion above. + if (fileExistsInCurrent && fileExistsInSomeParent) { + mergeType = GIT_DELTA_MODIFIED; + } else if (fileExistsInCurrent) { + mergeType = GIT_DELTA_ADDED; + } else if (fileExistsInSomeParent) { + mergeType = GIT_DELTA_DELETED; + } - if (baton->error_code != GIT_OK) { - if (baton->error_code != GIT_ITEROVER) { - baton->error = git_error_dup(git_error_last()); + FileHistoryEvent *fileHistoryEvent = new FileHistoryEvent( + mergeType, + mergeType != GIT_DELTA_DELETED, + true, + currentCommit, + NULL, + NULL + ); + baton->out->push_back(fileHistoryEvent); + git_tree_free(currentTree); + git_commit_free(currentCommit); + continue; + } - while(!baton->out->empty()) - { - std::pair > *pairToFree = baton->out->back(); - baton->out->pop_back(); - git_commit_free(pairToFree->first); - free(pairToFree->second.first); - free(pairToFree); + assert(firstMatchingParentIndex.first); + for (unsigned int parentIndex = 0; parentIndex < parentCount; ++parentIndex) { + if (parentIndex == firstMatchingParentIndex.second) { + continue; } - delete baton->out; + const git_oid *parentOid = git_commit_parent_id(currentCommit, parentIndex); + assert(parentOid != NULL); + git_revwalk_hide(baton->walk, parentOid); + } + git_commit_free(currentCommit); + git_tree_free(currentTree); + } - baton->out = NULL; + if (baton->error_code != GIT_OK && baton->error_code != GIT_ITEROVER) { + // Something went wrong in our loop, discard everything in the async worker + for (unsigned int i = 0; i < baton->out->size(); ++i) { + delete static_cast(baton->out->at(i)); } - } else { - baton->error_code = GIT_OK; + delete baton->out; + baton->out = NULL; + baton->error = git_error_dup(git_error_last()); } } void GitRevwalk::FileHistoryWalkWorker::HandleOKCallback() { if (baton->out != NULL) { - unsigned int size = baton->out->size(); - Local result = Nan::New(size); + const unsigned int size = baton->out->size(); + v8::Local result = Nan::New(size); for (unsigned int i = 0; i < size; i++) { - Local historyEntry = Nan::New(); - std::pair > *batonResult = baton->out->at(i); - Nan::Set(historyEntry, Nan::New("commit").ToLocalChecked(), GitCommit::New(batonResult->first, true)); - Nan::Set(historyEntry, Nan::New("status").ToLocalChecked(), Nan::New(batonResult->second.second)); - if (batonResult->second.second == GIT_DELTA_RENAMED) { - char *namePair = batonResult->second.first; - char *split = strchr(namePair, '\n'); - *split = '\0'; - char *oldName = split + 1; - - Nan::Set(historyEntry, Nan::New("oldName").ToLocalChecked(), Nan::New(oldName).ToLocalChecked()); - Nan::Set(historyEntry, Nan::New("newName").ToLocalChecked(), Nan::New(namePair).ToLocalChecked()); - } - Nan::Set(result, Nan::New(i), historyEntry); - - free(batonResult->second.first); - free(batonResult); + FileHistoryEvent *batonResult = static_cast(baton->out->at(i)); + Nan::Set(result, Nan::New(i), batonResult->toJavascript()); + delete batonResult; } - Local argv[2] = { + Nan::Set(result, Nan::New("reachedEndOfHistory").ToLocalChecked(), Nan::New(baton->error_code == GIT_ITEROVER)); + + v8::Local argv[2] = { Nan::Null(), result }; @@ -289,7 +439,7 @@ void GitRevwalk::FileHistoryWalkWorker::HandleOKCallback() } if (baton->error) { - Local err; + v8::Local err; if (baton->error->message) { err = Nan::Error(baton->error->message)->ToObject(); } else { @@ -297,7 +447,7 @@ void GitRevwalk::FileHistoryWalkWorker::HandleOKCallback() } err->Set(Nan::New("errno").ToLocalChecked(), Nan::New(baton->error_code)); err->Set(Nan::New("errorFunction").ToLocalChecked(), Nan::New("Revwalk.fileHistoryWalk").ToLocalChecked()); - Local argv[1] = { + v8::Local argv[1] = { err }; callback->Call(1, argv, async_resource); @@ -311,10 +461,10 @@ void GitRevwalk::FileHistoryWalkWorker::HandleOKCallback() } if (baton->error_code < 0) { - Local err = Nan::Error("Method next has thrown an error.")->ToObject(); + v8::Local err = Nan::Error("Method next has thrown an error.")->ToObject(); err->Set(Nan::New("errno").ToLocalChecked(), Nan::New(baton->error_code)); err->Set(Nan::New("errorFunction").ToLocalChecked(), Nan::New("Revwalk.fileHistoryWalk").ToLocalChecked()); - Local argv[1] = { + v8::Local argv[1] = { err }; callback->Call(1, argv, async_resource); From 6231c50ae5064e98775e1eed1babeb3da4ec9c71 Mon Sep 17 00:00:00 2001 From: Ian Hattendorf Date: Thu, 16 May 2019 17:46:25 -0700 Subject: [PATCH 2/4] File history walk: include both new and old file paths when renamed --- generate/templates/manual/revwalk/file_history_walk.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/generate/templates/manual/revwalk/file_history_walk.cc b/generate/templates/manual/revwalk/file_history_walk.cc index 555149e13..24b5ad16f 100644 --- a/generate/templates/manual/revwalk/file_history_walk.cc +++ b/generate/templates/manual/revwalk/file_history_walk.cc @@ -44,9 +44,10 @@ class FileHistoryEvent { Nan::Set(historyEntry, Nan::New("status").ToLocalChecked(), Nan::New(type)); Nan::Set(historyEntry, Nan::New("isMergeCommit").ToLocalChecked(), Nan::New(isMergeCommit)); if (type == GIT_DELTA_RENAMED) { - if (existsInCurrentTree) { + if (from != NULL) { Nan::Set(historyEntry, Nan::New("oldName").ToLocalChecked(), Nan::New(from).ToLocalChecked()); - } else { + } + if (to != NULL) { Nan::Set(historyEntry, Nan::New("newName").ToLocalChecked(), Nan::New(to).ToLocalChecked()); } } @@ -100,7 +101,7 @@ class FileHistoryEvent { false, currentCommit, delta->old_file.path, - NULL + delta->new_file.path ); git_diff_free(diff); git_tree_entry_free(currentEntry); @@ -139,7 +140,7 @@ class FileHistoryEvent { false, false, currentCommit, - NULL, + delta->old_file.path, delta->new_file.path ); git_diff_free(diff); From e3ed916e498c31ccb8a2ed84f44ae27d6d23d4c8 Mon Sep 17 00:00:00 2001 From: Ian Hattendorf Date: Thu, 16 May 2019 17:47:02 -0700 Subject: [PATCH 3/4] File history walk: include merge commit if trees have changed --- test/tests/revwalk.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/tests/revwalk.js b/test/tests/revwalk.js index 6c5865193..702bf35cf 100644 --- a/test/tests/revwalk.js +++ b/test/tests/revwalk.js @@ -234,6 +234,7 @@ describe("Revwalk", function() { }); magicShas = [ + "be6905d459f1b236e44b2445df25aff1783993e9", "4a34168b80fe706f52417106821c9cbfec630e47", "f80e085e3118bbd6aad49dad7c53bdc37088bf9b", "694b2d703a02501f288269bea7d1a5d643a83cc8", From 24cb2bfbb0f886ddfc022e6535756b7dc30a679e Mon Sep 17 00:00:00 2001 From: Ian Hattendorf Date: Mon, 20 May 2019 10:21:42 -0700 Subject: [PATCH 4/4] File history walk: memory management --- generate/templates/manual/revwalk/file_history_walk.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/generate/templates/manual/revwalk/file_history_walk.cc b/generate/templates/manual/revwalk/file_history_walk.cc index 24b5ad16f..49c23445e 100644 --- a/generate/templates/manual/revwalk/file_history_walk.cc +++ b/generate/templates/manual/revwalk/file_history_walk.cc @@ -352,10 +352,10 @@ void GitRevwalk::FileHistoryWalkWorker::Execute() } delete fileHistoryEvent; + git_commit_free(parentCommit); + git_tree_free(parentTree); - if (firstMatchingParentIndex.first) { - git_commit_free(parentCommit); - git_tree_free(parentTree); + if (firstMatchingParentIndex.first) { break; } } @@ -414,6 +414,8 @@ void GitRevwalk::FileHistoryWalkWorker::Execute() baton->out = NULL; baton->error = git_error_dup(git_error_last()); } + free((void *)baton->file_path); + baton->file_path = NULL; } void GitRevwalk::FileHistoryWalkWorker::HandleOKCallback()