git apply fails depending on git configuration

[mattbrictson]

My ~/.gitconfig had the setting autocrlf = input.

For some reason this caused mini_portile to fail to apply patches with git apply. I could not install nokogiri due to the problem. sparklemotion/nokogiri#1095

The solution in this case was to change my git configuration by removing the autocrlf setting.

Is there a better way to address this issue?

[sorah]

git apply -c 'autocrlf=false' ?

[knu]

Maybe changing the command line to git -c core.autocrlf=false apply ... or something like that will fix the issue.

[knu]

Or ENV['GIT_CONFIG'] = '/dev/null' could probably work as well.
Sorry for the noise.

[b-dean]

FWIW, I don't think it's a good idea to depend on git being installed at all. 4f4abd5 changed it from using patch to using git apply, but even patch is probably a bad idea. The whole point of mini_portile is make it easy to compile on different platforms but last I checked, git wasn't included with the Ruby devkit on Windows. Maybe it'd be better do do one of the following:

• compile patch or git or something else from source and use that
• use some pure ruby implementation of patch (if such an animal exists).

[luislavena]

@b-dean the entire idea of applying patches is a bad idea.

if you go back to the readme of mini_portile will see is not aimed for you to put at the gem installation process (make it part of extconf) but instead for you to build and target a specific version of the dependency:

https://github.com/luislavena/mini_portile#another-port-system-srsly

Building the dependencies at runtime has never been the intention of mini_portile, as is not a replacement of end-users port system and most definitely will fail, simply because it assumes a developer machine with certain tools installed.

RubyInstaller's DevKit do not bundle Git or patch, precisely because we don't use patch while building Ruby.

This git dependency is definitely a side-effect that we need to workout and need more thinking on a proper solution.

[martinb3]

For what it's worth, many folks bundling mini_portile aren't declaring the git dependency at all. This has broken nokogiri upstream for many, many folks.

[luislavena]

For what it's worth, many folks bundling mini_portile aren't declaring the git dependency at all.

I think you mean the need to have git for applying patches work, correct?

I don't think the dependency on mini_portile is the issue, but the usage of patching itself.

I'm really sorry people get hit by this issue, definitely this particular usage of mini_portile was not part of the original intention of the tool.

Will take a look to ways to workaround this issue in mini_portile, perhaps raising an exception when patch is being used and git is not found (or fall back to patch before that).

But projects depending on patching will need to state its dependencies for now.

Again, really sorry for the inconvenience this has caused.

[martinb3]

Hi @luislavena,

No problem at all. I totally understand mini_portile was not intended to be used this way. I just figured I'd make that previous comment so you knew what was happening. I figure you're going to hear about it from folks coming to report it as a bug, so I figured you should be aware of the impact. It sounds like @flavorjones has a workaround for now, by pinning the mini_portile gem to an older version.

Cheers,
@martinb3

[flavorjones]

For now Nokogiri will monkey-patch mini_portile until we can figure out the right thing to do here.

[jonforums]

As in my note to this nokogiri commit is updating the mini_portile monkey patch to check for git apply availability before assuming patch.exe a better band-aid than the current band-aid?

[luislavena]

@jonforums will keep the conversation here instead of nokogiri's commit for the sake of my mental sanity.

The reason patch.exe was not bundled or used with RubyInstaller's recipes or DevKit was, as you pointed out, the elevation of privileges that it triggered.

Seems there is a workaround for that since then:

http://irq5.io/2011/06/26/gnu-patch-and-windows-uac/

[jonforums]

@luislavena the folks at http://www.sourcetreeapp.com/ have chosen the route of bundling patch.exe and the patch.exe.manifest workaround.

I've not played with it enough to see if I like it, but one issue remains that I do dislike: patch.exe has a dependency on the cygwin-based msys/msys2 emulation DLL. That's likely OK for our DevKit solution in which we bring DevKit artifacts on and off PATH only during a native gem install. But I think a solution that has patch.exe and it's cygwin-DLL on PATH all the time is a bad idea due to potential, hard-to-find conflicts with other apps. It just makes a users system too flakey and gives MRI on Windows and undeserved bad reputation.

I'm going to look more into it, but I think removing the need for Nokogiri's monkey patch by a guarded patch capability in mini_portile may make sense. If git apply is available, use it. If not and patch is available, use it. If neither, give an error message.

I'm also initially positive on us adding msys2 patch.exe and patch.exe.manifest to our updated DevKit's as in this RubyInstaller pull request.

Thoughts?

[flavorjones]

