Msys2-packages: Issues with `msys2_shell.cmd` after today upgrade

[angelog0]

I see that now msys2_shell.cmd has line endings in DOS convection (CRLF).

I converted it in UNIX style (LF) with

d2u msys2_shell.cmd

and it has issues to start MSYS2.

Running from cmd,

C:\msys64\msys2_shell.cmd

starts MSYS2 but at DOS prompt I find

,*" non atteso.

(,* not expected).

If, instead, I start

C:\msys64\msys2_shell.cmd -msys2 -defterm -no-start

MSYS2 DOES NOT start and it prints at DOS PROMPT:

Impossibile trovare l'etichetta batch specificata - checkparams

(Impossible to find the specified label batch - checkparams).

The previous version of msys2_shell.cmd worked just fine in UNIX style.

[angelog0]

With current version, I cannot use any more this:

sed -i.orig -e 's/rem set MSYS=winsym/set MSYS=winsym/' msys2_shell.cmd

I have to do that manually. This is a regression.

[angelog0]

This message

,*" non atteso.

occurs also with the original msys2_shell.cmd not unix converted, i.e. in DOS style (CRLF)!

[StarWolf3000]

Undo that change regarding CRLF in Batch/NT scripts (.cmd): Labels don't work correctly when using UNIX line-endings (LF) and there's also no need to use it.

[API-Beast]

It seems like commit 11ba641 broke this. It also broke passing directories with spaces in them using the "-where" parameter.

[angelog0]

@StarWolf3000 wrote:

Undo that change regarding CRLF in Batch/NT scripts (.cmd): Labels don't work correctly when using UNIX line-endings (LF) and there's also no need to use it.

But also with CRLF, i.e. the new original .cmd, it prints this

,*" non atteso.

You have to start it from DOS prompt to see the above. And, as I wrote, the previous version worked also in UNIX style line endings (LF).

I think there is a subtle bug here...

[xzn]

It seems like commit 11ba641 broke this. It also broke passing directories with spaces in them using the "-where" parameter.

Any argument that needs to be quoted causes the script to break

I don't know enough about batch scripting to know why though.

[lazka]

@tanzislam ^

[tanzislam]

I'm certainly not knowledgeable about all use cases, but I'd imagine that being a Windows file format, the interpreter (cmd.exe) will prefer/expect it to be in CRLF line endings.

Out of interest, why are we applying d2u on it? Is this part of developer workflow or the build/packaging/publishing system?

BTW feel free to revert my change in .gitattributes if that fixes your problem. It's not really necessary for the command-line parsing bug I was trying to fix.

Also, the Wiki does warn against using pathnames with spaces in them ...

[xzn]

The wiki only mentions not installing msys2 in a path with space in name though. msys2_shell.cmd could be used start a msys2 shell in arbitrary working directory and passing additional arguments to the bash instance. Currently any argument having space in them no longer works.

According to msdn doc for /f tokens construct is meant for file parsing, it seems to ignore double quotes and so on in literal string parsing though

[angelog0]

Hi @tanzislam, you have to re-read the full thread.

The issue is not the usage of d2u but your original patches and the original msys2_shell.cmd released few days ago with all the CRLF.

If you prefer, even without applying d2u there are issues as @xzn described and as I described here.

[tanzislam]

