danger / danger-js

⚠️ Stop saying "you forgot to …" in code review
http://danger.systems/js/
MIT License
5.29k stars 367 forks source link

`danger local` git diff error #492

Open adam-moss opened 6 years ago

adam-moss commented 6 years ago

Hi,

When running danger local --dangerfile dangerfile.js I am getting:

Could not get diff from git between master and HEAD
{ Error: stdout maxBuffer exceeded
    at Socket.onChildStdout (child_process.js:325:14)
    at Socket.emit (events.js:160:13)
    at addChunk (_stream_readable.js:269:12)
    at readableAddChunk (_stream_readable.js:252:13)
    at Socket.Readable.push (_stream_readable.js:213:10)
    at Pipe.onread (net.js:599:20) cmd: 'git diff master...HEAD' }

For branch structure:

master -> develop -> build/danger

adam-moss commented 6 years ago

Additional to the above, and a total edge case, but the following error is raised when the repo is essentially empty (i.e. git init but no git commit has been performed):

Could not get diff from git between master and HEAD
{ Error: Command failed: git diff master...HEAD
fatal: ambiguous argument 'master...HEAD': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

    at ChildProcess.exithandler (child_process.js:272:12)
    at ChildProcess.emit (events.js:160:13)
    at maybeClose (internal/child_process.js:943:16)
    at Socket.stream.socket.on (internal/child_process.js:363:11)
    at Socket.emit (events.js:160:13)
    at Pipe._handle.close [as _onclose] (net.js:559:12)
  killed: false,
  code: 128,
  signal: null,
  cmd: 'git diff master...HEAD' }
orta commented 6 years ago

Lovely edge case, probably easy to do a check beforehand for some depth

orta commented 6 years ago

Also happened to me with a tag - I think - https://travis-ci.org/danger/danger-js/jobs/332522074#L758

$ DEBUG="*" danger local --dangerfile source/platforms/git/_tests/local_dangerfile_example.ts
You may have updated from Danger 2.x -> 3.x without updating from `danger` to `danger ci`.
  danger:process_runner Starting sub-process run with  +0ms
  danger:localGetDiff git diff master...HEAD +0ms
Could not get diff from git between master and HEAD
{ Error: Command failed: git diff master...HEAD
fatal: ambiguous argument 'master...HEAD': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
    at ChildProcess.exithandler (child_process.js:272:12)
    at ChildProcess.emit (events.js:160:13)
    at maybeClose (internal/child_process.js:943:16)
    at Socket.stream.socket.on (internal/child_process.js:363:11)
    at Socket.emit (events.js:160:13)
    at Pipe._handle.close [as _onclose] (net.js:559:12)
  killed: false,
  code: 128,
  signal: null,
  cmd: 'git diff master...HEAD' }

Didn't fail the build either

adam-moss commented 6 years ago

Heh well on the bright side at least it is not just me 😂

I wonder if your failure is because you're cloning the branch, are in a detached state, so don't have master to compare with?

ionutmiftode commented 6 years ago

Having the same issue here with big commits. any progress on this?

orta commented 6 years ago

There's https://github.com/danger/danger-js/pull/605 - which you're welcome to help out on 👍

