From ec97dbce629fdb4c035504eba5c3c60a6328e690 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Mon, 28 Mar 2016 11:06:06 -0700 Subject: [PATCH 01/12] Add support for git_repository__cleanup --- generate/input/descriptor.json | 6 +++--- test/tests/repository.js | 13 +++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index 7ab980efa..1210f1d53 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -1769,10 +1769,10 @@ "ignore": true }, "repository": { + "dependencies": [ + "git2/sys/repository.h" + ], "functions": { - "git_repository__cleanup": { - "ignore": true - }, "git_repository_discover": { "isAsync": true, "return": { diff --git a/test/tests/repository.js b/test/tests/repository.js index 5f7555ac4..a0635041d 100644 --- a/test/tests/repository.js +++ b/test/tests/repository.js @@ -67,6 +67,19 @@ describe("Repository", function() { }); }); + it("can be cleaned", function() { + this.repository.cleanup(); + + // try getting a commit after cleanup (to test that the repo is usable) + return this.repository.getHeadCommit() + .then(function(commit) { + assert.equal( + commit.toString(), + "32789a79e71fbc9e04d3eff7425e1771eb595150" + ); + }); + }); + it("can read the index", function() { return this.repository.index() .then(function(index) { From 045ec0e5f22fbf429d1cd6984486fac35a077ae4 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Thu, 24 Mar 2016 11:57:12 -0700 Subject: [PATCH 02/12] Add support for thread safety on async actions only --- .../templates/manual/include/lock_master.h | 27 +++++++--- generate/templates/manual/src/lock_master.cc | 4 +- generate/templates/partials/async_function.cc | 4 +- generate/templates/partials/sync_function.cc | 4 +- generate/templates/templates/nodegit.cc | 32 +++++++++--- test/runner.js | 4 ++ test/tests/index.js | 1 - test/tests/thread_safety.js | 51 ++++++++++--------- 8 files changed, 81 insertions(+), 46 deletions(-) diff --git a/generate/templates/manual/include/lock_master.h b/generate/templates/manual/include/lock_master.h index fd1a09b6f..94a622666 100644 --- a/generate/templates/manual/include/lock_master.h +++ b/generate/templates/manual/include/lock_master.h @@ -4,8 +4,15 @@ class LockMasterImpl; class LockMaster { +public: + enum Status { + Disabled = 0, + EnabledForAsyncOnly, + Enabled + }; - static bool enabled; +private: + static Status status; LockMasterImpl *impl; @@ -34,8 +41,8 @@ class LockMaster { public: // we lock on construction - template LockMaster(bool emptyGuard, const Types*... types) { - if(!enabled) { + template LockMaster(bool asyncAction, const Types*... types) { + if((status == Disabled) || ((status == EnabledForAsyncOnly) && !asyncAction)) { impl = NULL; return; } @@ -62,7 +69,7 @@ class LockMaster { void DestructorImpl(); public: TemporaryUnlock() { - // We can't return here if enabled is false + // We can't return here if disabled // It's possible that a LockMaster was fully constructed and registered // before the thread safety was disabled. // So we rely on ConstructorImpl to abort if there is no registered LockMaster @@ -80,15 +87,19 @@ class LockMaster { // Enables the thread safety system static void Enable() { - enabled = true; + status = Enabled; + } + + static void SetStatus(Status status) { + LockMaster::status = status; } static void Disable() { - enabled = false; + status = Disabled; } - static bool IsEnabled() { - return enabled; + static Status GetStatus() { + return status; } // Diagnostic information that can be provided to the JavaScript layer diff --git a/generate/templates/manual/src/lock_master.cc b/generate/templates/manual/src/lock_master.cc index bc26b9900..30679b534 100644 --- a/generate/templates/manual/src/lock_master.cc +++ b/generate/templates/manual/src/lock_master.cc @@ -87,7 +87,7 @@ NAN_GC_CALLBACK(LockMasterImpl::CleanupMutexes) { // this means that turning thread safety on and then off // could result in remaining mutexes - but they would get cleaned up // if thread safety is turned on again - if (!LockMaster::IsEnabled()) { + if (LockMaster::GetStatus() == LockMaster::Disabled) { return; } @@ -243,4 +243,4 @@ void LockMaster::TemporaryUnlock::DestructorImpl() { impl->Lock(false); } -bool LockMaster::enabled = false; +LockMaster::Status LockMaster::status = LockMaster::Disabled; diff --git a/generate/templates/partials/async_function.cc b/generate/templates/partials/async_function.cc index 4861c7831..c38066b99 100644 --- a/generate/templates/partials/async_function.cc +++ b/generate/templates/partials/async_function.cc @@ -80,9 +80,9 @@ NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) { void {{ cppClassName }}::{{ cppFunctionName }}Worker::Execute() { giterr_clear(); - + { - LockMaster lockMaster(true{%each args|argsInfo as arg %} + LockMaster lockMaster(/*asyncAction: */true{%each args|argsInfo as arg %} {%if arg.cType|isPointer%}{%if not arg.cType|isDoublePointer%} ,baton->{{ arg.name }} {%endif%}{%endif%} diff --git a/generate/templates/partials/sync_function.cc b/generate/templates/partials/sync_function.cc index ada0de62f..d53a5c2d0 100644 --- a/generate/templates/partials/sync_function.cc +++ b/generate/templates/partials/sync_function.cc @@ -35,9 +35,9 @@ if (Nan::ObjectWrap::Unwrap<{{ cppClassName }}>(info.This())->GetValue() != NULL {% endif %} giterr_clear(); - + { - LockMaster lockMaster(true{%each args|argsInfo as arg %} + LockMaster lockMaster(/*asyncAction: */false{%each args|argsInfo as arg %} {%if arg.cType|isPointer%}{%if not arg.isReturn%} ,{%if arg.isSelf %} Nan::ObjectWrap::Unwrap<{{ arg.cppClassName }}>(info.This())->GetValue() diff --git a/generate/templates/templates/nodegit.cc b/generate/templates/templates/nodegit.cc index f165307b3..189a4d622 100644 --- a/generate/templates/templates/nodegit.cc +++ b/generate/templates/templates/nodegit.cc @@ -25,12 +25,25 @@ void LockMasterEnable(const FunctionCallbackInfo& info) { LockMaster::Enable(); } -void LockMasterDisable(const FunctionCallbackInfo& info) { - LockMaster::Disable(); +void LockMasterSetStatus(const FunctionCallbackInfo& info) { + Nan::HandleScope scope; + + // convert the first argument to Status + if(info.Length() >= 0 && info[0]->IsNumber()) { + v8::Local value = info[0]->ToInt32(); + LockMaster::Status status = static_cast(value->Value()); + if(status >= LockMaster::Disabled && status <= LockMaster::Enabled) { + LockMaster::SetStatus(status); + return; + } + } + + // argument error + Nan::ThrowError("Argument must be one 0, 1 or 2"); } -void LockMasterIsEnabled(const FunctionCallbackInfo& info) { - info.GetReturnValue().Set(Nan::New(LockMaster::IsEnabled())); +void LockMasterGetStatus(const FunctionCallbackInfo& info) { + info.GetReturnValue().Set(Nan::New(LockMaster::GetStatus())); } void LockMasterGetDiagnostics(const FunctionCallbackInfo& info) { @@ -88,10 +101,17 @@ extern "C" void init(Local target) { ConvenientPatch::InitializeComponent(target); NODE_SET_METHOD(target, "enableThreadSafety", LockMasterEnable); - NODE_SET_METHOD(target, "disableThreadSafety", LockMasterDisable); - NODE_SET_METHOD(target, "isThreadSafetyEnabled", LockMasterIsEnabled); + NODE_SET_METHOD(target, "setThreadSafetyStatus", LockMasterSetStatus); + NODE_SET_METHOD(target, "getThreadSafetyStatus", LockMasterGetStatus); NODE_SET_METHOD(target, "getThreadSafetyDiagnostics", LockMasterGetDiagnostics); + Local threadSafety = Nan::New(); + threadSafety->Set(Nan::New("DISABLED").ToLocalChecked(), Nan::New((int)LockMaster::Disabled)); + threadSafety->Set(Nan::New("ENABLED_FOR_ASYNC_ONLY").ToLocalChecked(), Nan::New((int)LockMaster::EnabledForAsyncOnly)); + threadSafety->Set(Nan::New("ENABLED").ToLocalChecked(), Nan::New((int)LockMaster::Enabled)); + + target->Set(Nan::New("THREAD_SAFETY").ToLocalChecked(), threadSafety); + LockMaster::Initialize(); } diff --git a/test/runner.js b/test/runner.js index 0b14a0c0f..ba50eb179 100644 --- a/test/runner.js +++ b/test/runner.js @@ -6,7 +6,11 @@ var local = path.join.bind(path, __dirname); var NodeGit = require('..'); if(process.env.NODEGIT_TEST_THREADSAFETY) { + console.log('Enabling thread safety in NodeGit'); NodeGit.enableThreadSafety(); +} else if (process.env.NODEGIT_TEST_THREADSAFETY_ASYNC) { + console.log('Enabling thread safety for async actions only in NodeGit'); + NodeGit.setThreadSafetyStatus(NodeGit.THREAD_SAFETY.ENABLED_FOR_ASYNC_ONLY); } // Have to wrap exec, since it has a weird callback signature. diff --git a/test/tests/index.js b/test/tests/index.js index ff0fb6d35..a8baa3a4d 100644 --- a/test/tests/index.js +++ b/test/tests/index.js @@ -31,7 +31,6 @@ describe("Index", function() { after(function() { this.index.clear(); - NodeGit.disableThreadSafety(); }); it("can get the index of a repo and examine entries", function() { diff --git a/test/tests/thread_safety.js b/test/tests/thread_safety.js index ee341a95a..c02952630 100644 --- a/test/tests/thread_safety.js +++ b/test/tests/thread_safety.js @@ -22,43 +22,44 @@ describe("ThreadSafety", function() { }); it("can enable and disable thread safety", function() { - var originalValue = NodeGit.isThreadSafetyEnabled(); + var originalValue = NodeGit.getThreadSafetyStatus(); NodeGit.enableThreadSafety(); - assert.equal(true, NodeGit.isThreadSafetyEnabled()); + assert.equal(NodeGit.THREAD_SAFETY.ENABLED, + NodeGit.getThreadSafetyStatus()); - NodeGit.disableThreadSafety(); - assert.equal(false, NodeGit.isThreadSafetyEnabled()); + NodeGit.setThreadSafetyStatus(NodeGit.THREAD_SAFETY.ENABLED_FOR_ASYNC_ONLY); + assert.equal(NodeGit.THREAD_SAFETY.ENABLED_FOR_ASYNC_ONLY, + NodeGit.getThreadSafetyStatus()); - // flip the switch again, to make sure we test all transitions - // (we could have started with thread safety enabled) - NodeGit.enableThreadSafety(); - assert.equal(true, NodeGit.isThreadSafetyEnabled()); + NodeGit.setThreadSafetyStatus(NodeGit.THREAD_SAFETY.DISABLED); + assert.equal(NodeGit.THREAD_SAFETY.DISABLED, + NodeGit.getThreadSafetyStatus()); - if (originalValue) { - NodeGit.enableThreadSafety(); - } else { - NodeGit.disableThreadSafety(); - } + NodeGit.setThreadSafetyStatus(originalValue); }); it("can lock something and cleanup mutex", function() { + var diagnostics = NodeGit.getThreadSafetyDiagnostics(); + var originalCount = diagnostics.storedMutexesCount; // call a sync method to guarantee that it stores a mutex, // and that it will clean up the mutex in a garbage collection cycle this.repository.headDetached(); - var diagnostics = NodeGit.getThreadSafetyDiagnostics(); - if (NodeGit.isThreadSafetyEnabled()) { - // this is a fairly vague test - it just tests that something - // had a mutex created for it at some point (i.e., the thread safety - // code is not completely dead) - assert.ok(diagnostics.storedMutexesCount > 0); - // now test that GC cleans up mutexes - global.gc(); - diagnostics = NodeGit.getThreadSafetyDiagnostics(); - assert.equal(0, diagnostics.storedMutexesCount); - } else { - assert.equal(0, diagnostics.storedMutexesCount); + diagnostics = NodeGit.getThreadSafetyDiagnostics(); + switch(NodeGit.getThreadSafetyStatus()) { + case NodeGit.THREAD_SAFETY.ENABLED: + // this is a fairly vague test - it just tests that something + // had a mutex created for it at some point (i.e., the thread safety + // code is not completely dead) + assert.ok(diagnostics.storedMutexesCount > 0); + break; + case NodeGit.THREAD_SAFETY.ENABLED_FOR_ASYNC_ONLY: + assert.equal(originalCount, diagnostics.storedMutexesCount); + break; + + case NodeGit.THREAD_SAFETY.DISABLED: + assert.equal(0, diagnostics.storedMutexesCount); } }); }); From 86f3c6dd8a8f5c912d11ce3c216e3359a5444010 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Tue, 29 Mar 2016 12:24:51 -0700 Subject: [PATCH 03/12] Mark references as self-freeing --- generate/input/descriptor.json | 1 + 1 file changed, 1 insertion(+) diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index 1210f1d53..c3df3a53f 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -1537,6 +1537,7 @@ }, "reference": { "cppClassName": "GitRefs", + "selfFreeing": true, "functions": { "git_reference__alloc": { "ignore": true From a0d546cec3cd7b56d5d28e25b57d5fa4fd9b2017 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Tue, 29 Mar 2016 13:45:34 -0700 Subject: [PATCH 04/12] Duplicate returned oids --- generate/input/descriptor.json | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index c3df3a53f..48eb6968d 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -1591,6 +1591,16 @@ "isOptional": true } } + }, + "git_reference_target": { + "return": { + "shouldDuplicate": true + } + }, + "git_reference_target_peel": { + "return": { + "shouldDuplicate": true + } } } }, From f2eafe2ac6cdac0ac8b90b32895af0042749ba6b Mon Sep 17 00:00:00 2001 From: John Haley Date: Tue, 29 Mar 2016 18:06:24 -0700 Subject: [PATCH 05/12] Move postinstall script from bash to node --- package.json | 2 +- postinstall.js | 20 ++++++++++++++++++++ postinstall.sh | 12 ------------ 3 files changed, 21 insertions(+), 13 deletions(-) create mode 100755 postinstall.js delete mode 100755 postinstall.sh diff --git a/package.json b/package.json index 33719702f..9f820b454 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": "bash postinstall.sh" + "postinstall": "node postinstall.js" } } diff --git a/postinstall.js b/postinstall.js new file mode 100755 index 000000000..d022f3607 --- /dev/null +++ b/postinstall.js @@ -0,0 +1,20 @@ +#!/usr/bin/env node + +var fs = require("fs"); +var child_process = require("child_process"); + +if (process.platform !== "linux") { + return; +} + +child_process.exec("node lib/nodegit.js", function(error, stdout, stderr) { + if (stderr && ~stderr.indexOf("libstdc++")) { + console.log("[ERROR] Seems like the latest libstdc++ is missing on your system!"); + console.log(""); + console.log("On Ubuntu you can install it using:"); + console.log(""); + console.log("$ sudo add-apt-repository ppa:ubuntu-toolchain-r/test"); + console.log("$ sudo apt-get update"); + console.log("$ sudo apt-get install libstdc++-4.9-dev"); + } +}); diff --git a/postinstall.sh b/postinstall.sh deleted file mode 100755 index b3fd35fab..000000000 --- a/postinstall.sh +++ /dev/null @@ -1,12 +0,0 @@ -#!/usr/bin/env bash - -if [ -n "$(node lib/nodegit.js 2>&1 | grep libstdc++)" ]; then - echo "[ERROR] Seems like 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 From 593f9d58748b486b502f681d3d441996e6d91e9b Mon Sep 17 00:00:00 2001 From: Ivan K Date: Fri, 1 Apr 2016 14:41:31 -0500 Subject: [PATCH 06/12] build: build and publish binaries for 32-bit systems includes workaround which symlinks /usr/include/asm to /usr/include/asm-generic, see https://bugs.launchpad.net/ubuntu/+source/gcc-defaults/+bug/825574/comments/2 --- .travis.yml | 40 +++++++++++++++++++++++++++---------- lifecycleScripts/install.js | 3 +++ 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/.travis.yml b/.travis.yml index 55a1c2ad9..319b4eeea 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,12 +13,22 @@ sudo: false env: matrix: - - export NODE_VERSION="0.12" - - export NODE_VERSION="4.1" - - export NODE_VERSION="5.8" + - export NODE_VERSION="0.12" TARGET_ARCH="x64" + - export NODE_VERSION="4.1" TARGET_ARCH="x64" + - export NODE_VERSION="5.8" TARGET_ARCH="x64" matrix: fast_finish: true + include: + - os: linux + env: export NODE_VERSION="0.12" TARGET_ARCH="ia32" + sudo: required + - os: linux + env: export NODE_VERSION="4.1" TARGET_ARCH="ia32" + sudo: required + - os: linux + env: export NODE_VERSION="5.8" TARGET_ARCH="ia32" + sudo: required git: depth: 1 @@ -30,8 +40,8 @@ addons: packages: - build-essential - libssl-dev - - gcc-4.9 - - g++-4.9 + - gcc-4.9-multilib + - g++-4.9-multilib - lcov before_install: @@ -39,6 +49,10 @@ before_install: - export CXX=clang++ - export npm_config_clang=1 + - if [ $TARGET_ARCH == "ia32" ]; then + sudo ln -s /usr/include/asm-generic /usr/include/asm; + fi + - if [ $TRAVIS_OS_NAME != "linux" ]; then git clone https://github.com/creationix/nvm.git ./.nvm; source ./.nvm/nvm.sh; @@ -74,10 +88,14 @@ before_script: - git config --global user.email johndoe@example.com script: - - if [ -z "$TRAVIS_TAG" ] && [ $TRAVIS_OS_NAME == "linux" ] && [ $NODE_VERSION == "0.12" ]; then - npm test && npm run cov && npm run coveralls; + - if [ $TARGET_ARCH == "x64" ]; then + if [ -z "$TRAVIS_TAG" ] && [ $TRAVIS_OS_NAME == "linux" ] && [ $NODE_VERSION == "0.12" ]; then + npm test && npm run cov && npm run coveralls; + else + npm test; + fi else - npm test; + echo "Not running tests because the binary is not built for 64-bit systems"; fi after_success: @@ -85,11 +103,11 @@ after_success: npm install -g node-pre-gyp; npm install -g aws-sdk; node lifecycleScripts/clean; - node-pre-gyp package; - node-pre-gyp publish; + node-pre-gyp package --target_arch=$TARGET_ARCH; + node-pre-gyp publish --target_arch=$TARGET_ARCH; fi - - if [ $TRAVIS_BRANCH == "master" ] && [ $TRAVIS_PULL_REQUEST == "false" ] && [ $TRAVIS_OS_NAME == "linux" ] && [ $NODE_VERSION == "4.1" ]; then + - if [ $TRAVIS_BRANCH == "master" ] && [ $TRAVIS_PULL_REQUEST == "false" ] && [ $TRAVIS_OS_NAME == "linux" ] && [ $NODE_VERSION == "4.1" ] && [ $TARGET_ARCH == "x64" ]; then .travis/deploy-docs.sh; fi diff --git a/lifecycleScripts/install.js b/lifecycleScripts/install.js index 0302d6743..4fc3363e8 100644 --- a/lifecycleScripts/install.js +++ b/lifecycleScripts/install.js @@ -81,6 +81,8 @@ function build() { var builder = "node-gyp"; var debug = (process.env.BUILD_DEBUG ? "--debug" : ""); var target = ""; + var arch = (process.env.TARGET_ARCH ? + "--arch=" + process.env.TARGET_ARCH : ""); var distUrl = ""; var runtime = ""; @@ -114,6 +116,7 @@ function build() { "rebuild", debug, target, + arch, distUrl, runtime ] From 920619e94d8b18673f401d543f4510b9b91608c0 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Fri, 1 Apr 2016 15:27:53 -0700 Subject: [PATCH 07/12] Make remotes self-freeing --- generate/input/descriptor.json | 42 ++++++++++++------- .../templates/manual/include/functions/copy.h | 2 + .../templates/manual/src/functions/copy.cc | 5 +++ generate/templates/partials/convert_to_v8.cc | 2 +- generate/templates/templates/class_content.cc | 36 +++++++++++----- generate/templates/templates/class_header.h | 11 +++-- 6 files changed, 68 insertions(+), 30 deletions(-) diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index 48eb6968d..724bc9951 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -143,7 +143,7 @@ }, "git_blob_id": { "return": { - "shouldDuplicate": true + "ownedByThis": true } }, "git_blob_rawcontent": { @@ -389,12 +389,12 @@ }, "git_commit_author": { "return": { - "shouldDuplicate": true + "ownedByThis": true } }, "git_commit_committer": { "return": { - "shouldDuplicate": true + "ownedByThis": true } }, "git_commit_create": { @@ -424,17 +424,17 @@ }, "git_commit_id": { "return": { - "shouldDuplicate": true + "ownedByThis": true } }, "git_commit_parent_id": { "return": { - "shouldDuplicate": true + "ownedByThis": true } }, "git_commit_tree_id": { "return": { - "shouldDuplicate": true + "ownedByThis": true } } } @@ -1594,12 +1594,12 @@ }, "git_reference_target": { "return": { - "shouldDuplicate": true + "ownedByThis": true } }, "git_reference_target_peel": { "return": { - "shouldDuplicate": true + "ownedByThis": true } } } @@ -1627,6 +1627,7 @@ }, "remote": { "cType": "git_remote", + "selfFreeing": true, "functions": { "git_remote_create": { "isAsync": false @@ -1711,6 +1712,11 @@ }, "isAsync": true }, + "git_remote_get_refspec": { + "return": { + "ownedByThis": true + } + }, "git_remote_list": { "args": { "out": { @@ -1748,6 +1754,9 @@ }, "git_remote_set_push_refspecs": { "ignore": true + }, + "git_remote_stats": { + "ownedByThis": true } } }, @@ -2182,7 +2191,7 @@ }, "git_tag_id": { "return": { - "shouldDuplicate": true + "ownedByThis": true } }, "git_tag_create_lightweight": { @@ -2222,7 +2231,7 @@ }, "git_tag_tagger": { "return": { - "shouldDuplicate": true + "ownedByThis": true } }, "git_tag_target": { @@ -2234,7 +2243,7 @@ }, "git_tag_target_id": { "return": { - "shouldDuplicate": true + "ownedByThis": true } }, "git_tag_delete": { @@ -2252,6 +2261,9 @@ } } }, + "transfer_progress": { + "dupFunction": "git_transfer_progress_dup" + }, "transport": { "cType": "git_transport", "needsForwardDeclaration": false, @@ -2281,7 +2293,7 @@ "functions": { "git_tree_entry_byid": { "return": { - "shouldDuplicate": true + "ownedByThis": true } }, "git_tree_entry_byindex": { @@ -2289,12 +2301,12 @@ }, "git_tree_entry_byname": { "return": { - "shouldDuplicate": true + "ownedByThis": true } }, "git_tree_entry_id": { "return": { - "shouldDuplicate": true + "ownedByThis": true } }, "git_tree_entrycount": { @@ -2302,7 +2314,7 @@ }, "git_tree_id": { "return": { - "shouldDuplicate": true + "ownedByThis": true } }, "git_tree_walk": { diff --git a/generate/templates/manual/include/functions/copy.h b/generate/templates/manual/include/functions/copy.h index 299d07fea..69c733464 100644 --- a/generate/templates/manual/include/functions/copy.h +++ b/generate/templates/manual/include/functions/copy.h @@ -15,4 +15,6 @@ const git_time *git_time_dup(const git_time *arg); const git_diff_delta *git_diff_delta_dup(const git_diff_delta *arg); const git_diff_file *git_diff_file_dup(const git_diff_file *arg); +void git_transfer_progress_dup(git_transfer_progress **out, const git_transfer_progress *arg); + #endif diff --git a/generate/templates/manual/src/functions/copy.cc b/generate/templates/manual/src/functions/copy.cc index 48d20ce38..d885f0ea4 100644 --- a/generate/templates/manual/src/functions/copy.cc +++ b/generate/templates/manual/src/functions/copy.cc @@ -10,3 +10,8 @@ const git_error *git_error_dup(const git_error *arg) { result->message = strdup(arg->message); return result; } + +void git_transfer_progress_dup(git_transfer_progress **out, const git_transfer_progress *arg) { + *out = (git_transfer_progress *)malloc(sizeof(git_transfer_progress)); + memcpy(*out, arg, sizeof(git_transfer_progress)); +} diff --git a/generate/templates/partials/convert_to_v8.cc b/generate/templates/partials/convert_to_v8.cc index 4c669ac76..d9b1d8830 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 }} {% if shouldDuplicate %}, true{% endif %}); + to = {{ cppClassName }}::New({{= parsedName =}}, {{ selfFreeing|toBool }} {% if ownedByThis %}, info.This(){% endif %}); {% endif %} } else { diff --git a/generate/templates/templates/class_content.cc b/generate/templates/templates/class_content.cc index 312c2ef28..720c0d1e2 100644 --- a/generate/templates/templates/class_content.cc +++ b/generate/templates/templates/class_content.cc @@ -24,15 +24,24 @@ using namespace v8; using namespace node; {% if cType %} - {{ cppClassName }}::{{ cppClassName }}({{ cType }} *raw, bool selfFreeing, bool shouldDuplicate) { - if (shouldDuplicate) { - {% if shouldAlloc %} - this->raw = ({{ cType }} *)malloc(sizeof({{ cType }})); - {{ dupFunction }}(this->raw, raw); + {{ cppClassName }}::{{ cppClassName }}({{ cType }} *raw, bool selfFreeing, Local owner) { + if (!owner.IsEmpty()) { + // if we have an owner, there are two options - either we duplicate the raw object + // (so we own the duplicate, and can self-free it) + // or we keep a handle on the owner so it doesn't get garbage collected + // while this wrapper is accessible + {% if dupFunction %} + {% if shouldAlloc %} + this->raw = ({{ cType }} *)malloc(sizeof({{ cType }})); + {{ dupFunction }}(this->raw, raw); + {% else %} + {{ dupFunction }}(&this->raw, raw); + {% endif %} + selfFreeing = true; {% else %} - {{ dupFunction }}(&this->raw, raw); + this->owner.Reset(owner); + this->raw = raw; {% endif %} - selfFreeing = true; } else { this->raw = raw; } @@ -117,17 +126,22 @@ using namespace node; {{ 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 + info.Length() >= 3 && !info[2].IsEmpty() && info[2]->IsObject() ? info[2]->ToObject() : Local() ); object->Wrap(info.This()); info.GetReturnValue().Set(info.This()); } - Local {{ cppClassName }}::New(const {{ cType }} *raw, bool selfFreeing, bool shouldDuplicate) { + Local {{ cppClassName }}::New(const {{ cType }} *raw, bool selfFreeing, Local owner) { Nan::EscapableHandleScope scope; - 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()); + Local argv[3] = { Nan::New((void *)raw), Nan::New(selfFreeing), owner }; + return scope.Escape( + Nan::NewInstance( + Nan::New({{ cppClassName }}::constructor_template), + owner.IsEmpty() ? 2 : 3, // passing an empty handle as part of the arguments causes a crash + argv + ).ToLocalChecked()); } NAN_METHOD({{ cppClassName }}::GetSelfFreeingInstanceCount) { diff --git a/generate/templates/templates/class_header.h b/generate/templates/templates/class_header.h index cc77f52b3..63abef7b9 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, bool shouldDuplicate = false); + static Local New(const {{ cType }} *raw, bool selfFreeing, Local owner = Local()); {%endif%} bool selfFreeing; @@ -82,10 +82,15 @@ class {{ cppClassName }} : public Nan::ObjectWrap { private: - + // owner of the object, in the memory management sense. only populated + // when using ownedByThis, and the type doesn't have a dupFunction + // CopyablePersistentTraits are used to get the reset-on-destruct behavior. + {%if not dupFunction %} + Nan::Persistent > owner; + {%endif%} {%if cType%} - {{ cppClassName }}({{ cType }} *raw, bool selfFreeing, bool shouldDuplicate = false); + {{ cppClassName }}({{ cType }} *raw, bool selfFreeing, Local owner = Local()); ~{{ cppClassName }}(); {%endif%} From 416d47f46bf8966b41acf3b8b7e2dc04af1d44cc Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Fri, 1 Apr 2016 16:24:22 -0700 Subject: [PATCH 08/12] Add remote / refspec tests --- test/tests/commit.js | 22 +++-------------- test/tests/remote.js | 46 +++++++++++++++++++++++++++++++++++ test/utils/garbage_collect.js | 20 +++++++++++++++ 3 files changed, 69 insertions(+), 19 deletions(-) create mode 100644 test/utils/garbage_collect.js diff --git a/test/tests/commit.js b/test/tests/commit.js index e770a0c1e..2a0f27595 100644 --- a/test/tests/commit.js +++ b/test/tests/commit.js @@ -2,6 +2,9 @@ var assert = require("assert"); var path = require("path"); var promisify = require("promisify-node"); var fse = promisify(require("fs-extra")); + +var garbageCollect = require("../utils/garbage_collect.js"); + var local = path.join.bind(path, __dirname); // Have to wrap exec, since it has a weird callback signature. @@ -9,25 +12,6 @@ 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; diff --git a/test/tests/remote.js b/test/tests/remote.js index ff7948cad..1490df229 100644 --- a/test/tests/remote.js +++ b/test/tests/remote.js @@ -2,6 +2,8 @@ var assert = require("assert"); var path = require("path"); var local = path.join.bind(path, __dirname); +var garbageCollect = require("../utils/garbage_collect.js"); + describe("Remote", function() { var NodeGit = require("../../"); var Repository = NodeGit.Repository; @@ -379,4 +381,48 @@ describe("Remote", function() { } }); }); + + it("is kept alive by refspec", function() { + var repo = this.repository; + var Remote = NodeGit.Remote; + + garbageCollect(); + var startSelfFreeingCount = Remote.getSelfFreeingInstanceCount(); + var startNonSelfFreeingCount = Remote.getNonSelfFreeingConstructedCount(); + + var resolve; + var promise = new Promise(function(_resolve) { resolve = _resolve; }); + + var remote; + + repo.getRemote("origin") + .then(function(_remote) { + remote = _remote; + setTimeout(resolve, 0); + }); + + return promise + .then(function() { + // make sure we have created one self-freeing remote + assert.equal(startSelfFreeingCount + 1, + Remote.getSelfFreeingInstanceCount()); + assert.equal(startNonSelfFreeingCount, + Remote.getNonSelfFreeingConstructedCount()); + var refspec = remote.getRefspec(0); + assert.equal("refs/heads/*", refspec.src()); + remote = null; + garbageCollect(); + // the refspec should be holding on to the remote + assert.equal(startSelfFreeingCount + 1, + Remote.getSelfFreeingInstanceCount()); + + assert.equal("refs/heads/*", refspec.src()); + + refspec = null; + garbageCollect(); + // the remote should be freed now + assert.equal(startSelfFreeingCount, + Remote.getSelfFreeingInstanceCount()); + }); + }); }); diff --git a/test/utils/garbage_collect.js b/test/utils/garbage_collect.js new file mode 100644 index 000000000..a288b99c8 --- /dev/null +++ b/test/utils/garbage_collect.js @@ -0,0 +1,20 @@ +// 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; + } +} + +module.exports = garbageCollect; From 3d3d0ba396b99b8fc49cea22a8e093eb5b017a73 Mon Sep 17 00:00:00 2001 From: John Haley Date: Wed, 6 Apr 2016 15:09:35 -0700 Subject: [PATCH 09/12] Use default VM image on AppVeyor --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 337d1250f..b8a98ec40 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,8 +1,8 @@ # appveyor file # http://www.appveyor.com/docs/appveyor-yml -# Try out "interactive-mode" os: Windows Server 2012 R2 +image: Visual Studio 2015 platform: - x86 From f4c39d758cafebb156ec42c12bb16c97f48b2ea7 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Thu, 7 Apr 2016 09:34:56 -0700 Subject: [PATCH 10/12] Fix revwalk test to pass oid instead of commit --- test/tests/revwalk.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/tests/revwalk.js b/test/tests/revwalk.js index 85d708303..2fc704e27 100644 --- a/test/tests/revwalk.js +++ b/test/tests/revwalk.js @@ -315,7 +315,7 @@ describe("Revwalk", function() { var walker = repository.createRevWalk(); repository.getMasterCommit().then(function(firstCommitOnMaster) { - walker.walk(firstCommitOnMaster, function(err, commit) { + walker.walk(firstCommitOnMaster.id(), function(err, commit) { if (err && err.errno === NodeGit.Error.CODE.ITEROVER) { return done(); } From 6580ecbb1fed8ef35301f3b71146717e0a0f06bb Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Thu, 7 Apr 2016 10:12:58 -0700 Subject: [PATCH 11/12] Fix submodule test Increase timeout, use a nodegit test repo, and pass oid instead of object returned by peel --- test/tests/submodule.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/tests/submodule.js b/test/tests/submodule.js index 002b4ef12..b8c9c32ec 100644 --- a/test/tests/submodule.js +++ b/test/tests/submodule.js @@ -107,9 +107,11 @@ describe("Submodule", function() { }); it("can setup and finalize submodule add", function() { + this.timeout(30000); + var repo = this.repository; - var submodulePath = "hellogitworld"; - var submoduleUrl = "https://github.com/githubtraining/hellogitworld.git"; + var submodulePath = "nodegittest"; + var submoduleUrl = "https://github.com/nodegit/test.git"; var submodule; var submoduleRepo; @@ -134,7 +136,7 @@ describe("Submodule", function() { return reference.peel(NodeGit.Object.TYPE.COMMIT); }) .then(function(commit) { - return submoduleRepo.createBranch("master", commit); + return submoduleRepo.createBranch("master", commit.id()); }) .then(function() { return submodule.addFinalize(); From 5bd483bf2ee2eda6e94fa7ea73610cfa2a86dfbe Mon Sep 17 00:00:00 2001 From: John Haley Date: Thu, 7 Apr 2016 14:09:16 -0700 Subject: [PATCH 12/12] Bump to 0.12.2 --- CHANGELOG.md | 12 ++++++++++++ README.md | 2 +- package.json | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea02d278c..774e768ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Change Log +## [0.12.2](https://github.com/nodegit/nodegit/releases/tag/v0.12.2) (2016-04-07) + +[Full Changelog](https://github.com/nodegit/nodegit/compare/v0.12.1...v0.12.2) + +# Added + +- We now provide 32-bit binaries for linux [PR #980](https://github.com/nodegit/nodegit/pull/980) + +# Bug fixes + +- Added memory clean up for references [PR #977](https://github.com/nodegit/nodegit/pull/977) and remotes [PR #981](https://github.com/nodegit/nodegit/pull/981) + ## [0.12.1](https://github.com/nodegit/nodegit/releases/tag/v0.12.1) (2016-03-30) [Full Changelog](https://github.com/nodegit/nodegit/compare/v0.12.0...v0.12.1) diff --git a/README.md b/README.md index dc7724e7f..6e8c09e2c 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ NodeGit -**Stable: 0.12.1** +**Stable: 0.12.2** ## Have a problem? Come chat with us! ## diff --git a/package.json b/package.json index ee0eaa747..526ab5873 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "nodegit", "description": "Node.js libgit2 asynchronous native bindings", - "version": "0.12.1", + "version": "0.12.2", "homepage": "http://nodegit.org", "keywords": [ "libgit2",