Salt: file.line on windows not treating unix line endings correctly.
Description of Issue/Question
On windows when using file.line state module. If the file being changed has unix style line endings instead of dos line endings. The entire file can be changed.
Setup
A simple file to change
test2
test3
a simple state.
remove_test2:
file.line:
- name: 'c:\test\test.txt'
- mode: 'delete'
- match: 'test2'
Steps to Reproduce Issue
(Include debug logs if possible and relevant.)
if the file c:testtest.txt uses dos line endings it only removes test2 from the file.
however if the c:testtest.txt files is using Unix line endings the entire file is reduced to a blank file.
Unix line ending version
winion-0:
----------
ID: remove_conf_line
Function: file.line
Name: C:/test/test.txt
Result: True
Comment: Changes were made
Started: 20:02:27.416000
Duration: 15.0 ms
Changes:
----------
diff:
---
+++
@@ -1,3 +0,0 @@
-test1
-test2
-test3
Summary for winion-0
------------
Succeeded: 1 (changed=1)
Failed: 0
------------
Total states run: 1
Total run time: 15.000 ms
Dos line endings in c:testtest.txt
winion-0:
----------
ID: remove_conf_line
Function: file.line
Name: C:/test/test.txt
Result: True
Comment: Changes were made
Started: 20:04:08.523000
Duration: 21.0 ms
Changes:
----------
diff:
---
+++
@@ -1,3 +1,2 @@
test1
-test2
test3
Summary for winion-0
------------
Succeeded: 1 (changed=1)
Failed: 0
------------
Total states run: 1
Total run time: 21.000 ms
This only affects windows minions. Linux minions can handle the file with either line ending.
Versions Report
Salt Version:
Salt: 2018.3.0
Dependency Versions:
cffi: 1.10.0
cherrypy: 10.2.1
dateutil: 2.6.1
docker-py: Not Installed
gitdb: 2.0.3
gitpython: Not Installed
ioflo: Not Installed
Jinja2: 2.10
libgit2: Not Installed
libnacl: 1.6.1
M2Crypto: Not Installed
Mako: 1.0.7
msgpack-pure: Not Installed
msgpack-python: 0.4.8
mysql-python: Not Installed
pycparser: 2.18
pycrypto: 2.6.1
pycryptodome: Not Installed
pygit2: Not Installed
Python: 2.7.14 (v2.7.14:84471935ed, Sep 16 2017, 20:25:58) [MSC v.1500 64 bit (AMD64)]
python-gnupg: 0.4.1
PyYAML: 3.12
PyZMQ: 16.0.3
RAET: Not Installed
smmap: 2.0.3
timelib: 0.2.4
Tornado: 4.5.2
ZMQ: 4.1.6
System Versions:
dist:
locale: cp1252
machine: AMD64
release: 2016Server
system: Windows
version: 2016Server 10.0.14393 Multiprocessor Free
whytewolf
All 12 comments
ZD-2564
whytewolf
on 13 Jun 2018
@whytewolf is this a regression?
also ping @saltstack/team-windows any ideas here
Ch3LL
on 14 Jun 2018
from https://docs.python.org/3/tutorial/inputoutput.html
text mode, the default when reading is to convert platform-specific line endings (n on Unix, rn on Windows) to just n.
I would assume about would add platform-specific line endings
def _get_eol(line):
match = re.search('((?<!\r)\n|\r(?!\n)|\r\n)$', line)
return match and match.group() or ''
_get_eol will never see the line endings unless the file is open in binary mode. From what I can tell its open in text mode on Unix and Windows PY2 Binary, Windows PY3 Text.
damon-atkins
on 14 Jun 2018
I have made a few comments about Text Process in a few PR/Issues. Can we start a Discuss PR about what the requirements are and the assumptions. For example py3 introduced a newline option, which is also in PY2 io.open
newline controls how universal newlines works (it only applies to text mode). It can be None, '', 'n', 'r', and 'rn'. It works as follows:
https://docs.python.org/release/3.2/library/functions.html#open
On input, if newline is None, universal newlines mode is enabled. Lines in the input can end in 'n', 'r', or 'rn', and these are translated into 'n' before being returned to the caller. If it is '', universal newline mode is enabled, but line endings are returned to the caller untranslated. If it has any of the other legal values, input lines are only terminated by the given string, and the line ending is returned to the caller untranslated.
On output, if newline is None, any 'n' characters written are translated to the system default line separator, os.linesep. If newline is '', no translation takes place. If newline is any of the other legal values, any 'n' characters written are translated to the given string.
Also atomic_open is used which calls
ntf = tempfile.NamedTemporaryFile
Which convert n if open in text mode to platform default line ending. So original file \n out \r\n. And also does not support the newline option in io.open which is available in PY2 and PY3.
https://docs.python.org/2/library/io.html?highlight=newline%20controls#io.open
damon-atkins
on 14 Jun 2018
ping @terminalmage do you have any ideas around the proper way to handle this?
Ch3LL
on 21 Jun 2018
See also https://github.com/saltstack/salt/pull/46291#issuecomment-398771224
damon-atkins
on 21 Jun 2018
After much testing, I have not been able to get this to work on any version of salt under windows. As @damon-atkins says this could be a python issue at the core. Is there any way within python standards to handle this on windows as it is handled in Linux?
whytewolf
on 22 Jun 2018
@whytewolf Does the fix in #48380 fix this for you?
rallytime
on 2 Jul 2018
Sorry having trouble testing this fix. Apparently, I can't just drop it into salt://_modules since an import was changed. And I'm having trouble getting windows to recognize the updated copy in the directory.
Will try again. just thought I would note to let you know this is on my radar.
whytewolf
on 3 Jul 2018
Looks like we might be trying a different fix anyway. This was fixed in develop recently, and the fix is different from the one posted above.
rallytime
on 6 Jul 2018
Ok, there was a larger fix in develop that we are backporting to 2018.3. That is the fix to test @whytewolf, which is PR #48503. Thank you!
rallytime
on 10 Jul 2018
That patch looks to fix this issue.
whytewolf
on 12 Jul 2018
Related issues
lhost
路
3Comments
sfozz
路
3Comments
layer3switch
路
3Comments
mooperd
路
3Comments
qiushics
路
3Comments