Fix commit_create_cb

julianmesa-gitkraken · julianmesa-gitkraken · commit 8d159d8d743a · 2022-11-09T14:06:41.000+01:00
The definition was wrong parents should be an array of Commits not Oids.
Also we are copying the Oid in the callback so we are not returning the Oid so we must not set selfFreeing to false.
diff --git a/generate/input/callbacks.json b/generate/input/callbacks.json
@@ -168,7 +168,7 @@
       },
       {
         "name": "parents",
-        "cType": "const git_oid * []"
+        "cType": "const git_commit * []"
       },
       {
         "name": "payload",
diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json
@@ -3142,10 +3142,10 @@
             },
             {
               "name": "parents",
-              "cType": "const git_oid **",
+              "cType": "const git_commit **",
               "cppClassName": "Array",
               "jsClassName": "Array",
-              "arrayElementCppClassName": "GitOid",
+              "arrayElementCppClassName": "GitCommit",
               "arrayLengthArgumentName": "parent_count"
             },
             {
diff --git a/generate/templates/partials/configurable_callbacks.cc b/generate/templates/partials/configurable_callbacks.cc
@@ -145,11 +145,11 @@
             else if (!result->IsNull() && !result->IsUndefined()) {
               {% if _return.isOutParam %}
                 {{ _return.cppClassName }}* wrapper = Nan::ObjectWrap::Unwrap<{{ _return.cppClassName }}>(Nan::To<v8::Object>(result).ToLocalChecked());
-                wrapper->selfFreeing = false;
 
                 {% if _return.cppClassName == "GitOid" %}
                   git_oid_cpy(baton->{{ _return.name }}, wrapper->GetValue());
                 {% else %}
+                  wrapper->selfFreeing = false;
                   *baton->{{ _return.name }} = wrapper->GetValue();
                 {% endif %}
                 baton->result = {{ field.return.success }};
@@ -185,11 +185,11 @@
               else if (!result->IsNull() && !result->IsUndefined()) {
                 {% if _return.isOutParam %}
                   {{ _return.cppClassName }}* wrapper = Nan::ObjectWrap::Unwrap<{{ _return.cppClassName }}>(Nan::To<v8::Object>(result).ToLocalChecked());
-                  wrapper->selfFreeing = false;
 
                   {% if _return.cppClassName == "GitOid" %}
                     git_oid_cpy(baton->{{ _return.name }}, wrapper->GetValue());
                   {% else %}
+                    wrapper->selfFreeing = false;
                     *baton->{{ _return.name }} = wrapper->GetValue();
                   {% endif %}
                   baton->result = {{ field.return.success }};
diff --git a/lib/repository.js b/lib/repository.js
@@ -623,11 +623,11 @@ Repository.prototype.createCommitBuffer = function(
  * @param {Signature} committer
  * @param {String} message
  * @param {Tree|Oid|String} Tree
- * @param {Array} parents
+ * @param {Array of Commits} parents
  * @param {Function} onSignature Callback to be called with string to be signed
  * @return {Oid} The oid of the commit
  */
-Repository.prototype.createCommitWithSignature = function(
+Repository.prototype.createCommitWithSignature = async function(
   updateRef,
   author,
   committer,
@@ -636,32 +636,32 @@ Repository.prototype.createCommitWithSignature = function(
   parents,
   onSignature
 ) {
+  let repo = this;
+  let newParents = [];
+  let skippedSigning = false;
 
-  var repo = this;
-  var promises = [];
-  var commitContent;
-  var skippedSigning;
-
-  parents = parents || [];
-
-  promises.push(repo.getTree(tree));
+  if (!(tree instanceof Tree)) {
+    tree = await repo.getTree(tree);
+  }
 
-  parents.forEach(function(parent) {
-    promises.push(repo.getCommit(parent));
+  let showDeprecationWarning = false;
+  parents.forEach(async (parent) => {
+    if (!(parent instanceof Commit)) {
+      newParents.push(await repo.getCommit(parent));
+      showDeprecationWarning = true;
+    } else {
+      newParents.push(parent);
+    }
   });
 
-  const createCommitPromise = Promise.all(promises).then(function(results) {
-    tree = results[0];
-
-    // Get the normalized values for our input into the function
-    var parentsLength = parents.length;
-    parents = [];
+  if (showDeprecationWarning) {
+    console.warn('DeprecationWarning: Parents in Repository.createCommitWithSignature should be an array of Commits instead an array of Oid|Strings');
+  }
 
-    for (var i = 0; i < parentsLength; i++) {
-      parents.push(results[i + 1]);
-    }
+  parents = newParents;
 
-    return Commit.createBuffer(
+  const createCommitPromise = async () => {
+    let commitContent = await Commit.createBuffer(
       repo,
       author,
       committer,
@@ -671,13 +671,12 @@ Repository.prototype.createCommitWithSignature = function(
       parents.length,
       parents
     );
-  }).then(function(commitContentResult) {
-    commitContent = commitContentResult;
     if (!commitContent.endsWith("\n")) {
       commitContent += "\n";
     }
-    return onSignature(commitContent);
-  }).then(function({ code, field, signedData }) {
+
+    const { code, field, signedData } = await onSignature(commitContent);
+
     switch (code) {
       case NodeGit.Error.CODE.OK:
         return Commit.createWithSignature(
@@ -708,32 +707,28 @@ Repository.prototype.createCommitWithSignature = function(
         throw error;
       }
     }
-  });
+  }
 
   if (!updateRef) {
-    return createCommitPromise;
+    return createCommitPromise();
   }
 
-  return createCommitPromise
-    .then(function(commitOid) {
-      if (skippedSigning) {
-        return commitOid;
-      }
+  const commitOid = await createCommitPromise();
+  if (skippedSigning) {
+    return commitOid;
+  }
 
-      return repo.getCommit(commitOid)
-        .then(function(commitResult) {
-          return Reference.updateTerminal(
-            repo,
-            updateRef,
-            commitOid,
-            getReflogMessageForCommit(commitResult),
-            committer
-          );
-        })
-        .then(function() {
-          return commitOid;
-        });
-      });
+  const commitResult = await repo.getCommit(commitOid)
+
+  await Reference.updateTerminal(
+    repo,
+    updateRef,
+    commitOid,
+    getReflogMessageForCommit(commitResult),
+    committer
+  );
+
+  return commitOid;
 };
 
 /**

Original file line number	Diff line number	Diff line change
`@@ -168,7 +168,7 @@`
`168`	`168`	`},`
`169`	`169`	`{`
`170`	`170`	`"name": "parents",`
`171`		`- "cType": "const git_oid * []"`
	`171`	`+ "cType": "const git_commit * []"`
`172`	`172`	`},`
`173`	`173`	`{`
`174`	`174`	`"name": "payload",`
Original file line number	Diff line number	Diff line change
`@@ -3142,10 +3142,10 @@`
`3142`	`3142`	`},`
`3143`	`3143`	`{`
`3144`	`3144`	`"name": "parents",`
`3145`		`- "cType": "const git_oid **",`
	`3145`	`+ "cType": "const git_commit **",`
`3146`	`3146`	`"cppClassName": "Array",`
`3147`	`3147`	`"jsClassName": "Array",`
`3148`		`- "arrayElementCppClassName": "GitOid",`
	`3148`	`+ "arrayElementCppClassName": "GitCommit",`
`3149`	`3149`	`"arrayLengthArgumentName": "parent_count"`
`3150`	`3150`	`},`
`3151`	`3151`	`{`