From 6be31fe290e0efc8e4e8bb16797bb0a6dc6b2acc Mon Sep 17 00:00:00 2001 From: John Haley Date: Tue, 15 Mar 2016 12:25:05 -0700 Subject: [PATCH 01/33] Bump nan to 2.2.0 This should fix building for node 5.8.0+ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 03a750a6d..19dee3ecb 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "lcov-result-merger": "~1.0.2", "lodash": "~3.10.1", "mocha": "~2.3.4", - "nan": "~2.0.9", + "nan": "^2.2.0", "node-gyp": "~3.0.3", "nw-gyp": "~0.12.4" }, From e9d50fa446394da442080c089b19ea79c0bc60dd Mon Sep 17 00:00:00 2001 From: John Haley Date: Tue, 15 Mar 2016 12:35:14 -0700 Subject: [PATCH 02/33] Use node 5.8.x on CI --- .travis.yml | 2 +- appveyor.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index d10faa0fc..55a1c2ad9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,7 +15,7 @@ env: matrix: - export NODE_VERSION="0.12" - export NODE_VERSION="4.1" - - export NODE_VERSION="5.0" + - export NODE_VERSION="5.8" matrix: fast_finish: true diff --git a/appveyor.yml b/appveyor.yml index 41d4f90ff..337d1250f 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -31,7 +31,7 @@ environment: - nodejs_version: "0.12" # Node.js - nodejs_version: "4.1" - - nodejs_version: "5.0" + - nodejs_version: "5.8" matrix: fast_finish: true From 1cfb51b8bb8de122e16950d3d6d814af0a93d161 Mon Sep 17 00:00:00 2001 From: John Haley Date: Tue, 15 Mar 2016 15:01:57 -0700 Subject: [PATCH 03/33] Skip broken http test --- test/tests/clone.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/tests/clone.js b/test/tests/clone.js index 46830a889..cbb0d1c5d 100644 --- a/test/tests/clone.js +++ b/test/tests/clone.js @@ -27,7 +27,7 @@ describe("Clone", function() { }); }); - it("can clone with http", function() { + it.skip("can clone with http", function() { var test = this; var url = "http://git.tbranyen.com/smart/site-content"; From 26a4f0427360c29fb83426db9686f7b35b0be3a0 Mon Sep 17 00:00:00 2001 From: Kishan Sambhi Date: Tue, 15 Mar 2016 18:52:22 +0000 Subject: [PATCH 04/33] Put the path to node-pre-gyp in quotes in install.js This is to help resolve nodegit/nodegit#950 --- lifecycleScripts/install.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lifecycleScripts/install.js b/lifecycleScripts/install.js index 34989f2ac..c59ac9c5d 100644 --- a/lifecycleScripts/install.js +++ b/lifecycleScripts/install.js @@ -36,7 +36,7 @@ return installPrebuilt(); function installPrebuilt() { console.info("[nodegit] Fetching binary from S3."); var npg = pathForTool("node-pre-gyp"); - return exec(npg + " install --fallback-to-build=false") + return exec('"'+ npg + '" install --fallback-to-build=false') .then( function() { console.info("[nodegit] Completed installation successfully."); From 971527b1449d55419c8790ac5265f5f8a786fd90 Mon Sep 17 00:00:00 2001 From: Kishan Sambhi Date: Tue, 15 Mar 2016 20:29:17 +0000 Subject: [PATCH 05/33] Fix issue where " is required When building on appveyor, the linter exited with 1. To fix this, we are putting an \ before " --- lifecycleScripts/install.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lifecycleScripts/install.js b/lifecycleScripts/install.js index c59ac9c5d..0302d6743 100644 --- a/lifecycleScripts/install.js +++ b/lifecycleScripts/install.js @@ -36,7 +36,7 @@ return installPrebuilt(); function installPrebuilt() { console.info("[nodegit] Fetching binary from S3."); var npg = pathForTool("node-pre-gyp"); - return exec('"'+ npg + '" install --fallback-to-build=false') + return exec("\""+ npg + "\" install --fallback-to-build=false") .then( function() { console.info("[nodegit] Completed installation successfully."); From e404375cc9edcf5f4206f4f8689c824e38c5f8e3 Mon Sep 17 00:00:00 2001 From: Tyler Church Date: Thu, 17 Mar 2016 22:42:00 -0700 Subject: [PATCH 06/33] Stash.apply: Fix a typo --- lib/stash.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/stash.js b/lib/stash.js index 2df42ecca..fbc1ca170 100644 --- a/lib/stash.js +++ b/lib/stash.js @@ -32,7 +32,7 @@ Stash.apply = function(repo, index, options) { if (checkoutOptions) { options.checkoutOptions = - normalizeOptions(checkoutOptions, NodeGit.checkoutOptions); + normalizeOptions(checkoutOptions, NodeGit.CheckoutOptions); } return sApply(repo, index, options); From fb1bd51fbc81ced2e1bb4285456f245b59eccd8c Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Fri, 18 Mar 2016 10:05:40 -0700 Subject: [PATCH 07/33] Use type safe pointer instead of void * for New --- generate/templates/manual/include/wrapper.h | 2 +- generate/templates/manual/src/convenient_patch.cc | 4 ++-- generate/templates/manual/src/wrapper.cc | 2 +- generate/templates/partials/callback_helpers.cc | 2 +- generate/templates/partials/convert_to_v8.cc | 4 ++-- generate/templates/partials/field_accessors.cc | 2 +- generate/templates/templates/class_content.cc | 2 +- generate/templates/templates/class_header.h | 2 +- generate/templates/templates/struct_content.cc | 4 ++-- generate/templates/templates/struct_header.h | 2 +- 10 files changed, 13 insertions(+), 13 deletions(-) diff --git a/generate/templates/manual/include/wrapper.h b/generate/templates/manual/include/wrapper.h index cd7c26ff0..c040ea64d 100644 --- a/generate/templates/manual/include/wrapper.h +++ b/generate/templates/manual/include/wrapper.h @@ -20,7 +20,7 @@ class Wrapper : public Nan::ObjectWrap { static void InitializeComponent (Local target); void *GetValue(); - static Local New(void *raw); + static Local New(const void *raw); private: Wrapper(void *raw); diff --git a/generate/templates/manual/src/convenient_patch.cc b/generate/templates/manual/src/convenient_patch.cc index 8638cf0d7..dbe75ba74 100644 --- a/generate/templates/manual/src/convenient_patch.cc +++ b/generate/templates/manual/src/convenient_patch.cc @@ -303,7 +303,7 @@ NAN_METHOD(ConvenientPatch::OldFile) { git_diff_file *old_file = (git_diff_file *)malloc(sizeof(git_diff_file)); *old_file = Nan::ObjectWrap::Unwrap(info.This())->GetOldFile(); - to = GitDiffFile::New((void *)old_file, true); + to = GitDiffFile::New(old_file, true); return info.GetReturnValue().Set(to); } @@ -315,7 +315,7 @@ NAN_METHOD(ConvenientPatch::NewFile) { git_diff_file *new_file = (git_diff_file *)malloc(sizeof(git_diff_file)); *new_file = Nan::ObjectWrap::Unwrap(info.This())->GetNewFile(); if (new_file != NULL) { - to = GitDiffFile::New((void *)new_file, true); + to = GitDiffFile::New(new_file, true); } else { to = Nan::Null(); } diff --git a/generate/templates/manual/src/wrapper.cc b/generate/templates/manual/src/wrapper.cc index 0bcc9a745..3f2ea6121 100644 --- a/generate/templates/manual/src/wrapper.cc +++ b/generate/templates/manual/src/wrapper.cc @@ -42,7 +42,7 @@ NAN_METHOD(Wrapper::JSNewFunction) { info.GetReturnValue().Set(info.This()); } -Local Wrapper::New(void *raw) { +Local Wrapper::New(const void *raw) { Nan::EscapableHandleScope scope; Local argv[1] = { Nan::New((void *)raw) }; diff --git a/generate/templates/partials/callback_helpers.cc b/generate/templates/partials/callback_helpers.cc index 0acd215ae..b103d24e0 100644 --- a/generate/templates/partials/callback_helpers.cc +++ b/generate/templates/partials/callback_helpers.cc @@ -55,7 +55,7 @@ void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_async(uv_as {% if arg.isEnum %} Nan::New((int)baton->{{ arg.name }}), {% elsif arg.isLibgitType %} - {{ arg.cppClassName }}::New((void *)baton->{{ arg.name }}, false), + {{ 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 }}), diff --git a/generate/templates/partials/convert_to_v8.cc b/generate/templates/partials/convert_to_v8.cc index bfe40dde6..100885260 100644 --- a/generate/templates/partials/convert_to_v8.cc +++ b/generate/templates/partials/convert_to_v8.cc @@ -59,9 +59,9 @@ if ({{= parsedName =}} != NULL) { // {{= cppClassName }} {{= parsedName }} {% if cppClassName == 'Wrapper' %} - to = {{ cppClassName }}::New((void *){{= parsedName =}}); + to = {{ cppClassName }}::New({{= parsedName =}}); {% else %} - to = {{ cppClassName }}::New((void *){{= parsedName =}}, false); + to = {{ cppClassName }}::New({{= parsedName =}}, false); {% endif %} } else { diff --git a/generate/templates/partials/field_accessors.cc b/generate/templates/partials/field_accessors.cc index 17b338dc3..9261c2fcd 100644 --- a/generate/templates/partials/field_accessors.cc +++ b/generate/templates/partials/field_accessors.cc @@ -149,7 +149,7 @@ {% if arg.isEnum %} Nan::New((int)baton->{{ arg.name }}), {% elsif arg.isLibgitType %} - {{ arg.cppClassName }}::New((void *)baton->{{ arg.name }}, false), + {{ 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 }}), diff --git a/generate/templates/templates/class_content.cc b/generate/templates/templates/class_content.cc index c172e1fa5..34d7a0126 100644 --- a/generate/templates/templates/class_content.cc +++ b/generate/templates/templates/class_content.cc @@ -98,7 +98,7 @@ using namespace node; info.GetReturnValue().Set(info.This()); } - Local {{ cppClassName }}::New(void *raw, bool selfFreeing) { + Local {{ cppClassName }}::New(const {{ cType }} *raw, bool selfFreeing) { Nan::EscapableHandleScope scope; Local argv[2] = { Nan::New((void *)raw), Nan::New(selfFreeing) }; return scope.Escape(Nan::NewInstance(Nan::New({{ cppClassName }}::constructor_template), 2, argv).ToLocalChecked()); diff --git a/generate/templates/templates/class_header.h b/generate/templates/templates/class_header.h index 84c9b5c4f..502b67d72 100644 --- a/generate/templates/templates/class_header.h +++ b/generate/templates/templates/class_header.h @@ -45,7 +45,7 @@ class {{ cppClassName }} : public Nan::ObjectWrap { {{ cType }} *GetValue(); void ClearValue(); - static Local New(void *raw, bool selfFreeing); + static Local New(const {{ cType }} *raw, bool selfFreeing); {%endif%} bool selfFreeing; diff --git a/generate/templates/templates/struct_content.cc b/generate/templates/templates/struct_content.cc index cab524a80..80b496538 100644 --- a/generate/templates/templates/struct_content.cc +++ b/generate/templates/templates/struct_content.cc @@ -73,7 +73,7 @@ void {{ cppClassName }}::ConstructFields() { {% if not field.isEnum %} {% if field.hasConstructor |or field.isLibgitType %} Local {{ field.name }}Temp = {{ field.cppClassName }}::New( - &this->raw->{{ field.name }}, + {%if not field.cType|isPointer %}&{%endif%}this->raw->{{ field.name }}, false )->ToObject(); this->{{ field.name }}.Reset({{ field.name }}Temp); @@ -131,7 +131,7 @@ NAN_METHOD({{ cppClassName }}::JSNewFunction) { info.GetReturnValue().Set(info.This()); } -Local {{ cppClassName }}::New(void* raw, bool selfFreeing) { +Local {{ cppClassName }}::New(const {{ cType }} * raw, bool selfFreeing) { Nan::EscapableHandleScope scope; Local argv[2] = { Nan::New((void *)raw), Nan::New(selfFreeing) }; diff --git a/generate/templates/templates/struct_header.h b/generate/templates/templates/struct_header.h index 0320fbbf6..ac5a0e236 100644 --- a/generate/templates/templates/struct_header.h +++ b/generate/templates/templates/struct_header.h @@ -30,7 +30,7 @@ class {{ cppClassName }} : public Nan::ObjectWrap { {{ cType }} *GetValue(); void ClearValue(); - static Local New(void *raw, bool selfFreeing); + static Local New(const {{ cType }} *raw, bool selfFreeing); bool selfFreeing; From 6ad332882150e17e9856dd689e248f5b5dd6ce0f Mon Sep 17 00:00:00 2001 From: Chris Ball Date: Tue, 22 Mar 2016 13:18:20 -0400 Subject: [PATCH 08/33] Start up a FAQ --- FAQ.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 FAQ.md diff --git a/FAQ.md b/FAQ.md new file mode 100644 index 000000000..6d136b820 --- /dev/null +++ b/FAQ.md @@ -0,0 +1,13 @@ +NodeGit FAQ +----------- + +Feel free to add common problems with their solutions here, or just anything that wasn't clear at first. + +#### Error: callback returned unsupported credentials type #### + +As seen in nodegit/#959 -- some golang hackers have started to use the following stanza in .gitconfig to allow `go get` to work with private repos: +``` +[url "git@github.com:"] + insteadOf = https://github.com/ +``` +But if you do this, code can call `NodeGit.Clone.clone(url: 'https://foo')` and have the `authentication` callback be asked for **SSH** credentials instead of HTTPS ones, which might not be what your application expected. From 0df39e2c258efc289639e65f180a82887e8f07d6 Mon Sep 17 00:00:00 2001 From: "Aoyama, Shotaro" Date: Fri, 11 Mar 2016 20:29:52 +0900 Subject: [PATCH 09/33] fix example --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d743134d5..bea7d75be 100644 --- a/README.md +++ b/README.md @@ -116,8 +116,8 @@ Git.Clone("https://github.com/nodegit/nodegit", "./tmp") }) // Display information about the blob. .then(function(blob) { - // Show the name, sha, and filesize in bytes. - console.log(blob.entry.name() + blob.entry.sha() + blob.size() + "b"); + // Show the path, sha, and filesize in bytes. + console.log(blob.entry.path() + blob.entry.sha() + blob.rawsize() + "b"); // Show a spacer. console.log(Array(72).join("=") + "\n\n"); From da71a29de2e37d222c2559090deb66725fcb0a7a Mon Sep 17 00:00:00 2001 From: Tyler Church Date: Wed, 23 Mar 2016 16:14:45 -0700 Subject: [PATCH 10/33] diff.blobToBuffer: Fix passing in Buffers instead of strings. --- lib/diff.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/diff.js b/lib/diff.js index 34c279047..07c73a9a1 100644 --- a/lib/diff.js +++ b/lib/diff.js @@ -84,7 +84,7 @@ Diff.blobToBuffer= function( this, old_blob, old_as_path, - buffer, + bufferText, bufferLength, buffer_as_path, opts, From 1abdd394fcf29b2520f0b8f6907b7d7ca614bea3 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Tue, 22 Mar 2016 14:42:33 -0700 Subject: [PATCH 11/33] Add descriptor support for selfFreeing --- generate/scripts/helpers.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/generate/scripts/helpers.js b/generate/scripts/helpers.js index 5383bff58..e9b3268c2 100644 --- a/generate/scripts/helpers.js +++ b/generate/scripts/helpers.js @@ -143,6 +143,7 @@ var Helpers = { } }, + // returns the libgittype found in types decorateLibgitType: function(type, types, enums) { var normalizedType = Helpers.normalizeCtype(type.cType); var libgitType = Helpers.getLibgitType(normalizedType, types); @@ -164,6 +165,8 @@ var Helpers = { // we don't want to overwrite the c type of the passed in type _.merge(type, descriptor.types[normalizedType.replace("git_", "")] || {}, { cType: type.cType }); } + + return libgitType; }, decoratePrimaryType: function(typeDef, enums) { @@ -176,6 +179,7 @@ var Helpers = { typeDef.filename = typeDef.typeName; typeDef.isLibgitType = true; typeDef.dependencies = []; + typeDef.selfFreeing = Boolean(typeDefOverrides.selfFreeing); typeDef.fields = typeDef.fields || []; typeDef.fields.forEach(function (field, index, allFields) { @@ -241,7 +245,7 @@ var Helpers = { arg.cppClassName = Helpers.cTypeToCppName(arg.cType); arg.jsClassName = utils.titleCase(Helpers.cTypeToJsName(arg.cType)); - Helpers.decorateLibgitType(arg, libgit2.types, enums); + var libgitType = Helpers.decorateLibgitType(arg, libgit2.types, enums); // Some arguments can be callbacks if (Helpers.isCallbackFunction(type)) { @@ -271,6 +275,10 @@ var Helpers = { _.every(allArgs, function(_arg) { return !_arg.isSelf; }); } + if (arg.isReturn && libgitType) { + arg.selfFreeing = libgitType.selfFreeing; + } + if (arg.isReturn && fnDef.return && fnDef.return.type === "int") { fnDef.return.isErrorCode = true; fnDef.isAsync = true; From 91d90ed28bee0a8902065c02e0ae854cf32a39a9 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Thu, 17 Mar 2016 09:55:46 -0700 Subject: [PATCH 12/33] Mark commit wrappers as self-freeing Affects commit objects returned by: git_commit_lookup git_commit_lookup_prefix git_commit_nth_gen_ancestor git_commit_parent --- generate/input/descriptor.json | 1 + 1 file changed, 1 insertion(+) diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index f51d219ba..2ce8b27bd 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -354,6 +354,7 @@ } }, "commit": { + "selfFreeing": true, "functions": { "git_commit_amend": { "args": { From bc65733f832ed3ff69bfb3d9e79fe00e2892bc21 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Thu, 17 Mar 2016 10:01:02 -0700 Subject: [PATCH 13/33] Mark tree wrappers as self-freeing Affects tree objects returned by: git_tree_lookup git_tree_lookup_prefix --- generate/input/descriptor.json | 1 + 1 file changed, 1 insertion(+) diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index 2ce8b27bd..27c3dfcb6 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -2214,6 +2214,7 @@ ] }, "tree": { + "selfFreeing": true, "functions": { "git_tree_entry_free": { "ignore": true From 5a63abdf303142af8a30921d2cfc5a192a626a83 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Thu, 17 Mar 2016 10:09:35 -0700 Subject: [PATCH 14/33] Mark tag wrappers as self-freeing Affects tag objects returned by: git_tag_lookup git_tag_lookup_prefix --- generate/input/descriptor.json | 1 + 1 file changed, 1 insertion(+) diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index 27c3dfcb6..e79122646 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -2114,6 +2114,7 @@ } }, "tag": { + "selfFreeing": true, "functions": { "git_tag_foreach": { "ignore": true From f70865825726017b21dfa9f7f595eb7a0475f2e1 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Thu, 17 Mar 2016 10:13:46 -0700 Subject: [PATCH 15/33] Mark blob wrappers as self-freeing Affects blob objects returned by: git_blob_lookup git_blob_lookup_prefix --- generate/input/descriptor.json | 1 + 1 file changed, 1 insertion(+) diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index e79122646..ef48e0e7c 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -121,6 +121,7 @@ } }, "blob": { + "selfFreeing": true, "functions": { "git_blob_create_frombuffer": { "isAsync": false, From acd9bedf8c8b74e3bec3963e1832476ecd483fe0 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Tue, 22 Mar 2016 15:34:38 -0700 Subject: [PATCH 16/33] Add self-freeing diagnostic instance counting and leak test --- generate/templates/templates/class_content.cc | 23 +++++++++ generate/templates/templates/class_header.h | 6 +++ test/tests/commit.js | 48 +++++++++++++++++++ 3 files changed, 77 insertions(+) diff --git a/generate/templates/templates/class_content.cc b/generate/templates/templates/class_content.cc index 34d7a0126..92ce14f38 100644 --- a/generate/templates/templates/class_content.cc +++ b/generate/templates/templates/class_content.cc @@ -27,12 +27,21 @@ using namespace node; {{ cppClassName }}::{{ cppClassName }}({{ cType }} *raw, bool selfFreeing) { this->raw = raw; this->selfFreeing = selfFreeing; + + if (selfFreeing) { + SelfFreeingInstanceCount++; + } else { + NonSelfFreeingConstructedCount++; + } + } {{ cppClassName }}::~{{ cppClassName }}() { {% if freeFunctionName %} if (this->selfFreeing) { {{ freeFunctionName }}(this->raw); + SelfFreeingInstanceCount--; + this->raw = NULL; } {% endif %} @@ -77,6 +86,9 @@ using namespace node; {% endif %} {% endeach %} + Nan::SetMethod(tpl, "getSelfFreeingInstanceCount", GetSelfFreeingInstanceCount); + Nan::SetMethod(tpl, "getNonSelfFreeingConstructedCount", GetNonSelfFreeingConstructedCount); + Local _constructor_template = Nan::GetFunction(tpl).ToLocalChecked(); constructor_template.Reset(_constructor_template); Nan::Set(target, Nan::New("{{ jsClassName }}").ToLocalChecked(), _constructor_template); @@ -104,6 +116,14 @@ using namespace node; return scope.Escape(Nan::NewInstance(Nan::New({{ cppClassName }}::constructor_template), 2, argv).ToLocalChecked()); } + NAN_METHOD({{ cppClassName }}::GetSelfFreeingInstanceCount) { + info.GetReturnValue().Set(SelfFreeingInstanceCount); + } + + NAN_METHOD({{ cppClassName }}::GetNonSelfFreeingConstructedCount) { + info.GetReturnValue().Set(NonSelfFreeingConstructedCount); + } + {{ cType }} *{{ cppClassName }}::GetValue() { return this->raw; } @@ -147,3 +167,6 @@ using namespace node; {% if not cTypeIsUndefined %} Nan::Persistent {{ cppClassName }}::constructor_template; {% endif %} + +int {{ cppClassName }}::SelfFreeingInstanceCount; +int {{ cppClassName }}::NonSelfFreeingConstructedCount; diff --git a/generate/templates/templates/class_header.h b/generate/templates/templates/class_header.h index 502b67d72..76976fa3b 100644 --- a/generate/templates/templates/class_header.h +++ b/generate/templates/templates/class_header.h @@ -40,6 +40,10 @@ class {{ cppClassName }} : public Nan::ObjectWrap { static Nan::Persistent constructor_template; static void InitializeComponent (Local target); + // diagnostic count of self-freeing object instances + static int SelfFreeingInstanceCount; + // diagnostic count of constructed non-self-freeing object instances + static int NonSelfFreeingConstructedCount; {%if cType%} {{ cType }} *GetValue(); @@ -96,6 +100,8 @@ class {{ cppClassName }} : public Nan::ObjectWrap { {% endeach %} static NAN_METHOD(JSNewFunction); + static NAN_METHOD(GetSelfFreeingInstanceCount); + static NAN_METHOD(GetNonSelfFreeingConstructedCount); {%each fields as field%} {%if not field.ignore%} diff --git a/test/tests/commit.js b/test/tests/commit.js index 92f122abd..40f617edc 100644 --- a/test/tests/commit.js +++ b/test/tests/commit.js @@ -9,6 +9,25 @@ var exec = promisify(function(command, opts, callback) { return require("child_process").exec(command, opts, callback); }); +// aggressively collects garbage until we fail to improve terminatingIterations +// times. +function garbageCollect() { + var terminatingIterations = 3; + var usedBeforeGC = Number.MAX_VALUE; + var nondecreasingIterations = 0; + for ( ; ; ) { + global.gc(); + var usedAfterGC = process.memoryUsage().heapUsed; + if (usedAfterGC >= usedBeforeGC) { + nondecreasingIterations++; + if (nondecreasingIterations >= terminatingIterations) { + break; + } + } + usedBeforeGC = usedAfterGC; + } +} + describe("Commit", function() { var NodeGit = require("../../"); var Repository = NodeGit.Repository; @@ -624,4 +643,33 @@ describe("Commit", function() { assert.equal(this.committer.email(), "mike@panmedia.co.nz"); }); }); + + it("does not leak", function() { + var test = this; + + garbageCollect(); + var Commit = NodeGit.Commit; + var startSelfFreeingCount = Commit.getSelfFreeingInstanceCount(); + var startNonSelfFreeingCount = Commit.getNonSelfFreeingConstructedCount(); + + var resolve; + var promise = new Promise(function(_resolve) { resolve = _resolve; }); + + NodeGit.Commit.lookup(test.repository, oid) + .then(function() { + // get out of this promise chain to help GC get rid of the commit + setTimeout(resolve, 0); + }); + + return promise + .then(function() { + garbageCollect(); + var endSelfFreeingCount = Commit.getSelfFreeingInstanceCount(); + var endNonSelfFreeingCount = Commit.getNonSelfFreeingConstructedCount(); + // any new self-freeing commits should have been freed + assert.equal(startSelfFreeingCount, endSelfFreeingCount); + // no new non-self-freeing commits should have been constructed + assert.equal(startNonSelfFreeingCount, endNonSelfFreeingCount); + }); + }); }); From 93fd4f20f0d32eda4ec1d66fbdf799dc28506010 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Fri, 18 Mar 2016 10:36:51 -0700 Subject: [PATCH 17/33] Passing correct value for selfFreeing It's possible that we will eventually get rid of the selfFreeing parameter, and just go by the type descriptor, but for now we are more comfortable with a smaller change. --- generate/templates/partials/convert_to_v8.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generate/templates/partials/convert_to_v8.cc b/generate/templates/partials/convert_to_v8.cc index 100885260..9bc61b880 100644 --- a/generate/templates/partials/convert_to_v8.cc +++ b/generate/templates/partials/convert_to_v8.cc @@ -61,7 +61,7 @@ {% if cppClassName == 'Wrapper' %} to = {{ cppClassName }}::New({{= parsedName =}}); {% else %} - to = {{ cppClassName }}::New({{= parsedName =}}, false); + to = {{ cppClassName }}::New({{= parsedName =}}, {% if selfFreeing %}true{% else %}false{% endif %}); {% endif %} } else { From edbb2c8b8d92c953720a425d3e2e2ebff8169974 Mon Sep 17 00:00:00 2001 From: Tyler Wanek Date: Tue, 22 Mar 2016 13:33:24 -0700 Subject: [PATCH 18/33] Force duplication of libgit objects when it is convenient the structs git_tree_entry, git_oid, and git_signature all represent structs which are owned by some other object. In order to simplify our memory model, we should always duplicate these 3 structs when we pull them out of libgit2. This detaches these types of structs from their owning object, such that when we delete a commit, none of the oids or signatures that we retrieved from that commit would cause bad memory access. --- generate/input/descriptor.json | 13 ++++++++++--- generate/scripts/helpers.js | 4 ++++ generate/templates/templates/class_content.cc | 14 +++++++++++++- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index ef48e0e7c..ef7c7e676 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -1275,6 +1275,9 @@ "ignore": true }, "oid": { + "dupFunction": "git_oid_cpy", + "freeFunctionName": "free", + "selfDuplicating": true, "shouldAlloc": true, "functions": { "git_oid_cpy": { @@ -1879,6 +1882,8 @@ } }, "signature": { + "dupFunction": "git_signature_dup", + "selfDuplicating": true, "functions": { "git_signature_default": { "isAsync": false @@ -2218,9 +2223,6 @@ "tree": { "selfFreeing": true, "functions": { - "git_tree_entry_free": { - "ignore": true - }, "git_tree_entry_byindex": { "jsFunctionName": "_entryByIndex" }, @@ -2253,6 +2255,11 @@ } } }, + "tree_entry": { + "dupFunction": "git_tree_entry_dup", + "freeFunctionName": "git_tree_entry_free", + "selfDuplicating": true + }, "writestream": { "cType": "git_writestream", "needsForwardDeclaration": false diff --git a/generate/scripts/helpers.js b/generate/scripts/helpers.js index e9b3268c2..6e1f33613 100644 --- a/generate/scripts/helpers.js +++ b/generate/scripts/helpers.js @@ -181,6 +181,10 @@ var Helpers = { typeDef.dependencies = []; typeDef.selfFreeing = Boolean(typeDefOverrides.selfFreeing); + if (typeDefOverrides.freeFunctionName) { + typeDef.freeFunctionName = typeDefOverrides.freeFunctionName; + } + typeDef.fields = typeDef.fields || []; typeDef.fields.forEach(function (field, index, allFields) { var fieldOverrides = typeDefOverrides.fields || {}; diff --git a/generate/templates/templates/class_content.cc b/generate/templates/templates/class_content.cc index 92ce14f38..334fa9fbb 100644 --- a/generate/templates/templates/class_content.cc +++ b/generate/templates/templates/class_content.cc @@ -25,8 +25,17 @@ using namespace node; {% if cType %} {{ cppClassName }}::{{ cppClassName }}({{ cType }} *raw, bool selfFreeing) { + {% if selfDuplicating %} + {% if shouldAlloc %} + this->raw = ({{ cType }} *)malloc(sizeof({{ cType }})); + {{ dupFunction }}(this->raw, raw); + {% else %} + {{ dupFunction }}(&this->raw, raw); + {% endif %} + {% else %} this->raw = raw; this->selfFreeing = selfFreeing; + {%endif%} if (selfFreeing) { SelfFreeingInstanceCount++; @@ -37,7 +46,10 @@ using namespace node; } {{ cppClassName }}::~{{ cppClassName }}() { - {% if freeFunctionName %} + {% if selfDuplicating %} + {{ freeFunctionName }}(this->raw); + this->raw = NULL; + {% elsif freeFunctionName %} if (this->selfFreeing) { {{ freeFunctionName }}(this->raw); SelfFreeingInstanceCount--; From 2c95f8f7b992e420217c1805516ca1788b20dafc Mon Sep 17 00:00:00 2001 From: John Haley Date: Wed, 23 Mar 2016 08:26:11 -0700 Subject: [PATCH 19/33] Add `toBool` Combyne filter --- generate/scripts/generateNativeCode.js | 1 + generate/templates/filters/to_bool.js | 3 +++ 2 files changed, 4 insertions(+) create mode 100644 generate/templates/filters/to_bool.js diff --git a/generate/scripts/generateNativeCode.js b/generate/scripts/generateNativeCode.js index 93867b49c..4c43e0b04 100644 --- a/generate/scripts/generateNativeCode.js +++ b/generate/scripts/generateNativeCode.js @@ -70,6 +70,7 @@ module.exports = function generateNativeCode() { returnsCount: require("../templates/filters/returns_count"), returnsInfo: require("../templates/filters/returns_info"), titleCase: require("../templates/filters/title_case"), + toBool: require('../templates/filters/to_bool'), unPointer: require("../templates/filters/un_pointer"), upper: require("../templates/filters/upper") }; diff --git a/generate/templates/filters/to_bool.js b/generate/templates/filters/to_bool.js new file mode 100644 index 000000000..dd6063f28 --- /dev/null +++ b/generate/templates/filters/to_bool.js @@ -0,0 +1,3 @@ +module.exports = function(value) { + return !!value; +}; From 47903604e7dbaed519710eda79c57b8cd7f97b2a Mon Sep 17 00:00:00 2001 From: John Haley Date: Wed, 23 Mar 2016 08:26:50 -0700 Subject: [PATCH 20/33] Use `toBool` instead of `if/else` for `selfFreeing` --- generate/templates/partials/convert_to_v8.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generate/templates/partials/convert_to_v8.cc b/generate/templates/partials/convert_to_v8.cc index 9bc61b880..b344ecd26 100644 --- a/generate/templates/partials/convert_to_v8.cc +++ b/generate/templates/partials/convert_to_v8.cc @@ -61,7 +61,7 @@ {% if cppClassName == 'Wrapper' %} to = {{ cppClassName }}::New({{= parsedName =}}); {% else %} - to = {{ cppClassName }}::New({{= parsedName =}}, {% if selfFreeing %}true{% else %}false{% endif %}); + to = {{ cppClassName }}::New({{= parsedName =}}, {{ selfFreeing|toBool }}); {% endif %} } else { From 11a17cba7b5795f68659623510a8ac250dbbe247 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Wed, 23 Mar 2016 16:13:25 -0700 Subject: [PATCH 21/33] Add shouldDuplicate as arg-specific behavior --- generate/input/descriptor.json | 70 +++++++++++++++++-- generate/templates/partials/convert_to_v8.cc | 2 +- generate/templates/templates/class_content.cc | 28 ++++---- generate/templates/templates/class_header.h | 4 +- 4 files changed, 84 insertions(+), 20 deletions(-) diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index ef7c7e676..29f9e2ce4 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -141,6 +141,11 @@ "git_blob_filtered_content": { "ignore": true }, + "git_blob_id": { + "return": { + "shouldDuplicate": true + } + }, "git_blob_rawcontent": { "return": { "cppClassName": "Wrapper", @@ -382,6 +387,16 @@ } } }, + "git_commit_author": { + "return": { + "shouldDuplicate": true + } + }, + "git_commit_committer": { + "return": { + "shouldDuplicate": true + } + }, "git_commit_create": { "args": { "id": { @@ -406,6 +421,21 @@ }, "git_commit_create_from_ids": { "ignore": true + }, + "git_commit_id": { + "return": { + "shouldDuplicate": true + } + }, + "git_commit_parent_id": { + "return": { + "shouldDuplicate": true + } + }, + "git_commit_tree_id": { + "return": { + "shouldDuplicate": true + } } } }, @@ -1277,7 +1307,6 @@ "oid": { "dupFunction": "git_oid_cpy", "freeFunctionName": "free", - "selfDuplicating": true, "shouldAlloc": true, "functions": { "git_oid_cpy": { @@ -1883,7 +1912,6 @@ }, "signature": { "dupFunction": "git_signature_dup", - "selfDuplicating": true, "functions": { "git_signature_default": { "isAsync": false @@ -2139,6 +2167,11 @@ "git_tag_create_frombuffer": { "ignore": true }, + "git_tag_id": { + "return": { + "shouldDuplicate": true + } + }, "git_tag_create_lightweight": { "args": { "oid": { @@ -2174,6 +2207,11 @@ }, "isAsync": true }, + "git_tag_tagger": { + "return": { + "shouldDuplicate": true + } + }, "git_tag_target": { "args": { "target_out": { @@ -2181,6 +2219,11 @@ } } }, + "git_tag_target_id": { + "return": { + "shouldDuplicate": true + } + }, "git_tag_delete": { "return": { "isErrorCode": true @@ -2223,12 +2266,32 @@ "tree": { "selfFreeing": true, "functions": { + "git_tree_entry_byid": { + "return": { + "shouldDuplicate": true + } + }, "git_tree_entry_byindex": { "jsFunctionName": "_entryByIndex" }, + "git_tree_entry_byname": { + "return": { + "shouldDuplicate": true + } + }, + "git_tree_entry_id": { + "return": { + "shouldDuplicate": true + } + }, "git_tree_entrycount": { "jsFunctionName": "entryCount" }, + "git_tree_id": { + "return": { + "shouldDuplicate": true + } + }, "git_tree_walk": { "ignore": true } @@ -2257,8 +2320,7 @@ }, "tree_entry": { "dupFunction": "git_tree_entry_dup", - "freeFunctionName": "git_tree_entry_free", - "selfDuplicating": true + "freeFunctionName": "git_tree_entry_free" }, "writestream": { "cType": "git_writestream", diff --git a/generate/templates/partials/convert_to_v8.cc b/generate/templates/partials/convert_to_v8.cc index b344ecd26..4c669ac76 100644 --- a/generate/templates/partials/convert_to_v8.cc +++ b/generate/templates/partials/convert_to_v8.cc @@ -61,7 +61,7 @@ {% if cppClassName == 'Wrapper' %} to = {{ cppClassName }}::New({{= parsedName =}}); {% else %} - to = {{ cppClassName }}::New({{= parsedName =}}, {{ selfFreeing|toBool }}); + to = {{ cppClassName }}::New({{= parsedName =}}, {{ selfFreeing|toBool }} {% if shouldDuplicate %}, true{% endif %}); {% endif %} } else { diff --git a/generate/templates/templates/class_content.cc b/generate/templates/templates/class_content.cc index 334fa9fbb..312c2ef28 100644 --- a/generate/templates/templates/class_content.cc +++ b/generate/templates/templates/class_content.cc @@ -24,18 +24,19 @@ using namespace v8; using namespace node; {% if cType %} - {{ cppClassName }}::{{ cppClassName }}({{ cType }} *raw, bool selfFreeing) { - {% if selfDuplicating %} + {{ cppClassName }}::{{ cppClassName }}({{ cType }} *raw, bool selfFreeing, bool shouldDuplicate) { + if (shouldDuplicate) { {% if shouldAlloc %} this->raw = ({{ cType }} *)malloc(sizeof({{ cType }})); {{ dupFunction }}(this->raw, raw); {% else %} {{ dupFunction }}(&this->raw, raw); {% endif %} - {% else %} - this->raw = raw; + selfFreeing = true; + } else { + this->raw = raw; + } this->selfFreeing = selfFreeing; - {%endif%} if (selfFreeing) { SelfFreeingInstanceCount++; @@ -46,10 +47,7 @@ using namespace node; } {{ cppClassName }}::~{{ cppClassName }}() { - {% if selfDuplicating %} - {{ freeFunctionName }}(this->raw); - this->raw = NULL; - {% elsif freeFunctionName %} + {% if freeFunctionName %} if (this->selfFreeing) { {{ freeFunctionName }}(this->raw); SelfFreeingInstanceCount--; @@ -116,16 +114,20 @@ using namespace node; {% endif %} } - {{ cppClassName }}* object = new {{ cppClassName }}(static_cast<{{ cType }} *>(Local::Cast(info[0])->Value()), Nan::To(info[1]).FromJust()); + {{ cppClassName }}* object = new {{ cppClassName }}(static_cast<{{ cType }} *>( + Local::Cast(info[0])->Value()), + Nan::To(info[1]).FromJust(), + info.Length() >= 3 ? Nan::To(info[2]).FromJust() : false + ); object->Wrap(info.This()); info.GetReturnValue().Set(info.This()); } - Local {{ cppClassName }}::New(const {{ cType }} *raw, bool selfFreeing) { + Local {{ cppClassName }}::New(const {{ cType }} *raw, bool selfFreeing, bool shouldDuplicate) { Nan::EscapableHandleScope scope; - Local argv[2] = { Nan::New((void *)raw), Nan::New(selfFreeing) }; - return scope.Escape(Nan::NewInstance(Nan::New({{ cppClassName }}::constructor_template), 2, argv).ToLocalChecked()); + Local argv[3] = { Nan::New((void *)raw), Nan::New(selfFreeing), Nan::New(shouldDuplicate) }; + return scope.Escape(Nan::NewInstance(Nan::New({{ cppClassName }}::constructor_template), 3, argv).ToLocalChecked()); } NAN_METHOD({{ cppClassName }}::GetSelfFreeingInstanceCount) { diff --git a/generate/templates/templates/class_header.h b/generate/templates/templates/class_header.h index 76976fa3b..cc77f52b3 100644 --- a/generate/templates/templates/class_header.h +++ b/generate/templates/templates/class_header.h @@ -49,7 +49,7 @@ class {{ cppClassName }} : public Nan::ObjectWrap { {{ cType }} *GetValue(); void ClearValue(); - static Local New(const {{ cType }} *raw, bool selfFreeing); + static Local New(const {{ cType }} *raw, bool selfFreeing, bool shouldDuplicate = false); {%endif%} bool selfFreeing; @@ -85,7 +85,7 @@ class {{ cppClassName }} : public Nan::ObjectWrap { {%if cType%} - {{ cppClassName }}({{ cType }} *raw, bool selfFreeing); + {{ cppClassName }}({{ cType }} *raw, bool selfFreeing, bool shouldDuplicate = false); ~{{ cppClassName }}(); {%endif%} From 4283436b9312dc44dda26b5ba7707a514d823b5e Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Wed, 23 Mar 2016 16:20:25 -0700 Subject: [PATCH 22/33] Add test for shouldDuplicate --- test/tests/commit.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/tests/commit.js b/test/tests/commit.js index 40f617edc..e770a0c1e 100644 --- a/test/tests/commit.js +++ b/test/tests/commit.js @@ -672,4 +672,26 @@ describe("Commit", function() { assert.equal(startNonSelfFreeingCount, endNonSelfFreeingCount); }); }); + + it("duplicates signature", function() { + garbageCollect(); + var Signature = NodeGit.Signature; + var startSelfFreeingCount = Signature.getSelfFreeingInstanceCount(); + var startNonSelfFreeingCount = + Signature.getNonSelfFreeingConstructedCount(); + var signature = this.commit.author(); + + garbageCollect(); + var endSelfFreeingCount = Signature.getSelfFreeingInstanceCount(); + var endNonSelfFreeingCount = Signature.getNonSelfFreeingConstructedCount(); + // we should get one duplicated, self-freeing signature + assert.equal(startSelfFreeingCount + 1, endSelfFreeingCount); + assert.equal(startNonSelfFreeingCount, endNonSelfFreeingCount); + + signature = null; + garbageCollect(); + endSelfFreeingCount = Signature.getSelfFreeingInstanceCount(); + // the self-freeing signature should get freed + assert.equal(startSelfFreeingCount, endSelfFreeingCount); + }); }); From 74e85a5c7bf7b2b4c9098481f6c6a042e3d8826d Mon Sep 17 00:00:00 2001 From: Tyler Church Date: Wed, 23 Mar 2016 19:32:05 -0700 Subject: [PATCH 23/33] Switch from strdup to memcpy to faithfully copy all bytes and prevent a crash in some cases. --- generate/templates/partials/convert_from_v8.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/generate/templates/partials/convert_from_v8.cc b/generate/templates/partials/convert_from_v8.cc index 659e2fb0e..7dbdd1eb1 100644 --- a/generate/templates/partials/convert_from_v8.cc +++ b/generate/templates/partials/convert_from_v8.cc @@ -14,7 +14,9 @@ {%if cppClassName == 'String'%} String::Utf8Value {{ name }}(info[{{ jsArg }}]->ToString()); - from_{{ name }} = ({{ cType }}) strdup(*{{ name }}); + from_{{ name }} = ({{ cType }}) malloc({{ name }}.length() + 1); + memcpy((void *)from_{{ name }}, *{{ name }}, {{ name }}.length()); + memset((void *)(((char *)from_{{ name }}) + {{ name }}.length()), 0, 1); {%elsif cppClassName == 'GitStrarray' %} from_{{ name }} = StrArrayConverter::Convert(info[{{ jsArg }}]); @@ -24,7 +26,9 @@ {%elsif cppClassName == 'Wrapper'%} String::Utf8Value {{ name }}(info[{{ jsArg }}]->ToString()); - from_{{ name }} = ({{ cType }}) strdup(*{{ name }}); + from_{{ name }} = ({{ cType }}) malloc({{ name }}.length() + 1); + memcpy((void *)from_{{ name }}, *{{ name }}, {{ name }}.length()); + memset((void *)(((char *)from_{{ name }}) + {{ name }}.length()), 0, 1); {%elsif cppClassName == 'Array'%} Array *tmp_{{ name }} = Array::Cast(*info[{{ jsArg }}]); From a87b573f00b97842737abc41d8360bbc76a86f53 Mon Sep 17 00:00:00 2001 From: Tyler Church Date: Wed, 23 Mar 2016 20:51:03 -0700 Subject: [PATCH 24/33] Diff.blobToBuffer: consistently use Buffer.byteLength --- lib/diff.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/diff.js b/lib/diff.js index 07c73a9a1..b397ecd2b 100644 --- a/lib/diff.js +++ b/lib/diff.js @@ -72,7 +72,7 @@ Diff.blobToBuffer= function( var bufferLength; if (buffer instanceof Buffer) { bufferText = buffer.toString("utf8"); - bufferLength = buffer.length; + bufferLength = Buffer.byteLength(buffer, "utf8"); } else { bufferText = buffer; bufferLength = !buffer ? 0 : Buffer.byteLength(buffer, "utf8"); From 7cc1a412abfa6a23eb4b5c0581c2099c6ff95a4e Mon Sep 17 00:00:00 2001 From: Tyler Church Date: Thu, 24 Mar 2016 06:51:53 -0700 Subject: [PATCH 25/33] Correct Diff.blobToBuffer documentation. --- lib/diff.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/diff.js b/lib/diff.js index b397ecd2b..86a0cb60b 100644 --- a/lib/diff.js +++ b/lib/diff.js @@ -58,6 +58,19 @@ Diff.prototype.findSimilar = function(opts) { }; var blobToBuffer = Diff.blobToBuffer; +/** + * Directly run a diff between a blob and a buffer. + * @async + * @param {Blob} old_blob Blob for old side of diff, or NULL for empty blob + * @param {String} old_as_path Treat old blob as if it had this filename; can be NULL + * @param {String} buffer Raw data for new side of diff, or NULL for empty + * @param {String} buffer_as_path Treat buffer as if it had this filename; can be NULL + * @param {DiffOptions} opts Options for diff, or NULL for default options + * @param {Function} file_cb Callback for "file"; made once if there is a diff; can be NULL + * @param {Function} binary_cb Callback for binary files; can be NULL + * @param {Function} hunk_cb Callback for each hunk in diff; can be NULL + * @param {Function} line_cb Callback for each line in diff; can be NULL + */ Diff.blobToBuffer= function( old_blob, old_as_path, From c866391086889e5b4cdfdec745ecf808ea1b64e6 Mon Sep 17 00:00:00 2001 From: Tyler Church Date: Thu, 24 Mar 2016 07:00:35 -0700 Subject: [PATCH 26/33] Add some comments explaining the switch between strdup and malloc/memcpy/memset. --- generate/templates/partials/convert_from_v8.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/generate/templates/partials/convert_from_v8.cc b/generate/templates/partials/convert_from_v8.cc index 7dbdd1eb1..425e691f4 100644 --- a/generate/templates/partials/convert_from_v8.cc +++ b/generate/templates/partials/convert_from_v8.cc @@ -14,8 +14,13 @@ {%if cppClassName == 'String'%} String::Utf8Value {{ name }}(info[{{ jsArg }}]->ToString()); + // malloc with one extra byte so we can add the terminating null character C-strings expect: from_{{ name }} = ({{ cType }}) malloc({{ name }}.length() + 1); + // copy the characters from the nodejs string into our C-string (used instead of strdup or strcpy because nulls in + // the middle of strings are valid coming from nodejs): memcpy((void *)from_{{ name }}, *{{ name }}, {{ name }}.length()); + // ensure the final byte of our new string is null, extra casts added to ensure compatibility with various C types + // used in the nodejs binding generation: memset((void *)(((char *)from_{{ name }}) + {{ name }}.length()), 0, 1); {%elsif cppClassName == 'GitStrarray' %} @@ -26,8 +31,13 @@ {%elsif cppClassName == 'Wrapper'%} String::Utf8Value {{ name }}(info[{{ jsArg }}]->ToString()); + // malloc with one extra byte so we can add the terminating null character C-strings expect: from_{{ name }} = ({{ cType }}) malloc({{ name }}.length() + 1); + // copy the characters from the nodejs string into our C-string (used instead of strdup or strcpy because nulls in + // the middle of strings are valid coming from nodejs): memcpy((void *)from_{{ name }}, *{{ name }}, {{ name }}.length()); + // ensure the final byte of our new string is null, extra casts added to ensure compatibility with various C types + // used in the nodejs binding generation: memset((void *)(((char *)from_{{ name }}) + {{ name }}.length()), 0, 1); {%elsif cppClassName == 'Array'%} From 838c1adf92fa99f00eef9c2fc067bedcbb2e3cda Mon Sep 17 00:00:00 2001 From: Tyler Church Date: Thu, 24 Mar 2016 07:04:23 -0700 Subject: [PATCH 27/33] Update Diff.blobToBuffer documentation to make jshint happy. --- lib/diff.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/diff.js b/lib/diff.js index 86a0cb60b..c812c9f9b 100644 --- a/lib/diff.js +++ b/lib/diff.js @@ -62,11 +62,14 @@ var blobToBuffer = Diff.blobToBuffer; * Directly run a diff between a blob and a buffer. * @async * @param {Blob} old_blob Blob for old side of diff, or NULL for empty blob - * @param {String} old_as_path Treat old blob as if it had this filename; can be NULL + * @param {String} old_as_path Treat old blob as if it had this filename; + * can be NULL * @param {String} buffer Raw data for new side of diff, or NULL for empty - * @param {String} buffer_as_path Treat buffer as if it had this filename; can be NULL + * @param {String} buffer_as_path Treat buffer as if it had this filename; + * can be NULL * @param {DiffOptions} opts Options for diff, or NULL for default options - * @param {Function} file_cb Callback for "file"; made once if there is a diff; can be NULL + * @param {Function} file_cb Callback for "file"; made once if there is a diff; + * can be NULL * @param {Function} binary_cb Callback for binary files; can be NULL * @param {Function} hunk_cb Callback for each hunk in diff; can be NULL * @param {Function} line_cb Callback for each line in diff; can be NULL From 1ad0ef25bb5aa12f362abbcd744129cc4f087153 Mon Sep 17 00:00:00 2001 From: joshaber Date: Fri, 25 Mar 2016 11:34:55 -0400 Subject: [PATCH 28/33] #Async4Lyfe --- generate/input/descriptor.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index 29f9e2ce4..7ab980efa 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -949,9 +949,11 @@ "git_ignore_path_is_ignored": { "args": { "ignored": { + "shouldAlloc": true, "isReturn": true } - } + }, + "isAsync": true } } }, From ffec49b3dcdb564547946d6e17bff3d766e002bb Mon Sep 17 00:00:00 2001 From: joshaber Date: Fri, 25 Mar 2016 11:36:37 -0400 Subject: [PATCH 29/33] Update the test. --- test/tests/ignore.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/tests/ignore.js b/test/tests/ignore.js index 1286de90f..f834b28d0 100644 --- a/test/tests/ignore.js +++ b/test/tests/ignore.js @@ -18,7 +18,16 @@ describe("Ignore", function() { }); it("can determine if a path is ignored", function() { - assert.equal(Ignore.pathIsIgnored(this.repository, ".git"), true); - assert.equal(Ignore.pathIsIgnored(this.repository, "LICENSE"), false); + function expectIgnoreState(repo, fileName, expected) { + return Ignore.pathIsIgnored(repo, fileName) + .then(function(ignored) { + assert.equal(ignored, expected); + }); + } + + return Promise.all([ + expectIgnoreState(this.repository, ".git", true), + expectIgnoreState(this.repository, "LICENSE", false) + ]); }); }); From 1c7ad24915d20f5dbb2360ccb9235b1158ede770 Mon Sep 17 00:00:00 2001 From: Martin Heidegger Date: Tue, 29 Mar 2016 00:56:45 +0900 Subject: [PATCH 30/33] Added error when libstdc++ is missing and info how to resolve --- package.json | 3 ++- postinstall.sh | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100755 postinstall.sh diff --git a/package.json b/package.json index 19dee3ecb..06cd363c6 100644 --- a/package.json +++ b/package.json @@ -88,6 +88,7 @@ "rebuild": "node generate && node-gyp configure build", "recompileDebug": "node-gyp configure --debug build", "rebuildDebug": "node generate && node-gyp configure --debug build", - "xcodeDebug": "node-gyp configure -- -f xcode" + "xcodeDebug": "node-gyp configure -- -f xcode", + "postinstall": "./postinstall.sh" } } diff --git a/postinstall.sh b/postinstall.sh new file mode 100755 index 000000000..bf637981f --- /dev/null +++ b/postinstall.sh @@ -0,0 +1,12 @@ +#!/usr/bin/env bash + +if [ -n "$(node lib/nodegit.js 2>&1 | grep libstdc++)" ]; then + echo "[ERROR] Seems like you the latest libstdc++ is missing on your system!" + echo "" + echo "On Ubuntu you can install it using:" + echo "" + echo "$ sudo add-apt-repository ppa:ubuntu-toolchain-r/test" + echo "$ sudo apt-get update" + echo "$ sudo apt-get install libstdc++-4.9-dev" + exit 1 +fi \ No newline at end of file From 601cacb67a19a49d5266b3122684e7d62ae6d94b Mon Sep 17 00:00:00 2001 From: Martin Heidegger Date: Tue, 29 Mar 2016 01:10:08 +0900 Subject: [PATCH 31/33] Fixed package.json for windows --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 06cd363c6..0a5f1bf26 100644 --- a/package.json +++ b/package.json @@ -89,6 +89,6 @@ "recompileDebug": "node-gyp configure --debug build", "rebuildDebug": "node generate && node-gyp configure --debug build", "xcodeDebug": "node-gyp configure -- -f xcode", - "postinstall": "./postinstall.sh" + "postinstall": "bash postinstall.sh" } } From a606a99ecc1f8a88ebac3b4da257bfb6422dce36 Mon Sep 17 00:00:00 2001 From: Martin Heidegger Date: Tue, 29 Mar 2016 01:57:22 +0900 Subject: [PATCH 32/33] Grammatical fix in error message and EOL --- postinstall.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/postinstall.sh b/postinstall.sh index bf637981f..b3fd35fab 100755 --- a/postinstall.sh +++ b/postinstall.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash if [ -n "$(node lib/nodegit.js 2>&1 | grep libstdc++)" ]; then - echo "[ERROR] Seems like you the latest libstdc++ is missing on your system!" + echo "[ERROR] Seems like the latest libstdc++ is missing on your system!" echo "" echo "On Ubuntu you can install it using:" echo "" @@ -9,4 +9,4 @@ if [ -n "$(node lib/nodegit.js 2>&1 | grep libstdc++)" ]; then echo "$ sudo apt-get update" echo "$ sudo apt-get install libstdc++-4.9-dev" exit 1 -fi \ No newline at end of file +fi From fe433efdbab6b776489aa93fe934775284eb2a3a Mon Sep 17 00:00:00 2001 From: John Haley Date: Mon, 28 Mar 2016 11:15:19 -0700 Subject: [PATCH 33/33] Bump to 0.12.0 --- CHANGELOG.md | 17 +++++++++++++++++ README.md | 2 +- package.json | 4 ++-- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3ba6b6d7..494cd73a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,22 @@ # Change Log +## [0.12.0](https://github.com/nodegit/nodegit/releases/tag/v0.12.0) (2016-03-28) + +[Full Changelog](https://github.com/nodegit/nodegit/compare/v0.11.9...v0.12.0) + +# API changes +- `Ignore` + - Made `Ignore.pathIsIgnored` async [PR #970](https://github.com/nodegit/nodegit/pull/970) + +# Bug fixes + +- Added an error message when trying to install NodeGit without a required version of libstdc++ [PR #972](https://github.com/nodegit/nodegit/pull/972) +- Fix a crash when grabbing content out of a buffer that has unicode [PR #966](https://github.com/nodegit/nodegit/pull/966) +- Added some plumbing for better memory management [PR #958](https://github.com/nodegit/nodegit/pull/958) +- Fix `checkoutOptions` in `Stash#apply` [PR #956](https://github.com/nodegit/nodegit/pull/956) +- Fixed install when there is a space in the username on windows [PR #951](https://github.com/nodegit/nodegit/pull/951) +- Bump to nan@2.2.0 [PR #952](https://github.com/nodegit/nodegit/pull/952) + ## [0.11.9](https://github.com/nodegit/nodegit/releases/tag/v0.11.9) (2016-03-09) [Full Changelog](https://github.com/nodegit/nodegit/compare/v0.11.8...v0.11.9) diff --git a/README.md b/README.md index bea7d75be..8fec1521d 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ NodeGit -**Stable: 0.11.9** +**Stable: 0.12.0** ## Have a problem? Come chat with us! ## diff --git a/package.json b/package.json index 0a5f1bf26..33719702f 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "nodegit", "description": "Node.js libgit2 asynchronous native bindings", - "version": "0.11.9", + "version": "0.12.0", "homepage": "http://nodegit.org", "keywords": [ "libgit2", @@ -88,7 +88,7 @@ "rebuild": "node generate && node-gyp configure build", "recompileDebug": "node-gyp configure --debug build", "rebuildDebug": "node generate && node-gyp configure --debug build", - "xcodeDebug": "node-gyp configure -- -f xcode", + "xcodeDebug": "node-gyp configure -- -f xcode", "postinstall": "bash postinstall.sh" } }