Skip to content

Checkout's tree(*) function do not support Oid as a parameter#1190

Merged
srajko merged 1 commit into
nodegit:masterfrom
rcjsuen:patch-3
Jan 27, 2017
Merged

Checkout's tree(*) function do not support Oid as a parameter#1190
srajko merged 1 commit into
nodegit:masterfrom
rcjsuen:patch-3

Conversation

@rcjsuen
Copy link
Copy Markdown
Member

@rcjsuen rcjsuen commented Jan 17, 2017 •
edited
Loading

The docs incorrectly spec that an Oid can be used as a treeish but the libgit2 git_checkout_tree API actually only supports "a commit, tag or tree".

The following code will reproduce the problem.

> node bug.js
[Error: Object to checkout does not match repository]

var git = require("nodegit");
git.Repository.open("myrepo")
.then(function(repo) {
  return repo.getTagByName('v1.0.0').then(function(tag) {
    git.Checkout.tree(repo, tag.id(), { checkoutStrategy: git.Checkout.STRATEGY.SAFE_CREATE })
    .then(function() {
      console.log("ok");
    }).catch(function(err) {
      console.log(err);
    });
  });
});

@srajko
Copy link
Copy Markdown
Collaborator

srajko commented Jan 26, 2017

@rcjsuen Thanks for the doc fix! It's a little scary that we're letting the Oid get all the way to git_checkout_tree, which is expecting a git_object (and a git_oid is not a git_object) :-(

Checkout.tree does seem to take a String though - in your example, you could try tag.id().toString(). Could I bother you to replace Oid with String in the docs, to reflect that?

@rcjsuen
Correct parameter documentation for tree(*)
ffbd6a3 
The treeish parameter claims that an Oid is valid but the underlying
git_checkout_tree C api does not actually support Oid as a parameter.
The JavaScript function does however take a String. Fix the
documentation to replace Oid with String as a valid parameter.
@rcjsuen
Copy link
Copy Markdown
Member Author

rcjsuen commented Jan 27, 2017

Thanks for the code review, @srajko! I have updated the checkout.js as you requested.

Thanks for offering a solution as well. Though I don't actually need it as this was actually a bug reported by @deowk on Gitter. :P

@srajko srajko merged commit 5769070 into nodegit:master Jan 27, 2017
@rcjsuen rcjsuen deleted the patch-3 branch January 27, 2017 20:25
@johnhaley81 johnhaley81 changed the title Checkout's tree(*) function does not support Oid as a parameter Checkout's tree(*) function do not support Oid as a parameter Feb 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rcjsuen @srajko