From c2f6d811a07cd627d7b6f34fa6f12bc28ca66d29 Mon Sep 17 00:00:00 2001 From: Tyler Wanek Date: Tue, 9 Feb 2016 15:45:03 -0700 Subject: [PATCH 1/2] Optimize rename detection in historywalk --- .../manual/revwalk/file_history_walk.cc | 39 ++++++++++++++++--- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/generate/templates/manual/revwalk/file_history_walk.cc b/generate/templates/manual/revwalk/file_history_walk.cc index 5bf53e394..53c1202fc 100644 --- a/generate/templates/manual/revwalk/file_history_walk.cc +++ b/generate/templates/manual/revwalk/file_history_walk.cc @@ -55,6 +55,10 @@ void GitRevwalk::FileHistoryWalkWorker::Execute() } 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) { @@ -67,19 +71,23 @@ void GitRevwalk::FileHistoryWalkWorker::Execute() } if ( (baton->error_code = git_commit_tree(&parentTree, parent)) != GIT_OK || - (baton->error_code = git_diff_tree_to_tree(&diffs, repo, parentTree, thisTree, NULL)) != 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); break; } } else { - if ((baton->error_code = git_diff_tree_to_tree(&diffs, repo, NULL, thisTree, NULL)) != GIT_OK) { + if ((baton->error_code = git_diff_tree_to_tree(&diffs, repo, NULL, thisTree, &opts)) != GIT_OK) { git_commit_free(nextCommit); 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); @@ -127,10 +135,29 @@ void GitRevwalk::FileHistoryWalkWorker::Execute() } } - if ( - doRenamedPass && - (baton->error_code = git_diff_find_similar(diffs, NULL)) == GIT_OK - ) { + if (doRenamedPass) { + git_diff_free(diffs); + + if (parents == 1) { + if ((baton->error_code = git_diff_tree_to_tree(&diffs, repo, parentTree, thisTree, NULL)) != GIT_OK) { + git_commit_free(nextCommit); + 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); + break; + } + if((baton->error_code = git_diff_find_similar(diffs, NULL)) != GIT_OK) { + git_commit_free(nextCommit); + break; + } + } + flag = false; numDeltas = git_diff_num_deltas(diffs); for (unsigned int j = 0; j < numDeltas; ++j) { From d23ba8723dc56d1167a67c9bd80c2d05fba4c4b1 Mon Sep 17 00:00:00 2001 From: Tyler Wanek Date: Tue, 9 Feb 2016 16:35:36 -0700 Subject: [PATCH 2/2] Provide old and new name instead of just altname --- .../manual/revwalk/file_history_walk.cc | 21 ++++++++++++++++--- lib/revwalk.js | 4 +++- test/tests/revwalk.js | 6 ++++-- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/generate/templates/manual/revwalk/file_history_walk.cc b/generate/templates/manual/revwalk/file_history_walk.cc index 53c1202fc..0f511969f 100644 --- a/generate/templates/manual/revwalk/file_history_walk.cc +++ b/generate/templates/manual/revwalk/file_history_walk.cc @@ -175,13 +175,20 @@ void GitRevwalk::FileHistoryWalkWorker::Execute() const git_diff_delta *delta = git_patch_get_delta(nextPatch); bool isEqualOldFile = !strcmp(delta->old_file.path, baton->file_path); bool isEqualNewFile = !strcmp(delta->new_file.path, 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) { historyEntry = new std::pair >( nextCommit, - std::pair(strdup(delta->old_file.path), delta->status) + std::pair(strdup(outPair), delta->status) ); } else { historyEntry = new std::pair >( @@ -195,12 +202,14 @@ void GitRevwalk::FileHistoryWalkWorker::Execute() std::pair > *historyEntry; historyEntry = new std::pair >( nextCommit, - std::pair(strdup(delta->new_file.path), delta->status) + std::pair(strdup(outPair), delta->status) ); baton->out->push_back(historyEntry); flag = true; } + delete[] outPair; + git_patch_free(nextPatch); if (flag) { @@ -255,7 +264,13 @@ void GitRevwalk::FileHistoryWalkWorker::HandleOKCallback() 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) { - Nan::Set(historyEntry, Nan::New("altname").ToLocalChecked(), Nan::New(batonResult->second.first).ToLocalChecked()); + 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); diff --git a/lib/revwalk.js b/lib/revwalk.js index d0a5c653b..d754259c6 100644 --- a/lib/revwalk.js +++ b/lib/revwalk.js @@ -129,7 +129,9 @@ Revwalk.prototype.getCommits = function(count) { * @type {Object} * @property {Commit} commit the commit for this entry * @property {Number} status the status of the file in the commit - * @property {String} altname the other name that is provided when status is + * @property {String} newName the new name that is provided when status is + * renamed + * @property {String} oldName the old name that is provided when status is * renamed */ diff --git a/test/tests/revwalk.js b/test/tests/revwalk.js index b21d59b07..85d708303 100644 --- a/test/tests/revwalk.js +++ b/test/tests/revwalk.js @@ -286,7 +286,8 @@ describe("Revwalk", function() { }) .then(function(results) { assert.equal(results[0].status, NodeGit.Diff.DELTA.RENAMED); - assert.equal(results[0].altname, fileNameA); + assert.equal(results[0].newName, fileNameB); + assert.equal(results[0].oldName, fileNameA); }) .then(function() { var walker = repo.createRevWalk(); @@ -296,7 +297,8 @@ describe("Revwalk", function() { }) .then(function(results) { assert.equal(results[0].status, NodeGit.Diff.DELTA.RENAMED); - assert.equal(results[0].altname, fileNameB); + assert.equal(results[0].newName, fileNameB); + assert.equal(results[0].oldName, fileNameA); }) .then(function() { return fse.remove(repoPath);