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

Release 2.33.1 breaks opening visual code by git commit command #3471

Closed
hansschuijff opened this issue Oct 15, 2021 · 18 comments · Fixed by git-for-windows/build-extra#385
Closed

Comments

@hansschuijff
Copy link

After upgrading to version 2.33.1 I some git-commands didn't work any more on my win10 machine.
The reason seem to be the spaces in the path to code.cmd.

I tried reinstalling first, but the problem persisted and now I have reinstalled the previous version 2.33.0(2) and the problem is solved. I didn't know how to solve it otherwise and am stuck for now with the prev version.

I could still open vscode using the code command, but when git normally opens the editor (for instance in "git commit --amend" or even just "git commit") the response was an error about the path. I don't know if there are other commands too that open code and are broken, but the commit didn't work anymore after this update..

For instance this was the response to "git commit --amend" in git version 2.33.1.

C:\Users\Hans\Local Sites\sandbox\app\public\wp-content\plugins\genesis-connect-sensei-lms>git commit --amend
hint: Waiting for your editor to close the file... 'C:\Program' is not recognized as an internal or external command,
operable program or batch file.
error: There was a problem with the editor '"C:\Program Files\Microsoft VS Code\bin\code.cmd" --wait'.
Please supply the message using either -m or -F option.

Setup

  • 2.33.1 64-bit
  • vscode version 1.61.0
  • windows 10 64 bits
@dscho
Copy link
Member

dscho commented Oct 15, 2021

This seems to have been missed in git-for-windows/build-extra#384. @segevfiner, help?

@dscho
Copy link
Member

dscho commented Oct 15, 2021

The reason seem to be the spaces in the path to code.cmd.

@hansschuijff could you add more details here? I guess you installed VS Code system-wide? The report is a bit parsimonious with details.

@rimrul
Copy link
Member

rimrul commented Oct 15, 2021

error: There was a problem with the editor '"C:\Program Files\Microsoft VS Code\bin\code.cmd" --wait'.

This does look like the path is combined correctly and then quoted correctly, but that quoting gets messed up along the way.

@segevfiner
Copy link

Weird... I haven't touched the quotes in the command after all. I will try and repro.

@hansschuijff
Copy link
Author

@dscho yes, I installed all software system-wide. and have setup code to use bash as terminal too. Please ask if you need any other details. I'm not sure what you need.

@segevfiner
Copy link

P.S. As a workaround you can overwrite you can change your core.editor setting after installing Git, just note that it will be overwritten on Git for Windows upgrades AFAIK.

@segevfiner
Copy link

@hansschuijff Your .gitconfig will help, strip any private data in it, of course.

@hansschuijff
Copy link
Author

hansschuijff commented Oct 15, 2021

@segevfiner the content of my .gitconfig is as below (mainly aliasses I got from knowthecode). I just performed an upgrade without any setting changes, before this error occurred. Could be I also updated vscode yesterday, but I'm not sure if it was before or after. Reinstalling didn't help and reinstalling the prev version did, so I guess it doesn't matter.

[filter "lfs"]
	clean = git-lfs clean -- %f
	smudge = git-lfs smudge -- %f
	process = git-lfs filter-process
	required = true
[user]
	name = Hans Schuijff
	email = 
[color]
	ui = true
[core]
	excludesfile = ~/.gitignore_global
[winUpdater]
	recentlySeenVersion = 2.25.0.windows.1
