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.

Sign up for GitHub

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

Jump to bottom

BugFix: allow run_in_windows_bash in MSYS/Cygwin #8506

Merged
merged 1 commit into from
Feb 22, 2021
Merged

BugFix: allow run_in_windows_bash in MSYS/Cygwin #8506

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions conans/client/tools/win.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ def run_in_windows_bash(conanfile, bashcmd, cwd=None, subsystem=None, msys_mingw
It requires to have MSYS2, CYGWIN, or WSL
"""
env = env or {}
if platform.system() != "Windows":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cygwin also returns "Windows" here, could be only Msys will be different

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't check Cygwin (may check tomorrow), msys returns a different value for sure

Copy link
Member

@memsharded memsharded Feb 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix seems fine anyway, it was just an observation. Would you like to check something more, or can it be merged?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the completeness.
Cygwin:

sse4@DESKTOP-LAD63V0 ~
$ python -c "import platform; print(platform.system())"
CYGWIN_NT-10.0-18363

MSYS2:

sse4@DESKTOP-LAD63V0 MSYS ~
$ python -c "import platform; print(platform.system())"
MSYS_NT-10.0-18363

MinGW32 (MSYS2):

sse4@DESKTOP-LAD63V0 MINGW32 ~
$ python -c "import platform; print(platform.system())"
MINGW32_NT-10.0-18363

MinGW64 (MSYS2):

sse4@DESKTOP-LAD63V0 MINGW64 ~
$ python -c "import platform; print(platform.system())"
MINGW64_NT-10.0-18363

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curiously, it seems it can change, I am not sure why:

Cygwin:

$ python -c "import platform; print(platform.system())"
Windows

In any case, doesn't seem relevant, if it can change in other computers, need to take it into account.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is probably not a python from a Cygwin itself. run which python to check.
if it's /usr/bin/python, then it's cygwin's one.
if it's something like /cygdrive/c/users..., then it's installed in the main Windows system.

if not OSInfo().is_windows:
raise ConanException("Command only for Windows operating system")
subsystem = subsystem or OSInfo.detect_windows_subsystem()

Expand Down Expand Up @@ -693,7 +693,11 @@ def get_path_value(container, subsystem_name):
bash_path = OSInfo.bash_path()
bash_path = '"%s"' % bash_path if " " in bash_path else bash_path
login = "--login" if with_login else ""
wincmd = '%s %s -c %s' % (bash_path, login, escape_windows_cmd(to_run))
if platform.system() == "Windows":
# cmd.exe shell
wincmd = '%s %s -c %s' % (bash_path, login, escape_windows_cmd(to_run))
else:
wincmd = '%s %s -c %s' % (bash_path, login, to_run)
conanfile.output.info('run_in_windows_bash: %s' % wincmd)

# If is there any other env var that we know it contains paths, convert it to unix_path
Expand Down