Danger: Danger fails when Github PR contains binary files

Created on 27 Dec 2018  路  8Comments  路  Source: danger/danger

Report

What did you do?

In my GitHub PR, I have added a PDF file my_image.pdf. Danger ran on CircleCI with bundle exec danger --verbose

Image used in PR is my_image.pdf

What did you expect to happen?

Danger should run without any error & print the message successfully

What happened instead?

Danger failed with below error

+ ./ci/bundle.sh exec danger --verbose
+ export GEM_HOME=vendor/gem
+ GEM_HOME=vendor/gem
+ export GEM_PATH=vendor/gem
+ GEM_PATH=vendor/gem
+ '[' '!' -x vendor/gem/bin/bundle ']'
+ ./vendor/gem/bin/bundle exec danger --verbose
bundler: failed to load command: danger (/Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/bin/danger)
Danger::DSLError: 
[!] Invalid `Dangerfile` file: invalid byte sequence in UTF-8. Updating the Danger gem might fix the issue. Your Danger version: 5.15.0, latest Danger version: 6.0.1

 #  from Dangerfile:1
 #  -------------------------------------------
 >  updated_files_in_pr = git.modified_files
 #  message "#{updated_files_in_pr}"
 #  -------------------------------------------

  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/git-1.5.0/lib/git/diff.rb:135:in `split'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/git-1.5.0/lib/git/diff.rb:135:in `process_full_diff'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/git-1.5.0/lib/git/diff.rb:109:in `process_full'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/git-1.5.0/lib/git/diff.rb:68:in `each'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/danger-5.15.0/lib/danger/danger_core/plugins/dangerfile_git_plugin.rb:75:in `select'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/danger-5.15.0/lib/danger/danger_core/plugins/dangerfile_git_plugin.rb:75:in `modified_files'
  Dangerfile:1:in `eval_file'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/danger-5.15.0/lib/danger/danger_core/dangerfile.rb:297:in `eval'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/danger-5.15.0/lib/danger/danger_core/dangerfile.rb:297:in `eval_file'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/danger-5.15.0/lib/danger/danger_core/dangerfile.rb:200:in `block in parse'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/danger-5.15.0/lib/danger/danger_core/dangerfile.rb:197:in `instance_eval'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/danger-5.15.0/lib/danger/danger_core/dangerfile.rb:197:in `parse'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/danger-5.15.0/lib/danger/danger_core/dangerfile.rb:273:in `run'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/danger-5.15.0/lib/danger/danger_core/executor.rb:29:in `run'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/danger-5.15.0/lib/danger/commands/runner.rb:72:in `run'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/claide-1.0.2/lib/claide/command.rb:334:in `run'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/gems/danger-5.15.0/bin/danger:5:in `<top (required)>'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/bin/danger:23:in `load'
  /Users/distiller/AccountName/repo-name/vendor/bundle/ruby/2.4.0/bin/danger:23:in `<top (required)>'
Exited with code 1

Your Environment

  • Which CI are you running on? -> CircleCI
  • Are you running the latest version of Danger? -> 5.15.0
  • What is your Dangerfile?

    updated_files_in_pr = git.modified_files
    message "#{updated_files_in_pr}"
    

Most helpful comment

Hi! I was having the same issue and this bugfix PR branch for ruby-git fixed the issue for us. Also contains an excellent explanation of the problem. Workaround was implemented by adding this to our Gemfile:

# Temporary workaround for bug in binary file diffing
# https://github.com/danger/danger/issues/1055
# https://github.com/ruby-git/ruby-git/pull/405
gem 'git', :git => 'https://github.com/jcouball/ruby-git.git', :branch => 'normalize_encoding'

All 8 comments

I can't reproduce this on v 5.15.0 or Github Enterprise, any chance you could share the stack trace or create a failing unit test I could take a look at?

@daniel-beard Thanks for checking the issue on your side. I tried updating the DangerFile as mentioned above to verify this issue on 5.15.0 but I am still facing the same issue. I have also added the stack trace as requested. Can you please have a look at it? Thanks

Hi! I was having the same issue and this bugfix PR branch for ruby-git fixed the issue for us. Also contains an excellent explanation of the problem. Workaround was implemented by adding this to our Gemfile:

# Temporary workaround for bug in binary file diffing
# https://github.com/danger/danger/issues/1055
# https://github.com/ruby-git/ruby-git/pull/405
gem 'git', :git => 'https://github.com/jcouball/ruby-git.git', :branch => 'normalize_encoding'

Hi @interstateone . Thank you for the workaround. After using it, my issue got fixed. It would be great to have https://github.com/ruby-git/ruby-git/pull/405 gets merged in & Danger updates the git version :)

Any update on whether this will ever get resolved? We ran into this last month and worked around it but it would be great to have a proper fix in place.

You're welcome to take a look 馃憤

An option might to to make your own release of ruby-git with that commit and have danger rely on that

Ruby-git just merged the PR (https://github.com/ruby-git/ruby-git/pull/405) to the master.

New version of Danger with updated Ruby-git is released, i believe we can close this issue now.

https://github.com/danger/danger/pull/1181

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Antondomashnev picture Antondomashnev  路  7Comments

nikolaykasyanov picture nikolaykasyanov  路  8Comments

dzenbot picture dzenbot  路  7Comments

oct0f1sh picture oct0f1sh  路  4Comments

josete89 picture josete89  路  10Comments