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

Rewrite local git detection using git command #132

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dlackty
Copy link

@dlackty dlackty commented Jul 7, 2016

We recently encountered a error when we tried to run node-coveralls on a Git tagged revision without a branch. I know it sounds a bit of weird to do that, but it's the default behavior of Phabricator's Diffusion code review tool.

When debugging this issue, I found that we can get required information just using git command, so I rewrote the function and it works on our CircleCI setup.

This is error log:

fs.js:549
  return binding.open(pathModule._makeLong(path), stringToFlags(flags), mode);
                                                  ^

Error: ENOENT: no such file or directory, open '/home/ubuntu/blog/.git/refs/heads/master'
    at Error (native)
    at Object.fs.openSync (fs.js:549:18)
    at Object.fs.readFileSync (fs.js:397:15)
    at detectLocalGit (/home/ubuntu/blog/node_modules/coveralls/lib/detectLocalGit.js:26:19)
    at getBaseOptions (/home/ubuntu/blog/node_modules/coveralls/lib/getOptions.js:92:43)
    at Object.getOptions (/home/ubuntu/blog/node_modules/coveralls/lib/getOptions.js:149:3)
    at handleInput (/home/ubuntu/blog/node_modules/coveralls/lib/handleInput.js:7:8)
    at Socket.<anonymous> (/home/ubuntu/blog/node_modules/coveralls/bin/coveralls.js:16:5)
    at emitNone (events.js:72:20)
    at Socket.emit (events.js:166:7)

cat ./coverage/lcov.info | ./node_modules/.bin/coveralls returned exit code 1

Copy link
Owner

@nickmerwin nickmerwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dlackty thank you for your patience on this one. It seems the changes are causing build failures, could you take a look at those?

@gugu
Copy link

gugu commented Dec 23, 2016

@nickmerwin after git gc it puts that file to .git/packed-refs

So I vote for this pull request

@XhmikosR
Copy link
Contributor

XhmikosR commented Nov 20, 2019

The idea is good; it's better to rely on git itself than custom code.

My thoughts:

  1. we need to make sure everything works properly on all OS'es
  2. this might result in a slowness since we'll be spawning 2 git processes synchronously

@jtwebman
Copy link

jtwebman commented Mar 6, 2022

Since this library doesn't seem to be supported anymore I fix a bunch of things on a fork if you want to check it out and are still pulling the library into your packages: https://github.com/jtwebman/coveralls-next I am very open to changes like these.

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

Successfully merging this pull request may close these issues.

None yet

5 participants