(PUP-927) Read preserving line endings

[ferventcoder]

Previously, puppet's built in file and template functions would
read file contents using File.read. This would not cause problems
on *nix but could cause EOL conversion from \r\n to \n on Windows.

Add Puppet::FileSystem.read_preserve_line_endings for cases where line
endings are important and use File.readlines method, which is known to
preserve original line endings (from reading the source, read has defined
calls for Windows where readlines does not).

References:

• IO.Read - https://github.com/ruby/ruby/blob/v2_1_5/io.c#L9638
• IO.Readlines - https://github.com/ruby/ruby/blob/v2_1_5/io.c#L3324
• IO.Read Windows - https://github.com/ruby/ruby/blob/v2_1_5/io.c#L2819-L2828
• https://www.ruby-forum.com/topic/132198

Add Puppet::FileSystem.read_preserve_line_endings for cases where line
endings are important and use :mode => 'rb' checking first against utf-8
encoding, falling back to default_external when not valid, and falling back
to File.read when all else fails.

Supersedes #2397

[puppetcla]

CLA signed by all contributors.

[ferventcoder]

Strangely enough in Ruby 1.9.3 and Ruby 2.1.5 I couldn't get File.read to return incorrect line endings. So this may not quite be sufficient (since I was unable to get a failure case in specs prior to changing the behavior).

In that case prefer the read_preserve_line_endings method to do something similar to https://tickets.puppetlabs.com/browse/PUP-927?focusedCommentId=130424&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-130424 with checking valid_encoding? - see https://github.com/ferventcoder/puppet/blob/99abd2c78ba3b697521c9bc4911e6fd8266d914c/lib/puppet/file_system/windows.rb#L51-L53

[ferventcoder]

So I have a repro proving File.readlines is not sufficient. Will look at switching to binary.

[joshcooper]

I think the mode needs to be 'wb', otherwise ruby will strip '\r' while it's writing the test file that it tries to read back in later.

[ferventcoder]

Done.

[ferventcoder]

The question I have is should we just switch Puppet::FileSystem.read to use this method and not have a separate method? I wasn't trying to be too intrusive and wanted to catch the specific cases where this was occurring. Or maybe that should just wait until PUP-2564?

[ferventcoder]

This is ready to go again.

[Iristyle]

Test repo from @ferventcoder at https://github.com/ferventcoder/puppet-simple_templates (note it's missing .gitattributes so you're subject to your global Git settings)

[ferventcoder]

@Iristyle I take pull requests! 👍 I updated this.

[Iristyle]

@ferventcoder let's retarget this at the new 3.8 branch. Thanks!

[ferventcoder]

Closing, will be retargeting soon.