Please note that nokogiri's patch does not affect windows users. Nokogiri
packages cross compiled DLLs inside its windows gems.
On May 14, 2014 6:15 PM, "Jon" notifications@github.com wrote:

@luislavena https://github.com/luislavena the folks at
http://www.sourcetreeapp.com/ have chosen the route of bundling patch.exeand the
patch.exe.manifest workaround.

I've not played with it enough to see if I like it, but one issue remains
that I do dislike: patch.exe has a dependency on the cygwin-based
msys/msys2 emulation DLL. That's likely OK for our DevKit solution in which
we bring DevKit artifacts on and off PATH only during a native gem
install. But I think a solution that has patch.exe and it's cygwin-DLL on
PATH all the time is a bad idea due to potential, hard-to-find conflicts
with other apps. It just makes a users system too flakey and gives MRI on
Windows and undeserved bad reputation.

I'm going to look more into it, but I think removing the need for
Nokogiri's monkey patch by a guarded patch capability in mini_portile may
make sense. If git apply is available, use it. If not and patch is
available, use it. If neither, give an error message.

I'm also initially positive on us adding msys2 patch.exe and
patch.exe.manifest to our updated DevKit's as in this RubyInstaller pull
request oneclick/rubyinstaller#222.

Thoughts?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/32#issuecomment-43102096
.

[jonforums]

@flavorjones Nokogiri's 1.6.2.1 monkey patch to mini_portile affects Windows users who want to build nokogiri from source rather than use the (awesome, thank you for this continued support!) fat binary.

Source 1.6.2 built fine with my mingw-w64 4.9.0 DevKit but 1.6.2.1 appears to fail due to the current monkey patch hardcoding use of patch. Are you open for temporarily tweaking the current monkey patch for the guarded fallback I mention?

[jonforums]

Manually adding msys 2.6.1 patch to my DevKit failed to allow me to build Nokogiri 1.6.2.1 from source. This should have worked. Looks like more problems to spelunk (someone mentioned quoting on another issue).

http://paste.ubuntu.com/7464637/

libiconv's configure.log contains this single line:

configure: error: invalid variable name: `CPPFLAGS\'

[knu]

Oh, I didn't expect the built process through mini_portile might be run on a non-Unix shell.
shellescape/shelljoin may need to be monkey-patched in case of Windows.

[jonforums]

@knu maybe on shellescape/shelljoin, but it might be something else. Shelljoin is also used in 1.6.2 that I could build from source.

It may be that there is something in this nokogiri 1.6.2 and 1.6.2.1 diff of extconf.rb that explains why I can build 1.6.2 but not 1.6.2.1. I've not had time to review.

Nothing in the mini_portile 0.5.3 and 0.6.0 diff seems to be a problem.

[knu]

@jonforums Look at the line 208. Try changing "#{key}=#{value}".shellescape to something like %Q{"#{key}=#{value}"}.

[jonforums]

@knu with mingw.org patch.exe and patch.exe.manifest added to my 4.9.0 devkit and your line 208 tweak I get this success

http://paste.ubuntu.com/7468330/

I'm not familiar with the nokogiri build tooling and get these failures when trying to create a source gem:

C:\Users\Jon\Downloads\scratch>rake -T
rake aborted!
Errno::ENOENT: No such file or directory @ rb_sysopen - .cross_rubies
C:/Users/Jon/Downloads/scratch/Rakefile:93:in `read'
C:/Users/Jon/Downloads/scratch/Rakefile:93:in `<top (required)>'
(See full trace by running task with --trace)

C:\Users\Jon\Downloads\scratch>rake gem
rake aborted!
Errno::ENOENT: No such file or directory @ rb_sysopen - .cross_rubies
C:/Users/Jon/Downloads/scratch/Rakefile:93:in `read'
C:/Users/Jon/Downloads/scratch/Rakefile:93:in `<top (required)>'
(See full trace by running task with --trace)

Looks like things were meant to be built from Linux, but I don't have time to try to build a gem on my Arch box. If you'll show me a shortcut on how to build the nokogiri source gem on windows, I'll double check that it installs OK. It should given the ruby extconf.rb success from above worked.

I'm still advocating that you update the mini_portile monkey patch to implement guarded fallback: If git apply is available, use it. If not and patch is available, use it. If neither, fail the nokogiri source gem build with an error message.

This gives windows users who build nokogiri from source multiple options. It also means that we (rubyinstaller committers) do not have consider releasing a new devkit version for just patch.exe and unnecessarily thrashing users with a devkit upgrade.

[flavorjones]

For what it's worth, 0.7.0.rc1 will contain patches from #54 which will try to use git apply, then try to use patch, and then fail with a nice error message if neither is installed.