diff --git a/generate/input/callbacks.json b/generate/input/callbacks.json index c37a7c8ee..cb9f10079 100644 --- a/generate/input/callbacks.json +++ b/generate/input/callbacks.json @@ -57,24 +57,6 @@ "error": -1 } }, - "git_blob_chunk_cb": { - "args": [ - { - "name": "entry", - "cType": "const git_config_entry *" - }, - { - "name": "payload", - "cType": "void *" - } - ], - "return": { - "type": "int", - "noResults": 1, - "success": 0, - "error": -1 - } - }, "git_checkout_notify_cb": { "args": [ { @@ -326,7 +308,8 @@ "args": [ { "name": "diff_so_far", - "cType": "const git_diff *" + "cType": "const git_diff *", + "ignore": true }, { "name": "delta_to_add", @@ -347,11 +330,13 @@ "success": 0, "error": -1 } - },"git_diff_progress_cb": { + }, + "git_diff_progress_cb": { "args": [ { "name": "diff_so_far", - "cType": "const git_diff *" + "cType": "const git_diff *", + "ignore": true }, { "name": "old_path", @@ -551,7 +536,7 @@ "args": [ { "name": "out", - "cType": "git_repository **", + "cType": "git_remote **", "isReturn": true }, { diff --git a/generate/input/libgit2-supplement.json b/generate/input/libgit2-supplement.json index 30cfc3f1a..7b19afba9 100644 --- a/generate/input/libgit2-supplement.json +++ b/generate/input/libgit2-supplement.json @@ -176,6 +176,28 @@ }, "group": "filter_source" }, + "git_filter_source_repo": { + "args": [ + { + "name": "out", + "type": "git_repository **" + }, + { + "name": "src", + "type": "const git_filter_source *" + } + ], + "isManual": true, + "cFile": "generate/templates/manual/filter_source/repo.cc", + "isAsync": true, + "isPrototypeMethod": true, + "type": "function", + "group": "filter_source", + "return": { + "type": "int", + "isErrorCode": true + } + }, "git_patch_convenient_from_diff": { "args": [ { diff --git a/generate/scripts/generateNativeCode.js b/generate/scripts/generateNativeCode.js index 47a832419..8ba821903 100644 --- a/generate/scripts/generateNativeCode.js +++ b/generate/scripts/generateNativeCode.js @@ -52,6 +52,8 @@ module.exports = function generateNativeCode() { argsInfo: require("../templates/filters/args_info"), arrayTypeToPlainType: require("../templates/filters/array_type_to_plain_type"), asElementPointer: require("../templates/filters/as_element_pointer"), + callbackArgsInfo: require("../templates/filters/callback_args_info"), + callbackArgsCount: require("../templates/filters/callback_args_count"), cppToV8: require("../templates/filters/cpp_to_v8"), defaultValue: require("../templates/filters/default_value"), fieldsInfo: require("../templates/filters/fields_info"), diff --git a/generate/templates/filters/callback_args_count.js b/generate/templates/filters/callback_args_count.js new file mode 100644 index 000000000..26c7762ea --- /dev/null +++ b/generate/templates/filters/callback_args_count.js @@ -0,0 +1,18 @@ +module.exports = function(args) { + if (!args) { + return 0; + } + + return args.reduce( + function(count, arg) { + var shouldCount = !arg.isReturn && + !arg.isSelf && + arg.name !== "payload" && + arg.name !== "self" && + !arg.ignore; + + return shouldCount ? count + 1 : count; + }, + 0 + ); +}; diff --git a/generate/templates/filters/callback_args_info.js b/generate/templates/filters/callback_args_info.js new file mode 100644 index 000000000..a7285c0b8 --- /dev/null +++ b/generate/templates/filters/callback_args_info.js @@ -0,0 +1,27 @@ +module.exports = function(args) { + var result = args.reduce( + function(argList, arg) { + var useArg = !arg.isReturn && + !arg.isSelf && + arg.name !== "payload" && + arg.name !== "self" && + !arg.ignore; + + if (!useArg) { + return argList; + } + + arg.firstArg = argList.length === 0; + argList.push(arg); + + return argList; + }, + [] + ); + + if (result.length) { + result[result.length - 1].lastArg = true; + } + + return result; +}; diff --git a/generate/templates/filters/js_args_count.js b/generate/templates/filters/js_args_count.js index 5be437f41..17a56a1c1 100644 --- a/generate/templates/filters/js_args_count.js +++ b/generate/templates/filters/js_args_count.js @@ -5,11 +5,11 @@ module.exports = function(args) { if (!args) { return 0; } - + for(cArg = 0, jsArg = 0; cArg < args.length; cArg++) { var arg = args[cArg]; - if (!arg.isReturn && !arg.isSelf && !arg.isPayload) { + if (!arg.isReturn && !arg.isSelf) { jsArg++; } } diff --git a/generate/templates/manual/filter_source/repo.cc b/generate/templates/manual/filter_source/repo.cc new file mode 100644 index 000000000..cf9c1a833 --- /dev/null +++ b/generate/templates/manual/filter_source/repo.cc @@ -0,0 +1,90 @@ +// NOTE you may need to occasionally rebuild this method by calling the generators +// if major changes are made to the templates / generator. + +// Due to some garbage collection issues related to submodules and git_filters, we need to clone the repository +// pointer before giving it to a user. + +/* + * @param Repository callback + */ +NAN_METHOD(GitFilterSource::Repo) { + if (info.Length() == 0 || !info[0]->IsFunction()) { + return Nan::ThrowError("Callback is required and must be a Function."); + } + + RepoBaton *baton = new RepoBaton; + + baton->error_code = GIT_OK; + baton->error = NULL; + baton->src = Nan::ObjectWrap::Unwrap(info.This())->GetValue(); + + Nan::Callback *callback = new Nan::Callback(v8::Local::Cast(info[0])); + RepoWorker *worker = new RepoWorker(baton, callback); + + worker->SaveToPersistent("src", info.This()); + + AsyncLibgit2QueueWorker(worker); + return; +} + +void GitFilterSource::RepoWorker::Execute() { + git_error_clear(); + + { + LockMaster lockMaster(true, baton->src); + + git_repository *repo = git_filter_source_repo(baton->src); + baton->error_code = git_repository_open(&repo, git_repository_path(repo)); + + if (baton->error_code == GIT_OK) { + baton->out = repo; + } else if (git_error_last() != NULL) { + baton->error = git_error_dup(git_error_last()); + } + } +} + +void GitFilterSource::RepoWorker::HandleOKCallback() { + if (baton->error_code == GIT_OK) { + v8::Local to; + + if (baton->out != NULL) { + to = GitRepository::New(baton->out, true); + } else { + to = Nan::Null(); + } + + v8::Local argv[2] = {Nan::Null(), to}; + callback->Call(2, argv, async_resource); + } else { + if (baton->error) { + v8::Local err; + if (baton->error->message) { + err = Nan::Error(baton->error->message)->ToObject(); + } else { + err = Nan::Error("Method repo has thrown an error.")->ToObject(); + } + err->Set(Nan::New("errno").ToLocalChecked(), Nan::New(baton->error_code)); + err->Set(Nan::New("errorFunction").ToLocalChecked(), + Nan::New("FilterSource.repo").ToLocalChecked()); + v8::Local argv[1] = {err}; + callback->Call(1, argv, async_resource); + if (baton->error->message) + free((void *)baton->error->message); + free((void *)baton->error); + } else if (baton->error_code < 0) { + v8::Local err = + Nan::Error("Method repo has thrown an error.")->ToObject(); + err->Set(Nan::New("errno").ToLocalChecked(), + Nan::New(baton->error_code)); + err->Set(Nan::New("errorFunction").ToLocalChecked(), + Nan::New("FilterSource.repo").ToLocalChecked()); + v8::Local argv[1] = {err}; + callback->Call(1, argv, async_resource); + } else { + callback->Call(0, NULL, async_resource); + } + } + + delete baton; +} diff --git a/generate/templates/partials/callback_helpers.cc b/generate/templates/partials/callback_helpers.cc index 1abfaa991..7a40aed25 100644 --- a/generate/templates/partials/callback_helpers.cc +++ b/generate/templates/partials/callback_helpers.cc @@ -30,32 +30,27 @@ void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_async(void {% endif %} {% endeach %} - v8::Local argv[{{ cbFunction.args|jsArgsCount }}] = { - {% each cbFunction.args|argsInfo as arg %} - {% if arg | isPayload %} - {%-- payload is always the last arg --%} - // payload is null because we can use closure scope in javascript - Nan::Undefined() - {% elsif arg.isJsArg %} - {% if arg.isEnum %} - Nan::New((int)baton->{{ arg.name }}), - {% elsif arg.isLibgitType %} - {{ arg.cppClassName }}::New(baton->{{ arg.name }}, false), - {% elsif arg.cType == "size_t" %} - // HACK: NAN should really have an overload for Nan::New to support size_t - Nan::New((unsigned int)baton->{{ arg.name }}), - {% elsif arg.cppClassName == 'String' %} - Nan::New(baton->{{ arg.name }}).ToLocalChecked(), - {% else %} - Nan::New(baton->{{ arg.name }}), - {% endif %} + v8::Local argv[{{ cbFunction.args|callbackArgsCount }}] = { + {% each cbFunction.args|callbackArgsInfo as arg %} + {% if not arg.firstArg %}, {% endif %} + {% if arg.isEnum %} + Nan::New((int)baton->{{ arg.name }}) + {% elsif arg.isLibgitType %} + {{ arg.cppClassName }}::New(baton->{{ arg.name }}, false) + {% elsif arg.cType == "size_t" %} + // HACK: NAN should really have an overload for Nan::New to support size_t + Nan::New((unsigned int)baton->{{ arg.name }}) + {% elsif arg.cppClassName == 'String' %} + Nan::New(baton->{{ arg.name }}).ToLocalChecked() + {% else %} + Nan::New(baton->{{ arg.name }}) {% endif %} {% endeach %} }; Nan::TryCatch tryCatch; // TODO This should take an async_resource, but we will need to figure out how to pipe the correct context into this - Nan::MaybeLocal maybeResult = Nan::Call(*callback, {{ cbFunction.args|jsArgsCount }}, argv); + Nan::MaybeLocal maybeResult = Nan::Call(*callback, {{ cbFunction.args|callbackArgsCount }}, argv); v8::Local result; if (!maybeResult.IsEmpty()) { result = maybeResult.ToLocalChecked(); diff --git a/generate/templates/partials/field_accessors.cc b/generate/templates/partials/field_accessors.cc index 6325587d3..222034150 100644 --- a/generate/templates/partials/field_accessors.cc +++ b/generate/templates/partials/field_accessors.cc @@ -179,92 +179,34 @@ return; } - {% each field.args|argsInfo as arg %} - {% if arg.name == "payload" %} - {%-- Do nothing --%} - {% elsif arg.isJsArg %} - {% if arg.cType == "const char *" %} - if (baton->{{ arg.name }} == NULL) { - baton->{{ arg.name }} = ""; - } - {% elsif arg.cppClassName == "String" %} - v8::Local src; - if (baton->{{ arg.name }} == NULL) { - src = Nan::Null(); - } - else { - src = Nan::New(*baton->{{ arg.name }}).ToLocalChecked(); - } - {% endif %} - {% endif %} - {% endeach %} - - {% if field.isSelfReferential %} - {% if field.args|jsArgsCount|subtract 2| setUnsigned == 0 %} - v8::Local *argv = NULL; - {% else %} - v8::Local argv[{{ field.args|jsArgsCount|subtract 2| setUnsigned }}] = { - {% endif %} + {% if field.args|callbackArgsCount == 0 %} + v8::Local *argv = NULL; {% else %} - v8::Local argv[{{ field.args|jsArgsCount }}] = { - {% endif %} - {% each field.args|argsInfo as arg %} - {% if field.isSelfReferential %} - {% if not arg.firstArg %} - {% if field.args|jsArgsCount|subtract 1|or 0 %} - {% if arg.cppClassName == "String" %} - {%-- src is always the last arg --%} - src - {% elsif arg.isJsArg %} - {% if arg.isEnum %} - Nan::New((int)baton->{{ arg.name }}), - {% elsif arg.isLibgitType %} - {{ arg.cppClassName }}::New(baton->{{ arg.name }}, false), - {% elsif arg.cType == "size_t" %} - Nan::New((unsigned int)baton->{{ arg.name }}), - {% elsif arg.name == "payload" %} - {%-- skip, filters should not have a payload --%} - {% else %} - Nan::New(baton->{{ arg.name }}), - {% endif %} - {% endif %} - {% endif %} - {% endif %} - {% else %} - {% if arg.name == "payload" %} - {%-- payload is always the last arg --%} - Nan::New(instance->{{ fields|payloadFor field.name }}) - {% elsif arg.isJsArg %} - {% if arg.isEnum %} - Nan::New((int)baton->{{ arg.name }}), - {% elsif arg.isLibgitType %} - {{ arg.cppClassName }}::New(baton->{{ arg.name }}, false), - {% elsif arg.cType == "size_t" %} - // HACK: NAN should really have an overload for Nan::New to support size_t - Nan::New((unsigned int)baton->{{ arg.name }}), - {% elsif arg.cppClassName == "String" %} - Nan::New(baton->{{ arg.name }}).ToLocalChecked(), - {% else %} - Nan::New(baton->{{ arg.name }}), - {% endif %} + v8::Local argv[{{ field.args|callbackArgsCount }}] = { + {% each field.args|callbackArgsInfo as arg %} + {% if not arg.firstArg %},{% endif %} + {% if arg.isEnum %} + Nan::New((int)baton->{{ arg.name }}) + {% elsif arg.isLibgitType %} + {{ arg.cppClassName }}::New(baton->{{ arg.name }}, false) + {% elsif arg.cType == "size_t" %} + // HACK: NAN should really have an overload for Nan::New to support size_t + Nan::New((unsigned int)baton->{{ arg.name }}) + {% elsif arg.cppClassName == "String" %} + baton->{{ arg.name }} == NULL + ? Nan::EmptyString() + : Nan::New({%if arg.cType | isDoublePointer %}*{% endif %}baton->{{ arg.name }}).ToLocalChecked() + {% else %} + Nan::New(baton->{{ arg.name }}) {% endif %} - {% endif %} - {% endeach %} - {% if not field.isSelfReferential %} - }; - {% elsif field.args|jsArgsCount|subtract 2| setUnsigned > 0 %} + {% endeach %} }; {% endif %} Nan::TryCatch tryCatch; // TODO This should take an async_resource, but we will need to figure out how to pipe the correct context into this - {% if field.isSelfReferential %} - Nan::MaybeLocal maybeResult = Nan::Call(*(instance->{{ field.name }}.GetCallback()), {{ field.args|jsArgsCount|subtract 2| setUnsigned }}, argv); - {% else %} - Nan::MaybeLocal maybeResult = Nan::Call(*(instance->{{ field.name }}.GetCallback()), {{ field.args|jsArgsCount }}, argv); - {% endif %} - + Nan::MaybeLocal maybeResult = Nan::Call(*(instance->{{ field.name }}.GetCallback()), {{ field.args|callbackArgsCount }}, argv); v8::Local result; if (!maybeResult.IsEmpty()) { result = maybeResult.ToLocalChecked(); diff --git a/test/tests/filter.js b/test/tests/filter.js index ef1efc5c0..62aa4e863 100644 --- a/test/tests/filter.js +++ b/test/tests/filter.js @@ -2,6 +2,7 @@ var assert = require("assert"); var fse = require("fs-extra"); var path = require("path"); var local = path.join.bind(path, __dirname); +var garbageCollect = require("../utils/garbage_collect.js"); describe("Filter", function() { var NodeGit = require("../../"); @@ -216,7 +217,7 @@ describe("Filter", function() { }, 0) .then(function(result) { assert.strictEqual(result, NodeGit.Error.CODE.OK); - global.gc(); + garbageCollect(); return fse.writeFile( packageJsonPath, @@ -338,7 +339,8 @@ describe("Filter", function() { return Checkout.head(test.repository, opts); }) .then(function() { - global.gc(); + garbageCollect(); + return Registry.unregister(filterName); }) .then(function(result) { @@ -637,7 +639,7 @@ describe("Filter", function() { ); assert.notStrictEqual(readmeContent, message); fse.writeFileSync(readmePath, "whoa", "utf8"); - global.gc(); + garbageCollect(); var opts = { checkoutStrategy: Checkout.STRATEGY.FORCE, @@ -725,7 +727,7 @@ describe("Filter", function() { cleanup: function() {} }, 0) .then(function(result) { - global.gc(); + garbageCollect(); assert.strictEqual(result, NodeGit.Error.CODE.OK); }) .then(function() { @@ -742,7 +744,7 @@ describe("Filter", function() { ); }) .then(function(oid) { - global.gc(); + garbageCollect(); return test.repository.getHeadCommit(); }) .then(function(commit) { @@ -755,7 +757,7 @@ describe("Filter", function() { postInitializeReadmeContents, "testing commit contents" ); assert.strictEqual(commit.message(), "test commit"); - global.gc(); + garbageCollect(); return commit.getEntry("README.md"); }) @@ -842,7 +844,7 @@ describe("Filter", function() { ); assert.notEqual(packageContent, ""); - global.gc(); + garbageCollect(); return fse.writeFile( packageJsonPath, "Changing content to trigger checkout", @@ -1076,4 +1078,65 @@ describe("Filter", function() { }); }); }); + + describe("FilterSource", function() { + var message = "some new fancy filter"; + + before(function() { + var test = this; + return fse.readFile(readmePath, "utf8") + .then((function(content) { + test.originalReadmeContent = content; + })); + }); + + afterEach(function() { + this.timeout(15000); + return fse.writeFile(readmePath, this.originalReadmeContent); + }); + + it("a FilterSource has an async repo getter", function() { + var test = this; + + return Registry.register(filterName, { + apply: function(to, from, source) { + return source.repo() + .then(function() { + return NodeGit.Error.CODE.PASSTHROUGH; + }); + }, + check: function(source) { + return source.repo() + .then(function() { + return NodeGit.Error.CODE.OK; + }); + } + }, 0) + .then(function(result) { + assert.strictEqual(result, NodeGit.Error.CODE.OK); + }) + .then(function() { + var readmeContent = fse.readFileSync( + packageJsonPath, + "utf-8" + ); + assert.notStrictEqual(readmeContent, message); + + return fse.writeFile( + packageJsonPath, + "Changing content to trigger checkout" + ); + }) + .then(function() { + var opts = { + checkoutStrategy: Checkout.STRATEGY.FORCE, + paths: "package.json" + }; + return Checkout.head(test.repository, opts); + }) + .then(function() { + garbageCollect(); + }); + }); + }); });