[alias]
	# Opens the Atom editor.
	atom = ! atom

	# ===================================
	# Edit global config
	# ===================================
	
	edit     = config --global --edit;
	settings = config --global --edit;

	# ===================================
	# Viewing history
	# ===================================

	# Shows a graphical log.
	logone = log --oneline --decorate --all --graph

	# Shows the log for the last commit.
	loglast = show --summary

	# Less verbose status.
	st = status --short --branch

	# ===================================
	# Staging
	# ===================================

	stagea = "!f() { \
		clear; \
		git add -A; \
		git st; \
		echo 'Staging all the working changes....'; \
	}; f"

	unstagea = "!f() { \
		clear; \
		git reset HEAD --; \
		git st; \
		echo 'Unstaging all the changes by moving them back to the working directory....'; \
	}; f"

	unstage = "!f() { \
		clear; \
		git reset HEAD ${1-'--'}; \
		git st; \
		echo 'Unstaging change(s) by moving it(them) back into the working directory....'; \
	}; f"

	# ===================================
	# Commits
	# ===================================

	# Commits everything in the working and staging ares.
	# When a commit message is not provided, it pops the editor open.
	commita = "!f() { \
		git add -A; \
		commit_msg=$1; \
		if [[ $commit_msg != '' ]]; \
		then \
			git commit -m \"$commit_msg\"; \
		else \
			git commit; \
		fi; \
		clear; \
		git st; \
		git loglast; \
		echo 'Committed all changes....'; \
	}; f"

	# Uncommits the last commit, throwing away the commit and moving the changes
	# back into the working directory.
	uncommit = "!f() { \
		git reset HEAD^ -q; \
		clear; \
		git st; \
		echo 'Uncommitted the last commit, moving the changes back into your working directory....'; \
	}; f"

	# Pops open your text editor, where you can reword the last commit message.
	# Any WIP changes are stashed and then reapplied after you get done changing the message.
	reword = "!f() { \
		git stash; \
		git commit --amend; \
		git stash pop; \
		clear; \
		git loglast; \
		echo 'Fixed the commit message....'; \
	}; f"

	# Adds all of the new changes (WIP) in both your working and staging areas to the last commit.
	# It keeps the commit message.  Therefore, no commit message is needed.
	addto = "!f() { \
		git add -A; \
		git commit --amend --no-edit; \
		clear; \
		git loglast; \
		echo 'Added all working and staged changes (WIP) to the last commit....'; \
	}; f"

	# Rolls everything back to the last commit. CAUTION: this command deletes
	# all of the changes you've made.
	rollback = "!f() { \
		git unstage; \
		git checkout -- .; \
		git clean -fd; \
		clear; \
		git st; \
		echo 'Rolled back to the last commit....'; \
	}; f"

	# ===================================
	# Branches
	# ===================================

	# Creates a new branch and checks it out.
	newbranch = checkout -b
	newbr     = newbranch;

	# Deletes both the local and remote branch.
	deletebranch = "!f() { \
		name=$1; \
		if [[ $name == '' ]]; \
		then \
			echo 'Whoops, you forgot to give me the branch name.'; exit -1; \
		fi; \
		clear; \
		git branch -D $name; \
		echo \"Deleted the local ${name} branch.\"; \
		git branch; \
		git push origin --delete $name; \
		echo \"Deleted the remote ${name} branch.\"; \
	}; f"
	removebr = deletebranch;
	delbr    = deletebranch;

	# Renames the current branch.
	rename = "!f() { \
		new_name=$1; \
		if [[ $new_name == '' ]]; \
		then \
			echo 'Whoops, you forgot to give me the new branch name.'; exit -1; \
		fi; \
		clear; \
		git branch -m $new_name; \
		echo \"Renamed the current branch to ${new_name}.\"; \
		git branch; \
	}; f"
	renbr = rename;

	# Checkout a different branch.
	switch = checkout
	sw = checkout

	# show branches.
	br = "!f() { \
		clear; \
		git branch; \
	}; f"
[push]
	default = current

@dscho
Copy link
Member

dscho commented Oct 15, 2021

I haven't touched the quotes in the command after all.

No, but you have replaced a call to an .exe by a call to a .cmd. There is some weird stuff going on in either MSYS2 Bash or the MSYS2 runtime to execute .cmd because you cannot simply call CreateProcessW() with a .cmd script.

@dscho
Copy link
Member

dscho commented Oct 15, 2021

Your .gitconfig will help, strip any private data in it, of course.

That would apply if VS Code was installed user-wide. But it is installed system-wide, hence the core.editor configuration goes into C:\Program Files\Git\etc\gitconfig.

@dscho
Copy link
Member

dscho commented Oct 15, 2021

@hansschuijff could you start Git Bash as Administrator (e.g. hitting the Windows key, typing "Git Bash" and then pressing Ctrl+Shift+Enter), then call code /etc/gitconfig, delete the .cmd suffix in the core.editor setting, save, and try whether that works?

@segevfiner
Copy link

segevfiner commented Oct 15, 2021

Something is messed up with git config perhaps?, doing git config --global core.editor '"C:\Users\Segev\With Spaces\foo.cmd" --wait' (From PowerShell BTW, cause escaping rules might matter here). Causes it to add another core.editor setting with C:\Users\Segev\With.

EDIT: Might be just idiosyncratic PowerShell syntax, I'll try again in cmd.

@hansschuijff
Copy link
Author

@dscho I tried your suggestion and removing .cmd works here. In the previous version the command said code.exe instead of code.cmd. With .cmd removed from core.editor code opens again when I do the commit to enter the commit message.

segevfiner added a commit to segevfiner/build-extra that referenced this issue Oct 15, 2021
segevfiner added a commit to segevfiner/build-extra that referenced this issue Oct 15, 2021
They also include a `bin\code` bash script which seems to work, executing a `.cmd` doesn't seem to work for some reason.


Signed-off-by: Segev Finer <[email protected]>
@dscho
Copy link
Member

