-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix poetry env activate for Bash on Windows
#10415
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
base: main
Are you sure you want to change the base?
Changes from all commits
5079946
bd286c9
c878e16
f21beee
bfc048f
9de611f
e26a8c1
7b430e7
ec0cacc
37ed6b7
1d68240
1d20eda
ce23789
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,3 +17,6 @@ | |
|
|
||
| .venv | ||
| /poetry.toml | ||
|
|
||
| # Ignore Python bytecode and cache directories | ||
| __pycache__/ | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -28,7 +28,7 @@ def handle(self) -> int: | |||||||||||||||||||
| env = EnvManager(self.poetry).get() | ||||||||||||||||||||
|
|
||||||||||||||||||||
| try: | ||||||||||||||||||||
| shell, _ = shellingham.detect_shell() | ||||||||||||||||||||
| shell, *_ = shellingham.detect_shell() | ||||||||||||||||||||
| except shellingham.ShellDetectionFailure: | ||||||||||||||||||||
| shell = "" | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -41,30 +41,39 @@ def handle(self) -> int: | |||||||||||||||||||
| ) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| def _get_activate_command(self, env: Env, shell: str) -> str: | ||||||||||||||||||||
| if shell == "fish": | ||||||||||||||||||||
| command, filename = "source", "activate.fish" | ||||||||||||||||||||
| elif shell == "nu": | ||||||||||||||||||||
| command, filename = "overlay use", "activate.nu" | ||||||||||||||||||||
| elif shell in ["csh", "tcsh"]: | ||||||||||||||||||||
| command, filename = "source", "activate.csh" | ||||||||||||||||||||
| elif shell in ["powershell", "pwsh"]: | ||||||||||||||||||||
| command, filename = ".", "activate.ps1" | ||||||||||||||||||||
| shell_configs = { | ||||||||||||||||||||
| "fish": ("source", "activate.fish"), | ||||||||||||||||||||
| "nu": ("overlay use", "activate.nu"), | ||||||||||||||||||||
| "csh": ("source", "activate.csh"), | ||||||||||||||||||||
| "tcsh": ("source", "activate.csh"), | ||||||||||||||||||||
| "powershell": (".", "activate.ps1"), | ||||||||||||||||||||
| "pwsh": (".", "activate.ps1"), | ||||||||||||||||||||
| "cmd": (".", "activate.bat"), | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| command, filename = shell_configs.get(shell, ("source", "activate")) | ||||||||||||||||||||
|
|
||||||||||||||||||||
| activation_script = env.bin_dir / filename | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if not activation_script.exists(): | ||||||||||||||||||||
| if shell == "cmd" and not WINDOWS: | ||||||||||||||||||||
| fallback_script = env.bin_dir / "activate" | ||||||||||||||||||||
| if fallback_script.exists(): | ||||||||||||||||||||
| return f"source {self._quote(str(fallback_script), 'bash')}" | ||||||||||||||||||||
| return "" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if shell in ["powershell", "pwsh"]: | ||||||||||||||||||||
| return f'& "{activation_script}"' | ||||||||||||||||||||
| elif shell == "cmd": | ||||||||||||||||||||
| command, filename = ".", "activate.bat" | ||||||||||||||||||||
| return f'"{activation_script}"' | ||||||||||||||||||||
| else: | ||||||||||||||||||||
| command, filename = "source", "activate" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| if (activation_script := env.bin_dir / filename).exists(): | ||||||||||||||||||||
| if WINDOWS: | ||||||||||||||||||||
| return f"{self._quote(str(activation_script), shell)}" | ||||||||||||||||||||
| return f"{command} {self._quote(str(activation_script), shell)}" | ||||||||||||||||||||
| return "" | ||||||||||||||||||||
|
|
||||||||||||||||||||
| @staticmethod | ||||||||||||||||||||
| def _quote(command: str, shell: str) -> str: | ||||||||||||||||||||
| if WINDOWS: | ||||||||||||||||||||
| if shell == "cmd": | ||||||||||||||||||||
| return f'"{command}"' | ||||||||||||||||||||
| if shell in ["powershell", "pwsh"]: | ||||||||||||||||||||
| return f'& "{command}"' | ||||||||||||||||||||
| return shlex.quote(command) | ||||||||||||||||||||
| if WINDOWS and shell not in ["powershell", "pwsh", "cmd"]: | ||||||||||||||||||||
| return shlex.quote(command) | ||||||||||||||||||||
| elif shell in {"powershell", "pwsh", "cmd"}: | ||||||||||||||||||||
| return command | ||||||||||||||||||||
| else: | ||||||||||||||||||||
| return shlex.quote(command) | ||||||||||||||||||||
|
Comment on lines
+74
to
+79
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This is equivalent, isn't it? |
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,6 +77,118 @@ def test_env_activate_prints_correct_script_on_windows( | |
| assert line == f'{prefix}"{tmp_venv.bin_dir / ext!s}"' | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "shell, ext, expected_prefix", | ||
| ( | ||
| ("cmd", "activate.bat", ""), | ||
| ("pwsh", "activate.ps1", "& "), | ||
| ("powershell", "activate.ps1", "& "), | ||
| ), | ||
| ) | ||
| @pytest.mark.skipif(not WINDOWS, reason="Only Windows shells") | ||
| def test_env_activate_windows_shells_get_quoted_path_only( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is more or less just a copy of |
||
| tmp_venv: VirtualEnv, | ||
| mocker: MockerFixture, | ||
| tester: CommandTester, | ||
| shell: str, | ||
| ext: str, | ||
| expected_prefix: str, | ||
| ) -> None: | ||
| mocker.patch("shellingham.detect_shell", return_value=(shell, None)) | ||
| mocker.patch("poetry.utils.env.EnvManager.get", return_value=tmp_venv) | ||
|
|
||
| tester.execute() | ||
|
|
||
| line = tester.io.fetch_output().rstrip("\n") | ||
| expected = f'{expected_prefix}"{tmp_venv.bin_dir / ext!s}"' | ||
| assert line == expected | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "shell, command, ext", | ||
| ( | ||
| ("bash", "source", ""), | ||
| ("zsh", "source", ""), | ||
| ("fish", "source", ".fish"), | ||
| ("nu", "overlay use", ".nu"), | ||
| pytest.param( | ||
| "csh", | ||
| "source", | ||
| ".csh", | ||
| marks=pytest.mark.skipif( | ||
| WINDOWS, reason="csh activator not created on Windows" | ||
| ), | ||
| ), | ||
| pytest.param( | ||
| "tcsh", | ||
| "source", | ||
| ".csh", | ||
| marks=pytest.mark.skipif( | ||
| WINDOWS, reason="tcsh activator not created on Windows" | ||
| ), | ||
| ), | ||
| ("sh", "source", ""), | ||
| ), | ||
| ) | ||
| def test_env_activate_unix_shells_get_command_with_path( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks like a copy of |
||
| tmp_venv: VirtualEnv, | ||
| mocker: MockerFixture, | ||
| tester: CommandTester, | ||
| shell: str, | ||
| command: str, | ||
| ext: str, | ||
| ) -> None: | ||
| mocker.patch("shellingham.detect_shell", return_value=(shell, None)) | ||
| mocker.patch("poetry.utils.env.EnvManager.get", return_value=tmp_venv) | ||
|
|
||
| tester.execute() | ||
|
|
||
| line = tester.io.fetch_output().rstrip("\n") | ||
| expected_path = f"{tmp_venv.bin_dir}/activate{ext}" | ||
| if WINDOWS: | ||
| import shlex | ||
|
|
||
| quoted_path = shlex.quote(str(tmp_venv.bin_dir / f"activate{ext}")) | ||
| expected = f"{command} {quoted_path}" | ||
| else: | ||
| expected = f"{command} {expected_path}" | ||
|
|
||
| assert line == expected | ||
|
|
||
|
|
||
| def test_env_activate_bash_on_windows_gets_source_command( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is already covered by |
||
| tmp_venv: VirtualEnv, | ||
| mocker: MockerFixture, | ||
| tester: CommandTester, | ||
| ) -> None: | ||
| mocker.patch("shellingham.detect_shell", return_value=("bash", None)) | ||
| mocker.patch("poetry.utils.env.EnvManager.get", return_value=tmp_venv) | ||
|
|
||
| mocker.patch("poetry.console.commands.env.activate.WINDOWS", True) | ||
|
|
||
| tester.execute() | ||
|
|
||
| line = tester.io.fetch_output().rstrip("\n") | ||
|
|
||
| assert line.startswith("source ") | ||
| assert "activate" in line | ||
| assert not (line.startswith('"') and line.endswith('"') and "source" not in line) | ||
|
|
||
|
|
||
| def test_env_activate_unknown_shell_defaults_to_source( | ||
| tmp_venv: VirtualEnv, | ||
| mocker: MockerFixture, | ||
| tester: CommandTester, | ||
| ) -> None: | ||
| mocker.patch("shellingham.detect_shell", return_value=("unknown_shell", None)) | ||
| mocker.patch("poetry.utils.env.EnvManager.get", return_value=tmp_venv) | ||
|
|
||
| tester.execute() | ||
|
|
||
| line = tester.io.fetch_output().rstrip("\n") | ||
| assert line.startswith("source ") | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("verbosity", ["", "-v", "-vv", "-vvv"]) | ||
| def test_no_additional_output_in_verbose_mode( | ||
| tmp_venv: VirtualEnv, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this for? I know that Powershell exists for Linux but cmd? Does this cover a real-world use case? Does a test fail if we remove this?