rohit-gohri commented 5 years ago
Could not get diff from git between master and HEAD
{ Error: Command failed: git diff master...HEAD
fatal: ambiguous argument 'master...HEAD': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:

I received the same error. I was trying to use danger local for push events on Drone CI to send slack alerts from danger among other things. But it fails with this message:

+ yarn danger local
yarn run v1.9.4
$ /src/node_modules/.bin/danger local
Could not get diff from git between master and HEAD
/src/node_modules/danger/distribution/platforms/git/localGetDiff.js:17
throw new Error(data.toString());
^
Error: fatal: ambiguous argument 'master...HEAD': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
at Socket.<anonymous> (/src/node_modules/danger/distribution/platforms/git/localGetDiff.js:17:19)
at Socket.emit (events.js:182:13)
at addChunk (_stream_readable.js:283:12)
at readableAddChunk (_stream_readable.js:264:11)
at Socket.Readable.push (_stream_readable.js:219:10)
at Pipe.onStreamRead [as onread] (internal/stream_base_commons.js:94:17)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. 

The cloning step for the build was:

Initialized empty Git repository in /src/.git/
+ git fetch origin +refs/heads/danger2.0:
From https://github.com/smartprix/src
* branch danger2.0 -> FETCH_HEAD
* [new branch] danger2.0 -> origin/danger2.0
+ git checkout 69bafe7f7b63ad119663adc06fb6a8f16d1a618c -b danger2.0
Switched to a new branch 'danger2.0' 

I can use a custom clone step, I just don't know what is it that is missing. Any ideas? I tried adding a fetch master command, but that didn't help

orta commented 5 years ago

Danger local isn't for using on CI - that's what danger ci is for

rohit-gohri commented 5 years ago

Danger local isn't for using on CI - that's what danger pr is for

But that skips for non PRs, is there any way to run danger for non PR commits? Like for commits directly on master?

orta commented 5 years ago

Then yeah, that's what you'll need to set up - master in that repo needs to represent the before, and the current branch as the merged master

rohit-gohri commented 5 years ago

Then yeah, that's what you'll need to set up - master in that repo needs to represent the before, and the current branch as the merged master

Yeah, just did that. Fetching itself didn't create a local ref for master. Needed to run: git fetch origin master:master. That :master sets the local ref.

agrublev commented 5 years ago

Then yeah, that's what you'll need to set up - master in that repo needs to represent the before, and the current branch as the merged master

Yeah, just did that. Fetching itself didn't create a local ref for master. Needed to run: git fetch origin master:master. That :master sets the local ref.

THANK YOU! That fixed it :)

weitzman commented 4 years ago

Why does Danger care about having two refs? Can't it just run your rules on the current codebase? For my use case it doesn't need to know about git at all. All I depend on is --failOnErrors. Is Danger not the right tool for this?

orta commented 4 years ago

Danger uses the two refs to generate the diff for your code changes

Danger JS on the other hand relies on the GitHub api for that

anuroop15 commented 3 years ago

Then yeah, that's what you'll need to set up - master in that repo needs to represent the before, and the current branch as the merged master

Yeah, just did that. Fetching itself didn't create a local ref for master. Needed to run: git fetch origin master:master. That :master sets the local ref.

What if my repo doesn't contain a master branch??

orta commented 3 years ago

You're welcome to look at adding a PR which checks master exists, and if not to try main as the base

jeanpan commented 2 years ago

I got this error when I tried to run docker local.

$ danger local --verbose
Could not get diff from git between master and HEAD
/xxx/node_modules/danger/distribution/platforms/git/localGetDiff.js:21
            throw new Error(data.toString());
            ^

Error: fatal: ambiguous argument 'master...HEAD': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

    at Socket.<anonymous> (/xxx/node_modules/danger/distribution/platforms/git/localGetDiff.js:21:19)

However, we are using develop branch as the primary branch, not master. Does anyone know how to resolve the issue?

rohit-gohri commented 2 years ago

However, we are using develop branch as the primary branch, not master. Does anyone know how to resolve the issue?

There is a --base option for danger local. You can use that to provide the base branch

jeanpan commented 2 years ago

Oh good to know! Thank you @rohit-gohri!

Ph1Doc commented 1 year ago

However, we are using develop branch as the primary branch, not master. Does anyone know how to resolve the issue?

There is a --base option for danger local. You can use that to provide the base branch

Does it work now? I run this command in another branch to check local changes with dev

swift run danger-swift local -b dev --dangerfile ../../Dangerfile.swift --cwd ../../

Result:

Build complete! (1.12s)
Could not get diff from git between dev and HEAD
/snapshot/danger-js/distribution/platforms/git/localGetDiff.js:21
            throw new Error(data.toString());
            ^

Error: fatal: ambiguous argument 'dev...HEAD': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

    at Socket.<anonymous> (/snapshot/danger-js/distribution/platforms/git/localGetDiff.js:21:19)
    at Socket.emit (node:events:527:28)
    at addChunk (node:internal/streams/readable:315:12)
    at readableAddChunk (node:internal/streams/readable:289:9)
    at Socket.Readable.push (node:internal/streams/readable:228:10)
    at Pipe.onStreamRead (node:internal/stream_base_commons:190:23)

3.15.0 version