saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Install Salt from the Salt package repositories here:
https://docs.saltproject.io/salt/install-guide/en/latest/
Apache License 2.0
14.23k stars 5.49k forks source link

modules win_path, win_snmp, win_system, win_useradd and Unicode #36011

Closed damon-atkins closed 7 years ago

damon-atkins commented 8 years ago

Description of Issue/Question

The following need to have from __future__ import unicode_literals Add to the code and re-tested.

modules/win_path.py modules/win_snmp.py modules/win_system.py modules/win_useradd.py

modules/reg.py now supports Unicode however it does contain support for backwards compatibility. The above use modules/reg.py

The following change would help in moving to Python 3 on windows.

Ch3LL commented 8 years ago

ping @twangboy fyi looks like we need to get this added to convert to python3.

@damon-atkins thanks for the heads up

damon-atkins commented 8 years ago

django porting to PY3 standard https://docs.djangoproject.com/en/1.10/topics/python3/

damon-atkins commented 7 years ago

See also #40637 salt/modules/win_wua.py

twangboy commented 7 years ago

@cachedout Is this something we need to pursue?

damon-atkins commented 7 years ago

One of the reason for the change to reg.py because software package names are not always English or might have the single characters ™ © . This was causing win_pkg issues.

If you want to be able to add Italian User Name Description to a system, which has a default encoding of English then this is required. reg.py accepts unicode and non-unicode (converts it into unicode system default) and always outputs unicode/binary. You could take the few encoding functions in reg.py and move them into utils/win_functions.py to use in other modules.

Before change happens some research needs to happen on the windows api/command used to ensure it accepts Unicode i.e. not written as badly as PY2 Registry interface. (The internal PY2 registry interface behind the scenes converts Unicode back to String , and then back to Unicode before calling the Windows API. PY3 registry interface is all Unicode no conversion. Looking forward to a beta PY3 Windows release to fix language issues)

cachedout commented 7 years ago

@twangboy I assume that you want to support Unicode characters for these modules, in which case, yes this will need to be done.

twangboy commented 7 years ago

@cachedout Nitrogen?

cachedout commented 7 years ago

@twangboy Yes.

twangboy commented 7 years ago

Setup

Add a path with unicode characters to the %PATH%. Used ρ: c:\python27\scripts;c:\program files\python35\scripts;c:\program files\python35;c:\windows\system32;c:\windows;c:\windows\system32\wbem;c:\windows\system32\windowspowershell\v1.0;c:\program files\amazon\cfn-bootstrap;c:\program files\git\cmd;c:\program files (x86)\windows kits\8.1\windows performance toolkit;c:\unicode\testρ

First of all, the win_path module seems to change all the paths to lower case. Doesn't effect it, but it shouldn't mess with existing paths.

Alright, so what I'm seeing on this so far is that from a Linux Master and a Windows Minion running on Py3 with Unicode character, the win_path module works correctly. The last item returned with win_path.get_path is: c:\unicode\testρ as it should be.

Linux master on Py2 doesn't work. You get something like this: :\unicode\test? (yes it's missing the c for some reason and the Unicode ρ is translated to ?. This is pretty much the problem I've had with Unicode on Py2 on Windows.

So, it seems that Py3 fixes this issue. However, when using salt-call in a command shell on Windows on Py3, it still doesn't work. The Windows console uses the CP437 codepage while python uses UTF-8 and fails to display the Unicode correctly. The issue comes when you try to do a salt-call --local win_path.add c:\unicode\testρ, again the translation fails and instead of c:\unicode\testρ, you get c:\unicode\test? in the path.

Not sure how to fix this...

damon-atkins commented 7 years ago

Try yaml_utf8 is True. Given Salt trying support PY3 and its Unicode. Maybe this should default to true. I.e. start displaying a warning when this is false. (if this fixes part of the issue)

Have a read of https://superuser.com/questions/269818/change-default-code-page-of-windows-console-to-utf-8 and http://stackoverflow.com/questions/14109024/how-to-make-unicode-charset-in-cmd-exe-by-default

Maybe add to the salt-call.bat and optionally set it back to the default.

@echo off
CHCP 65001
.....
python
set py_error=%ERRORLEVEL%
CHCP 850
exit /B %py_error%
twangboy commented 7 years ago

So, changing the codepage in the console where you're running salt-call seems to work pretty good.

damon-atkins commented 7 years ago

If I start cmd.exe

C:\> chcp
Active code page: 850

https://en.wikipedia.org/wiki/Code_page_850 https://en.wikipedia.org/wiki/Code_page_437 https://ss64.com/nt/chcp.html

And you have suggest its 437 in an early post. I suggest you change the cp to 65001 and not change it back as it looks like it will be different per country.

twangboy commented 7 years ago

@damon-atkins This requires more testing, obviously, but I was thinking more along the lines of documenting that those who need to use Unicode characters in commands issued via the CLI should change the codepage to 65001. It seems like permanent codepage changes cause Windows boot failure in some cases. I need to test if we need to add a chcp command to the .bat files as suggested above. Mine seemed to work fine without it... Again, needs more testing.

twangboy commented 7 years ago

@damon-atkins Adding the chcp to the batch files removes the need to change the codepage

damon-atkins commented 7 years ago

Hi @morganwillcock Do you run windows as non-English ? Can you do some testing?

morganwillcock commented 7 years ago

@damon-atkins Sorry, I only use The Queen's English. I think I've got packages installed with Unicode characters in their names, if that is of any use to you...

damon-atkins commented 7 years ago

Thanks @morganwillcock for the response.. For these changes need something other English for lots of stuff on Windows not just Software Packages.

Ch3LL commented 7 years ago

@twangboy is this okay to close due to those PRs or is there remaining work?

twangboy commented 7 years ago

@Ch3LL Some of the PR's are marked Pending Discussion... So, I don't think this should be closed until we decide what we're going to do on those.

Ch3LL commented 7 years ago

K thanks for the status update