OK so according to SS64, the delimiters used for parameter-splitting by the shell (before starting the script) include =, but the FOR /F ... IN command has a smaller default list of delimiters (and also doesn't recognise quoted arguments with spaces in them as one token). I'll have to increment the shiftCounter more carefully in light of this.

[xzn]

I looked around a bit in your link. I would recommend revert change to msys2_shell.cmd in 11ba641

And for your use case pass "--gtest_color=yes". Since it's probably too complicated otherwise.

(If the goal is to pass parameters to bash verbatim there's quite a few things to take care of, count the number of spaces in each argument and add to the number of tokens to skip, count other non-space delimiters and subtract from the number of tokens to skip, Parse " and deal with "" escape and so on)

[angelog0]

@tanzislam, I have seen your commits, so tried this msys2_shell.cmd which is in UNIX line endings format and it seems to work: I use this

C:/msys64/msys2_shell.cmd -msys2 -defterm -no-start

to start MSYS2 in Windows Terminal Preview.

Anyway, just running this

C:\msys64\msys2_shell.cmd

from cmd, starts MSYS2 but at CMD prompt prints:

,* delims=,;= " non atteso.

(_non atteso_ == unexpected)

please, test msys2_shell.cmd at CMD prompt and see if you can remove that...

BTW, instead of uncommenting the line

rem set MSYS=winsymlinks:nativestrict

can we add another option to have native symlinks? Thanks.

[tanzislam]

@angelog0 Does it still fail for you without using UNIX line endings? (i.e. using it exactly as Git checks it out, without applying d2u)

Also, there is a TAB character in the source file line in that error message, which is essential. If you're copying it out from your browser that TAB character may have been replaced with a space.

[xzn]

The commit solved my use case as well.

Apparently tokens for for /f doesn't like index of 0, somehow everything after is part of the error message?

Edit:
There's one question:
How do I pass parentheses to bash from msys2_shell.cmd with the current change? they all get subsiituted with x. Say if I want to run a script stored under some directory with parentheses in its name, should I just avoid that for now?

[angelog0]

@tanzislam, really I didn't have changed the line endings. I downloaded it with (see the link in my previous post):

wget https://raw.githubusercontent.com/msys2/MSYS2-packages/528cd2acf8e7216e27b53ad8d93c7dbe39fd11fd/filesystem/msys2_shell.cmd

and it already was in UNIX line endings! Anyway, to test your request, I applied u2d converting it DOS line endings but the same string shows up at CMD prompt... I think there is something in the code it don't like.

[xzn]

Attempt at updating msys2_shell.cmd,

just wondering would quoting the arguments in double quotes and skipping replacing all the parentheses suffice?

[tanzislam]

I've pushed two more commits to my PR to address the corner-case bugs that @angelog0 and @xzn found.

@angelog0, regarding:

BTW, instead of uncommenting the line

rem set MSYS=winsymlinks:nativestrict

can we add another option to have native symlinks? Thanks.

I'd recommend (for now) exporting MSYS=winsymlinks:nativestrict yourself before invoking msys2_shell.cmd, instead of editing it with sed (I'm guessing this is why you were applying d2u, for sed to be able to work on it). You can accomplish this inline, like:

set MSYS=winsymlinks:nativestrict & C:\msys64\msys2_shell.cmd -defterm -no-start ...

Or raise another PR for that feature. It should be easier than this one. :slightly_smiling_face:

@xzn, regarding:

How do I pass parentheses to bash from msys2_shell.cmd with the current change? they all get subsiituted with x. Say if I want to run a script stored under some directory with parentheses in its name, should I just avoid that for now?

I don't think parentheses (and other cmd.exe metacharacters) worked before anyway, as they would interfere with the script lines where %SHELL_ARGS% was placed in the Bash command line. I now changed them to !SHELL_ARGS! (delayed expansion) to make it work.

[angelog0]

@tanzislam, I do not understand why you say that what I flagged is a consequence of using d2u... I have already flagged that the msys2_shell.cmd from your commits IS in UNIX line endings.. IS not the result of d2u!

Also your last commit is in that way when downloaded with wget:

wget https://raw.githubusercontent.com/msys2/MSYS2-packages/d02e5adf7d6f8a277940efac26b0f60d0e007ea4/filesystem/msys2
_shell.cmd

and this time it WORKS in CMD prompt without issues. So the thesis that it MUST BE in DOP line endings (CRLF) IS FALSE.

Summarizing:

• the last msys2_shell.cmd is in UNIX line endings format (LF);

  • running it from CMD prompt:

    C:\msys64\msys2_shell.cmd

  DOES NOT PRODUCE any garbage string.

The same if I uncomment the line regarding the native symlinks.

WHY one should start the MSYS2 .cmd as you suggest? Even if it were in CRLF, it would be simplest uncommenting that line. I use the .cmd to start MSYS2 in Windows Terminal:

"commandline" : "C:/msys64/msys2_shell.cmd -msys2 -defterm -no-start"

and I decide to use it to avoid this:

"commandline": "cmd.exe /c \"set MSYSTEM=MSYS&& set MSYS=winsymlinks:nativestrict&& C:/msys64/usr/bin/bash.exe --login\""

[tanzislam]

@angelog0,

I do not understand why you say that what I flagged is a consequence of using d2u... I have already flagged that the msys2_shell.cmd from your commits IS in UNIX line endings.. IS not the result of d2u!

It was my initial suspicion, but no longer. The line endings don't matter here; cmd.exe works fine either way. (I'm still puzzled as to why you needed to ensure LF line endings at all, but it doesn't matter now.)

One unrelated point, though ... the msys2_shell.cmd file that I'm editing on my Windows 7 laptop (in my clone of the Git repo using Git for Windows) is in CRLF line endings. Git normalizes the file to LF line endings when committing, so the repo contains LF line endings. When you're using wget, you're getting the un-converted version of the file (because wget doesn't know to respect/apply the settings in .gitattributes). If you were to git clone the repo instead, Git would CRLF-convert the file for you.

Regarding:

WHY one should start the MSYS2 .cmd as you suggest? Even if it were in CRLF, it would be simplest uncommenting that line.

I'm not fundamentally opposed to doing this. However:

• It is an unrelated feature / behavior change. This Issue and PR are merely about fixing up the bugs I introduced.

• If this were done from the beginning of the MSYS2 project, it would have been much simpler. Now there are existing applications installed (without native symlinks mode) in users' PCs, and any impact of enabling this mode by default should be carefully assessed by the MSYS2 maintainers. I'm not on the MSYS2 team, and have limited time for that sort of work.

I'm actually using a very similar setting in Windows Terminal (in a Windows 10 VM on a MacBook):

"commandline": "cmd.exe /C set MSYS=winsymlinks:nativestrict & C:\\msys64\\msys2_shell.cmd -defterm -no-start -mingw64 -full-path"

@xzn, I just realized that I misidentified you as "xvn" in some of my earlier postings. Fixed them now. Sorry about that!