fix: only promisify raw natives so re-evaluation does not stack wrappers

generate/templates/templates/nodegit.js

@@ -9,7 +9,20 @@ try {
 
 var rawApi = require("node-gyp-build")(path.join(__dirname, ".."));
 
-var promisify = fn => fn && util.promisify(fn); // jshint ignore:line
+// The native binding is a process-wide singleton, so when this file is evaluated more than
+// once in the same process the methods on the shared classes are no longer the raw
+// callback-style natives: they are the promise-returning wrappers installed by an earlier
+// evaluation (util.promisify below, or an extension such as lookupWrapper). Re-promisifying
+// those makes every call emit a DEP0174 deprecation warning and leak a pending promise per
+// extra layer, so only ever wrap the raw natives — anything else is already promise-based
+// and is returned untouched.
+var promisify = fn => { // jshint ignore:line
+  if (typeof fn !== "function" ||
+      !Function.prototype.toString.call(fn).includes("[native code]")) {
+    return fn;
+  }
+  return util.promisify(fn);
+};
 
 // For disccussion on why `cloneDeep` is required, see:
 // https://github.com/facebook/jest/issues/3552

test/tests/reload.js

@@ -0,0 +1,28 @@
+var path = require("path");
+var childProcess = require("child_process");
+var local = path.join.bind(path, __dirname);
+
+describe("Reload", function() {
+  // lib/nodegit.js can be evaluated more than once in a single process while the native
+  // binding stays a process-wide singleton. Re-evaluations must not re-promisify the shared
+  // native methods: each extra util.promisify layer emits a DEP0174 deprecation warning on
+  // every call and leaks a pending promise. Run in a child process so this suite's own
+  // module cache is untouched.
+  it("does not re-promisify shared natives when re-evaluated in one process",
+    function(done) {
+      this.timeout(30000);
+
+      var fixture = local("../utils/reload_fixture.js");
+      childProcess.execFile(process.execPath, [fixture],
+        function(error, stdout, stderr) {
+          if (error) {
+            done(new Error(
+              "reload fixture exited with code " + error.code + "\n" + stdout + stderr
+            ));
+            return;
+          }
+
+          done();
+        });
+    });
+});

test/utils/reload_fixture.js

@@ -0,0 +1,63 @@
+// Evaluates lib/nodegit.js twice in one process (fresh JS module registry, shared native
+// binding), then exercises methods that earlier versions would re-promisify. Exits
+// non-zero if any DEP0174 deprecation warning is emitted.
+var path = require("path");
+
+var libDir = path.resolve(__dirname, "..", "..", "lib");
+var nodegitPath = path.join(libDir, "nodegit.js");
+var projectRepoPath = path.resolve(__dirname, "..", "..");
+
+var warningCount = 0;
+process.on("warning", function(warning) {
+  if (warning.code === "DEP0174") {
+    warningCount++;
+  }
+});
+
+// The native .node binding stays cached (it is a process-wide singleton no matter what);
+// only the package's JS modules are re-evaluated.
+function purgePackageJsFromCache() {
+  Object.keys(require.cache).forEach(function(key) {
+    var isNativeModule = key.slice(-5) === ".node";
+    var isPackageJs = key.indexOf(libDir) === 0 ||
+      key.indexOf("node-gyp-build") !== -1;
+
+    if (!isNativeModule && isPackageJs) {
+      delete require.cache[key];
+    }
+  });
+}
+
+var first = require(nodegitPath);
+
+purgePackageJsFromCache();
+var second = require(nodegitPath);
+
+if (second === first) {
+  console.error("fixture failed to re-evaluate nodegit");
+  process.exit(2);
+}
+
+// A promisify layer wrapped around an already promise-returning function emits DEP0174 at
+// call time, which is what we detect. Commit.lookup goes through the lookupWrapper
+// extension, the layer that historically got re-promisified.
+second.Repository.open(projectRepoPath)
+  .then(function(repo) {
+    return repo.getHeadCommit().then(function(commit) {
+      return second.Commit.lookup(repo, commit.id());
+    });
+  })
+  .catch(function(error) {
+    console.error("fixture could not exercise lookups: " + error);
+    process.exit(2);
+  })
+  .then(function() {
+    setTimeout(function() {
+      if (warningCount > 0) {
+        console.error("DEP0174 warnings after re-evaluation: " + warningCount);
+        process.exit(1);
+      }
+
+      process.exit(0);
+    }, 100);
+  });