Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Commit.find Failure #92

Open
cjbarth opened this issue Jan 20, 2017 · 2 comments
Open

Commit.find Failure #92

cjbarth opened this issue Jan 20, 2017 · 2 comments

Comments

@cjbarth
Copy link

cjbarth commented Jan 20, 2017

Commit.find is failing because it is trying to call git ref-list --pretty=raw --max-count=1 The reason it isn't using the required parameter of <commit-id> is because the Commit.find call is in a callback for fs.readFile which returns an error because the folder .git/refs/heads doesn't contain anything (well some folders, but no files), so fs.readFile returns an error, which the callback call to Commit.find completely ignores and thereby fails. I feel like instead of going through all that effort to find the Head.current one could just call git ref-list --pretty=raw --max-count=1 HEAD and be done with it.

I'm calling git(path/to/repo).branch(callback) that calls Repo.prototype.branch which calls Head.current(this, callback) because of my not having specified a name.

@cjbarth
Copy link
Author

cjbarth commented Jan 20, 2017

I've changed

    Head.current = function(repo, callback) {
      return fs.readFile(repo.dot_git + "/HEAD", function(err, data) {
        var branch, m, ref;
        if (err) {
          return callback(err);
        }
        ref = /ref: refs\/heads\/([^\s]+)/.exec(data);
        if (!ref) {
          return callback(new Error("Current branch is not a valid branch."));
        }
        m = ref[0], branch = ref[1];
        return fs.readFile(repo.dot_git + "/refs/heads/" + branch, function(err, id) {
          return Commit.find(repo, id, function(err, commit) {
            if (err) {
              return callback(err);
            }
            return callback(null, new Head(branch, commit));
          });
        });
      });
    };

to

    Head.current = function(repo, callback) {
      return fs.readFile(repo.dot_git + "/HEAD", function(err, data) {
        var branch, m, ref;
        if (err) {
          return callback(err);
        }
        ref = /ref: refs\/heads\/([^\s]+)/.exec(data);
        if (!ref) {
          return callback(new Error("Current branch is not a valid branch."));
        }
        m = ref[0], branch = ref[1];
        return Commit.find(repo, "HEAD", function(err, commit) {
          if (err) {
            return callback(err);
          }
          return callback(null, new Head(branch, commit));
        });
      });
    };

And the problem is resolved. Would you be open for a PR for such a change, or do you see something wrong with this approach?

@cjbarth
Copy link
Author

cjbarth commented Feb 14, 2017

I've found that the assumption that files will be in refs/heads is flawed: https://git-scm.com/book/en/v2/Git-Internals-Maintenance-and-Data-Recovery

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant