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

Shell - Some commands were not successfully executed #170

Closed
shivapoudel opened this issue Sep 29, 2018 · 9 comments
Closed

Shell - Some commands were not successfully executed #170

shivapoudel opened this issue Sep 29, 2018 · 9 comments
Labels

Comments

@shivapoudel
Copy link

I am receiving Some commands were not successfully executed message with this below config :)
capture

This is my install.conf.yaml file. I am on Windows 10 with Dotbot 1.12.5:

- defaults:
    link:
      force: true
      create: true
      relink: true

- clean: ['~']

- link:
    ~/.gitconfig: git/gitconfig
    ~/.gitignore_global: git/gitignore_global
    ~/.minttyrc: mintty/minttyrc
    ~/.npmrc: npm/npmrc
    ~/.vim: vim/vim
    ~/.vimrc: vim/vimrc

- shell:
  - vim +PluginInstall +qall
@anishathalye
Copy link
Owner

Looks like your vim +PluginInstall +qall failed. Some steps I'd take when debugging:

  • try running it directly in your terminal to see if it returns successfully
  • run it with stdout: true / stderr: true in Dotbot and see if it prints any errors

@shivapoudel
Copy link
Author

Modify config shell setup to like this:

- shell:
    -
      command: vim +PluginInstall +qall
      stdout: true

No luck @anishathalye same error and I don't get any output too

@anishathalye
Copy link
Owner

Did you try running it directly? Also, did you try enabling stderr too?

@shivapoudel
Copy link
Author

@anishathalye I am using Mintty which is shipped in Git for Windows. Tried stderr too but no luck :)

@anishathalye
Copy link
Owner

What happens if you run the command directly?

@shivapoudel
Copy link
Author

It work with direct execution :)

capture
capture-1

@anishathalye
Copy link
Owner

I don't use vim plug, so I'm not very familiar with it. Does it require input on stdin before proceeding? Also, this issue seems somewhat related: junegunn/vim-plug#225

@shivapoudel
Copy link
Author

shivapoudel commented Sep 30, 2018

@anishathalye solved through running Git CMD with administritive privilage instead of Git Bash.

@anishathalye
Copy link
Owner

Nice, glad you got it working!

anishathalye added a commit that referenced this issue May 1, 2020
On POSIX-like systems, calling `subprocess.call()` with both
`shell=True` and `executable='...'` has the following behavior:

> If `shell=True`, on POSIX the _executable_ argument specifies a
> replacement shell for the default `/bin/sh`.

(via https://docs.python.org/3/library/subprocess.html?highlight=subprocess#popen-constructor)

This seems to have a similar behavior on Windows, but this is
problematic when a POSIX shell is substituted for cmd.exe. This is
because when `shell=True`, the shell is invoked with a '/c' argument,
which is the correct argument for cmd.exe but not for Bash, which
expects a '-c' argument instead. See here:
https://github.com/python/cpython/blob/1def7754b7a41fe57efafaf5eff24cfa15353444/Lib/subprocess.py#L1407

This is problematic when combined with Dotbot's behavior, where the
`executable` argument is set based on `$SHELL`. For example, when
running in Git Bash, the `$SHELL` environment variable is set to Bash,
so any commands run by Dotbot will fail (because it'll invoke Bash with
a '/c' argument).

This behavior of setting the `executable` argument based on `$SHELL` was
introduced in 7593d8c. This is the
desired behavior. See discussion in
#97 and
#100.

Unfortunately, this doesn't work quite right on Windows. This patch
works around the issue by avoiding setting the `executable` argument
when the platform is Windows, which is tested using
`platform.system() == 'Windows'`. This means that shell commands
executed by Dotbot on this platform will always be run using cmd.exe.
Invocations of single programs or simple commands will probably work
just fine in cmd.exe. If Bash-like behavior is desired, the user will
have to write their command as `bash -c '...'`.

This shouldn't have any implications for backwards-compatibility,
because setting the `executable` argument on Windows didn't do the right
thing anyways. Previous workarounds that users had should continue to
work with the new code.

When using Python from CYGWIN, `platform.system()` returns something
like 'CYGWIN_NT-...', so it won't be detected with the check, but this
is the correct behavior, because CYGWIN Python's `subprocess.call()` has
the POSIX-like behavior.

This patch also refactors the code to factor out the
`subprocess.call()`, which was being called in both `link.py` and
`shell.py`, so the workaround can be applied in a single place.

See the following issues/pull requests for a discussion of this bug:
- #170
- #177
- #219

An issue has also been raised in Python's issue tracker:
- https://bugs.python.org/issue40467

Thanks to @shivapoudel for originally reporting the issue, @SuJiKiNen
for debugging it and submitting a pull request, and @mohkale for
suggesting factoring out the code so that other plugins could use it.
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

2 participants