gift icon indicating copy to clipboard operation
gift copied to clipboard

Commit.find Failure

Open cjbarth opened this issue 8 years ago • 2 comments

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 avatar Jan 20 '17 14:01 cjbarth

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 avatar Jan 20 '17 15:01 cjbarth

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

cjbarth avatar Feb 14 '17 13:02 cjbarth