Salt Text Handling of different Line Endings

[damon-atkins]

Description of Issue/Question

I will raise this as a separate issue as its bigger than this PR.
There are multiple competing Line Ending Formats:

• LF+CR
• LF
• CR
• CR+LF

It's a bad assumptions on Python part that only one type will be on the platform.
Application A on windows may use LF+CR and application B may use LF. Or on Unix you may use salt to generate a file on Unix, which is provide to windows clients using Samba.
Text handling needs to hand all cases on a platform.

Here is an example
C:\salt>salt-call --version
salt-call 2016.3.3 (Boron)

I think this will take a brain trust to decide on a way forward.

[lorengordon]

Related issues:

• file.replace turns CRLF line endings to CRCRLF #12284 (should be closed though)
• file.blockreplace uses wrong line endings on Windows #14002
• Complete and flexible approach to line endings (LF, CRLF, ... newlines) in file.* functions #14135
• Append/prepend in file.replace assumes '\n' line endings #27368

[Ch3LL]

I want to get a couple more people on this issue to determine how we could move forward these issues because obviously as @lorengordon points out there are multiple issues were this has caused issues.

ping @twangboy @terminalmage @cachedout just wanted to get any of your inputs on whether you think there is an easy way for us to handle these situations within salt which we could track here. thanks

[damon-atkins]

I see the option as

• Editing in place, read the first line (or 1MB or use mmap??) of a text file to determine the line feed before performing any action. eg. regex (.[\r\n].) If file does not exist use **kargs newline='\r\n' or if newline=None use the platforms default newline
• Use the same line feed as the original file which may come from http:// salt:// etc.

[damon-atkins]

From def prepend(path, _args, *_kwargs):

    try:
        with salt.utils.fopen(path) as fhr:
            contents = fhr.readlines()
    except IOError:
        contents = []
    preface = []
    for line in args:
        preface.append('{0}\n'.format(line))
    with salt.utils.fopen(path, "w") as ofile:
        contents = preface + contents
        ofile.write(''.join(contents))
    return 'Prepended {0} lines to "{1}"'.format(len(args), path)

I am adding this here as I suspect this is a general comment as well on handling text files.
prepend() assumes file is small by reading it into memory. It also does not have a back out for disk full e.g. rename original, read original 1MB (or use mmap??) at a time, write out new file. And if back-out needed, rename backup file to original file. Other checks are free disk space to perform the operation. I have also seen Configuration Management Tools backup a file every-time its changes to a backup location.

[lorengordon]

Couple notes:

• io.open() also has some of this platform-specific newline logic built-in.
• I like the idea of reading the first line to determine the line ending, if the file already exists. This can be a utility function.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[damon-atkins]

This is still outstanding

[stale]

Thank you for updating this issue. It is no longer marked as stale.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[stale]

Thank you for updating this issue. It is no longer marked as stale.

[waynew]

I think there's an even higher (lower?) level perspective here that we need to address, and my answer to

just wanted to get any of your inputs on whether you think there is an easy way for us to handle these situations within salt which we could track here.

is... no.

---

Historically, (python2), bytes and text was conflated. That meant that you could read fnord.jpg and fnord.txt into the same str in Python. For Salt, this was pretty great, because it meant that we could treat text and binary files the same, and usually get the expected behavior.

With Python3, however, text is unicode and bytes are bytes. If you want to treat something as text, you have to explicitly tell Python that it's text. Or that bytes should be bytes.

Right now Salt mostly assumes that everything is text/UTF-8, rather than requiring that we're told what kind of thing it is. And in many cases, we don't even allow being told that something isn't text. We just treat it all as text.

This fundamental shift in Python2/3 text/bytes separation is great for ensuring that we really have the representation that we expect. But it's also caused a few issues within Salt - and this is one of them. Python, if a file is opened in text mode, will translate '\n' to the appropriate OS line endings. If it's opened in binary mode then whatever bytes we feed the file will be written there - such as LF+CR, CR+LF, LF, CR, or any other weird combination.

The unfortunate part of this problem is that this probably requires a pretty fundamental shift in how Salt thinks about data, and will have an impact throughout the Salt codebase.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[lorengordon]

wow, 30 days? that's a really tight activity window for issues! c'mon @stalebot

[stale]

Thank you for updating this issue. It is no longer marked as stale.

[twangboy]

@damon-atkins Is this still an issue on 3006.2?

[twangboy]

Closing this issue due to age and lack of activity. Please test this on version 3006.2 and create a new issue if the problem persists. The new issue template has more information and will allow us to track and reproduce the issue more effectively. Thanks!

[damon-atkins]

@twangboy I do not believe any code changes have happen to resolve this. Salt does not read a file to find out the line ending it users, or allow the user to indicate which line ending to force use.