Monkey Patch git ruby gem #111

glennsarti · 2017-02-22T22:29:53Z

Previously modulesync would throw an error when a file had a line ending warning
emitted during a diff. This is common on cross platform modules. This commit
monkey patches the git ruby gem to handle extra text before the actual diff
contents. A monkey patch was used as the git ruby gem does not seem to be
actively maintained, and it will some time before this issue is resolved and a
new gem available.

Git ruby gem issue is ruby-git/ruby-git#326

glennsarti · 2017-02-22T22:31:11Z

Repro case:

Download a repository which contains a file with CRLF endings on an operating system that uses LF endings
Extracting the diff will throw a NoMethodError: undefined method '[]' for nil:NilClass error in method process_full_diff

Example output;

Overriding any local changes to repositories in modules
Not managing Rakefile in puppetlabs-chocolatey
Not managing spec/spec_helper.rb in puppetlabs-chocolatey
Using no-op. Files in puppetlabs-chocolatey may be changed but will not be committed.
Files changed:
Error while updating puppetlabs-chocolatey
bundler: failed to load command: msync (/mnt/c/Source/modulesync_configs/.bundle/bash/ruby/2.3.0/bin/msync)
NoMethodError: undefined method `[]' for nil:NilClass
  /mnt/c/Source/modulesync_configs/.bundle/bash/ruby/2.3.0/gems/git-1.3.0/lib/git/diff.rb:151:in `block in process_full_diff'
  /mnt/c/Source/modulesync_configs/.bundle/bash/ruby/2.3.0/gems/git-1.3.0/lib/git/diff.rb:134:in `each'
  /mnt/c/Source/modulesync_configs/.bundle/bash/ruby/2.3.0/gems/git-1.3.0/lib/git/diff.rb:134:in `process_full_diff'
  /mnt/c/Source/modulesync_configs/.bundle/bash/ruby/2.3.0/gems/git-1.3.0/lib/git/diff.rb:109:in `process_full'
  /mnt/c/Source/modulesync_configs/.bundle/bash/ruby/2.3.0/gems/git-1.3.0/lib/git/diff.rb:68:in `each'
  /mnt/c/Source/modulesync_configs/.bundle/bash/ruby/2.3.0/bundler/gems/modulesync-899483f48304/lib/modulesync/git.rb:162:in `update_noop'
  /mnt/c/Source/modulesync_configs/.bundle/bash/ruby/2.3.0/bundler/gems/modulesync-899483f48304/lib/modulesync.rb:114:in `manage_module'
  /mnt/c/Source/modulesync_configs/.bundle/bash/ruby/2.3.0/bundler/gems/modulesync-899483f48304/lib/modulesync.rb:133:in `block in update'

The reason it fails because the code at; https://github.com/schacon/ruby-git/blob/master/lib/git/diff.rb#L139-L151 expects the first line to contain the filename whereas raw diff comes back as;

warning: CRLF will be replaced by LF in examples/init.pp.
The file will have its original line endings in your working directory.
diff --git a/.gitattributes b/.gitattributes
index 900ea0c..02d4646 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1,5 +1,5 @@
 #This file is generated by ModuleSync, do not edit.

Previously modulesync would throw an error when a file had a line ending warning emitted during a diff. This is common on cross platform modules. This commit monkey patches the git ruby gem to handle extra text before the actual diff contents. A monkey patch was used as the git ruby gem does not seem to be actively maintained, and it will some time before this issue is resolved and a new gem available. As the monkey_patch can contain code from other parties, which does not conform to module_sync's rubocopy styles, this file should is ignored explicitly in rubocops configuration.

rnelson0

I think we should open an issue to track this, to see when the git gem is patched and to pay down our tech debt.

rnelson0 · 2017-02-22T23:16:58Z

lib/monkey_patches.rb

+      full_diff_utf8_encoded = @full_diff.encode("UTF-8", "binary", {
+    :invalid => :replace,
+    :undef => :replace
+  })


Some funky alignment here.

Yeah...That was a copy-n-paste from the git ruby gem. It has a combo of spaces and tabs :-( I was in two minds to fix it. I went with to keep it the same as the original as much as possible so it's easier to see where the patch is by comparing this code and the original.

However I'm happy to go through it and modify the alignments.

If that's from the original it makes sense to keep it.

glennsarti · 2017-02-22T23:46:30Z

@rnelson0 Did you mean an issue on this repo or the git ruby gem? Already raised ruby-git/ruby-git#326 to track it.

glennsarti · 2017-02-22T23:48:45Z

And I'm not a fan of monkey patching either, but I couldn't see any other way of getting this done without a major overhaul. This may also fix #45 too

rnelson0 · 2017-02-23T00:26:29Z

An issue on this gem, so we don't forget about it.

glennsarti · 2017-02-23T03:37:12Z

Raised Issue #112

glennsarti · 2017-02-23T05:40:26Z

Thanks @rnelson0 !

This was referenced Feb 22, 2017

(maint) Allow gitattributes to be configurable puppetlabs/modulesync_configs#128

Merged

(maint) Add puppetlabs-puppet_agent as a managed module puppetlabs/modulesync_configs#129

Merged

glennsarti force-pushed the ticket/maint/monkey_path_git_gem branch from c3eec28 to 55bd5ae Compare February 22, 2017 23:00

rnelson0 requested changes Feb 22, 2017

View reviewed changes

rnelson0 approved these changes Feb 23, 2017

View reviewed changes

glennsarti mentioned this pull request Feb 23, 2017

Diff fails with NoMethodError: undefined method '[]' for nil:NilClass when line ending warnings are emitted #112

Open

rnelson0 merged commit 7ddc50d into voxpupuli:master Feb 23, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Monkey Patch git ruby gem #111

Monkey Patch git ruby gem #111

glennsarti commented Feb 22, 2017

glennsarti commented Feb 22, 2017

rnelson0 left a comment

rnelson0 Feb 22, 2017

glennsarti Feb 22, 2017 •

edited

Loading

rnelson0 Feb 23, 2017

glennsarti commented Feb 22, 2017

glennsarti commented Feb 22, 2017

rnelson0 commented Feb 23, 2017

glennsarti commented Feb 23, 2017 •

edited

Loading

glennsarti commented Feb 23, 2017

Monkey Patch git ruby gem #111

Monkey Patch git ruby gem #111

Conversation

glennsarti commented Feb 22, 2017

glennsarti commented Feb 22, 2017

rnelson0 left a comment

Choose a reason for hiding this comment

rnelson0 Feb 22, 2017

Choose a reason for hiding this comment

glennsarti Feb 22, 2017 • edited Loading

Choose a reason for hiding this comment

rnelson0 Feb 23, 2017

Choose a reason for hiding this comment

glennsarti commented Feb 22, 2017

glennsarti commented Feb 22, 2017

rnelson0 commented Feb 23, 2017

glennsarti commented Feb 23, 2017 • edited Loading

glennsarti commented Feb 23, 2017

glennsarti Feb 22, 2017 •

edited

Loading

glennsarti commented Feb 23, 2017 •

edited

Loading