Some incorrect usage related to the recent `shell=False` issue

[jim60105]

[!IMPORTANT] In short, shell=False with a command string combining arguments does not work on Linux.

When shell=False, we should use a sequence instead of a string. This is because it behaves differently on different platforms, and using a sequence should work on both Windows and Linux. https://stackoverflow.com/a/30830867/8706033

For a complete explanation, please refer to the python documentation: https://docs.python.org/3/library/subprocess.html#popen-constructor

I have extracted the key points as follows:

args should be a sequence of program arguments or else a single string or path-like object. By default, the program to execute is the first item in args if args is a sequence. If args is a string, the interpretation is platform-dependent and described below. See the shell and executable arguments for additional differences from the default behavior. Unless otherwise stated, it is recommended to pass args as a sequence.

On POSIX, if args is a string, the string is interpreted as the name or path of the program to execute. However, this can only be done if not passing arguments to the program.

On Windows, if args is a sequence, it will be converted to a string in a manner described in Converting an argument sequence to a string on Windows. This is because the underlying CreateProcess() operates on strings.

So this will mess things up:

https://github.com/bmaltais/kohya_ss/blob/80091ee701462e0a341ffbb693b9ee81f628d5fd/kohya_gui/merge_lycoris_gui.py#L67-L72

[bmaltais]

Yeah... I need to go back to those that still use the string format and convert them back to use the run_cmd dictionary of arguments...

[bmaltais]

I have merged the change to the dev branch. I hope this will run fine on all platforms... including runpod, vastai and others like colab.

[jim60105]

I have merged the change to the dev branch. I hope this will run fine on all platforms... including runpod, vastai and others like colab.

I can confirm it works on WSL2 docker for LoRA trainning and Merge LyCORIS.

~~Note that I added the --do_not_use_shell because I am unsure if it will be adopted or what the current default value is.~~ I take a look at the code and understood the changes, the arguments part still needs to be removed. Thanks for your work.👍