Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

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

Merged
srajko merged 1 commit intonodegit:masternodegit/nodegit:masterfrom
rcjsuen:patch-3rcjsuen/nodegit:patch-3Copy head branch name to clipboard
Jan 27, 2017
Merged

Checkout's tree(*) function do not support Oid as a parameter#1190
srajko merged 1 commit intonodegit:masternodegit/nodegit:masterfrom
rcjsuen:patch-3rcjsuen/nodegit:patch-3Copy head branch name to clipboard

Conversation

@rcjsuen
Copy link
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
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
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
Morty Proxy This is a proxified and sanitized view of the page, visit original site.