dscho commented Oct 15, 2021

@dscho I tried your suggestion and removing .cmd works here. In the previous version the command said code.exe instead of code.cmd. With .cmd removed from core.editor code opens again when I do the commit to enter the commit message.

Excellent!

Explanation:

  • When executing .exe programs, the MSYS2 Bash (or the MSYS2 runtime) do not have to jump through special hoops, a simple CreateProcessW() is enough.
  • The core.editor setting is actually executed through a shell invocation, therefore it handles the double-quoted path correctly.
  • Shell scripts, or more correctly: files starting with a #!<path-to-interpreter> line, are not actually executed directly via CreateProcessW(). Instead, the MSYS2 runtime has provisions to execute the interpreter with the file name as the first parameter.
  • Batch files, i.e. .cmd and .bat files, are somehow executed via cmd.exe (but I haven't really figured out where, exactly, my guess is that the lpApplicationName is left at NULL, contrary to CreateProcessW()'s documentation which specifically asks for it to be set to cmd.exe to execute batch files). It is my guess that things go wrong here, as the command line for cmd.exe would need to be enclosed in extra double quotes.
  • By using the code shell script that is included in VS Code (instead of the code.cmd batch file), we neatly side-step all the intricacies of quoting the arguments for cmd.exe correctly: we simply let the MSYS2 runtime handle it all.

[Hopefully](/segevfiner/build-extra/commit/a74cd3d384c61012fa315c3c1ca6047839ba0c46) [fix](/segevfiner/build-extra/commit/a74cd3d384c61012fa315c3c1ca6047839ba0c46) [git-for-windows/git#3471](https://github.com/git-for-windows/git/issues/3471)

@segevfiner how about including the information I provided in the commit message, and opening a PR?

@dscho
Copy link
Member

dscho commented Oct 15, 2021

@segevfiner how about including the information I provided in the commit message, and opening a PR?

Oh, I see, there is already git-for-windows/build-extra#385... 😄

@dscho dscho added this to the Next release milestone Oct 15, 2021
segevfiner added a commit to segevfiner/build-extra that referenced this issue Oct 15, 2021
… Code

There seems to be a quoting bug somewhere trying to execute an editor
that is a .cmd script with spaces in its path. VS Code also includes a
`bin\code` shell script which seems to work correctly even with spaces
in the path.

@dscho Explanation:
* When executing .exe programs, the MSYS2 Bash (or the MSYS2 runtime) do
  not have to jump through special hoops, a simple CreateProcessW() is
  enough.
* The core.editor setting is actually executed through a shell
  invocation, therefore it handles the double-quoted path correctly.
* Shell scripts, or more correctly: files starting with a
  #!<path-to-interpreter> line, are not actually executed directly via
  CreateProcessW(). Instead, the MSYS2 runtime has provisions to execute
  the interpreter with the file name as the first parameter.
* Batch files, i.e. .cmd and .bat files, are somehow executed via
  cmd.exe (but I haven't really figured out where, exactly, my guess is
  that the lpApplicationName is left at NULL, contrary to
  CreateProcessW()'s documentation which specifically asks for it to be
  set to cmd.exe to execute batch files). It is my guess that things go
  wrong here, as the command line for cmd.exe would need to be enclosed
  in extra double quotes.
* By using the code shell script that is included in VS Code (instead of
  the code.cmd batch file), we neatly side-step all the intricacies of
  quoting the arguments for cmd.exe correctly: we simply let the MSYS2
  runtime handle it all.

Fixes git-for-windows/git#3471

Signed-off-by: Segev Finer <[email protected]>
dscho added a commit to git-for-windows/build-extra that referenced this issue Oct 15, 2021
dscho added a commit to git-for-windows/build-extra that referenced this issue Oct 15, 2021
Configuring a system-wide VS Code as Git's editor [was
broken](git-for-windows/git#3471), which
has been fixed.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member

dscho commented Oct 15, 2021

@hansschuijff could I ask you to verify that the latest snapshot works as expected?

@hansschuijff
Copy link
Author

@dscho I'm not sure what you're asking, but I installed the snapshot and it worked for me. Note that before installing I had already changed the config to remove the file extension in the code.editor setting, like you suggested, so it worked before installing the snapshot too. Hope it helps. Thanks for the swift response on this issue.

@dscho
Copy link
Member

dscho commented Oct 15, 2021

I installed the snapshot and it worked for me.

Excellent!

Note that before installing I had already changed the config to remove the file extension in the code.editor setting, like you suggested, so it worked before installing the snapshot too.

Right, but the reinstallation would have overwritten your edits. That it works afterwards, still, means that we now write the correct value.

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

Successfully merging a pull request may close this issue.

4 participants