Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Packaged echo.exe removing braces from cmdline arguments #561

Closed
asottile opened this issue Dec 7, 2015 · 16 comments
Closed

Packaged echo.exe removing braces from cmdline arguments #561

asottile opened this issue Dec 7, 2015 · 16 comments
Labels

Comments

@asottile
Copy link

asottile commented Dec 7, 2015

Version info:

C:\Users\Anthony>git --version
git version 2.6.3.windows.1

C:\Users\Anthony> C:\Users\Anthony\AppData\Local\Programs\Git\usr\bin\echo.exe hi{1}
hi1

Expected:

hi{1}

Interestingly enough, this works:

# With a space between the text and the braces
C:\Users\Anthony> C:\Users\Anthony\AppData\Local\Programs\Git\usr\bin\echo.exe 'hi {1}'
hi {1}
@Radrik5
Copy link

Radrik5 commented Dec 7, 2015

It's not because of space but because of quotes:

e:\Projects> "C:\Program Files\Git\usr\bin\echo.exe" 'x{1}'
x{1}

e:\Projects> "C:\Program Files\Git\usr\bin\echo.exe" "x{1}"
x{1}

Not reproducible in Git Bash:

~ $ /c/Program\ Files/Git/usr/bin/echo.exe x{1}
x{1}

@dscho
Copy link
Member

dscho commented Dec 7, 2015

I agree with @Radrik5 that you need to quote arguments you do not want to be interpolated in any way.

@dscho dscho closed this as completed Dec 7, 2015
@dscho dscho added the wontfix label Dec 7, 2015
@asottile
Copy link
Author

asottile commented Dec 7, 2015

I assure you this is not a situation where interpolation should be happening. But first let me convince you of it:

First, expected behaviour from ubuntu:

$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 15.04
Release:    15.04
Codename:   vivid
$ echo hi{1}
hi{1}

Using cmd.exe's builtin:

C:\Users\Anthony>echo hi{1}
hi{1}

Avoiding the shell with python subprocess:

C:\Users\Anthony>python -c "import subprocess; print(subprocess.check_output((r'C:\Users\Anthony\AppData\Local\Programs\Git\usr\bin\echo.exe', 'hi{1}'), shell=False))"
hi1

EDIT: and some other executable:

C:\Users\Anthony>cat test.bat
@echo off
echo %*

C:\Users\Anthony>test.bat hi{1}
hi{1}

@dscho
Copy link
Member

dscho commented Dec 7, 2015

I assure you this is not a situation where interpolation should be happening

I know that {1} should not be interpolated.

But since you do not want the argument to be interpolated, you should quote it. And since that works around the curly braces problem, I cannot afford to spend time fixing this issue ("wontfix").

Of course you should feel more than welcome to give it a shot yourself. The first thing to do, if you choose to accept this task, is to run the command through strace.exe to try to figure out where things go wrong. If you get stuck anywhere, just present what you found out so far and explain where you are stuck.

@asottile
Copy link
Author

asottile commented Dec 7, 2015

If the goal is to provide a conforming implementation of echo (which I believe it is), this is actually a bug.

I can't just go quoting strings, this'll lead to another class of bugs with over-quoting (I'm not even using a shell in the first place!):

C:\Users\IEUser>python
Python 2.7.10 (default, May 23 2015, 09:40:32) [MSC v.1500 32 bit (Intel)] on wi
n32
Type "help", "copyright", "credits" or "license" for more information.
>>> import subprocess
>>> print(subprocess.check_output((r'C:\Program Files\Git\usr\bin\echo.exe', '"hi{1}"')))
\hi{1}"

>>>

That said, looking at the strace output: http://paste.pound-python.org/show/Kv30PQaptsi0IFEstkfS/ it seems the problem is here (you can see it receives the correct arguments, then munges them):

  230   45754 [main] echo 3012 build_argv: cmd = '"C:\Program Files\Git\usr\bin\echo.exe" hi{1}', winshell = 1, glob = 1
  214   45968 [main] echo 3012 build_argv: argv[0] = 'C:\Program Files\Git\usr\bin\echo.exe'
  165   46133 [main] echo 3012 lstat64: entering
  124   46257 [main] echo 3012 normalize_posix_path: src hi1

@dscho
Copy link
Member

dscho commented Dec 7, 2015

Is it possible that there is actually a file hi1 in the current directory?

@dscho
Copy link
Member

dscho commented Dec 7, 2015

Never mind, I just verified that such a file does not need to exist for that behavior.

@dscho
Copy link
Member

dscho commented Dec 7, 2015

Try

> set MSYS=noglob

> "\Program Files\git\usr\bin\echo.exe" hi{1}
hi{1}

@bukzor
Copy link

bukzor commented Dec 7, 2015

@dscho Is that thing ($MSYS) documented somewhere? It certainly doesn't seem to be POSIX.

Also, where exactly is git-for-windows getting its coreutils from?
The version posted by MSYS is 5.97.3 but we see 8.24 in this environment.
http://sourceforge.net/projects/mingw/files/MSYS/Base/coreutils/

@bukzor
Copy link

bukzor commented Dec 7, 2015

@bukzor
Copy link

bukzor commented Dec 7, 2015

I can't find any reference to 'noglob' in that repo.

@bukzor
Copy link

bukzor commented Dec 7, 2015

There's a noglob defined for the $CYGWIN32 environment variable which seems to match this behavior: https://cygwin.com/cygwin-ug-net/using-cygwinenv.html

(no)glob[:ignorecase] - if set, command line arguments containing UNIX-style file wildcard characters
(brackets, braces, question mark, asterisk, escaped with \) are expanded into lists of files that match
those wildcards. This is applicable only to programs run from non-Cygwin programs such as a CMD
prompt. That means that this setting does not affect globbing operations for shells such as bash, sh, tcsh,
zsh, etc. Default is set.

The intent seems to be to have bourne-shell-like glob behavior when not using the bourne shell.

@dscho
Copy link
Member

dscho commented Dec 8, 2015

There's a noglob defined for the $CYGWIN32 environment variable

Correct. Since MSys2 is a (friendly) fork of Cygwin, the environment variable has been renamed (to avoid interfering with Cygwin when trying to mix and match MSys2 with Cygwin).

The intent seems to be to have bourne-shell-like glob behavior when not using the bourne shell.

Indeed. Unix users seem to expect globs to work when running from cmd.exe, I guess.

@Radrik5
Copy link

Radrik5 commented Dec 8, 2015

I think it shouldn't expand braces without comma: x{1}. Bash doesn't expand such expressions.

@bukzor
Copy link

bukzor commented Dec 8, 2015

The fact that it expands '"{1}"' to {1}" is entirely wrong, as well.

On Tue, Dec 8, 2015 at 12:38 PM Dmitry Oksenchuk notifications@github.com
wrote:

I think it shouldn't expand braces without comma: x{1}. Bash doesn't
expand such expressions.


Reply to this email directly or view it on GitHub
#561 (comment)
.

@dscho
Copy link
Member

dscho commented Dec 9, 2015

Hey, feel free to fix this and contribute the fix to the Cygwin project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants