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

fix run with nested quotes #17487

Merged
merged 9 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
12 changes: 6 additions & 6 deletions conan/tools/env/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,18 @@ def environment_wrap_command(conanfile, env_filenames, env_folder, cmd, subsyste
if bats:
launchers = " && ".join('"{}"'.format(b) for b in bats)
if ps1s:
ps1_launchers = f'{powershell} -Command "' + " ; ".join('&\'{}\''.format(f) for f in ps1s) + '"'
cmd = cmd.replace('"', "'")
return '{} && {} ; cmd /c {}'.format(launchers, ps1_launchers, cmd)
ps1_launchers = f'{powershell} -Command "' + " ; ".join('&\'{}\''.format(f) for f in ps1s)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello @czoido

I am pretty sure the .join('&\'{}\''.format(f) for f in ps1s) could be changed as well to .join('{}'.format(f) for f in ps1s)
The minimal local tests I have run seem to go well with this change and it does improve readability in my opinion
I think the & syntax is not necessary with the -Command anymore. Am I mistaken?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @Todiq,
Removing the & when invoking the scripts causes that the environment variables that should be injected from the ps1 files will not be applied correctly in the execution contexts, so I'm afraid those are necessary.
Is it possible to give us an small reproducible example to be able to add a test to check what this PR is fixing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just added some tests.
Reproducible case is here : conan-io/conan-center-index#26187

Copy link
Contributor

@czoido czoido Dec 18, 2024

Choose a reason for hiding this comment

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

Thank you so much @Todiq , I could reproduce the issue, let me check what you added and simplify a bit.

cmd = cmd.replace('"', r'\"')
return '{} && {} ; cmd /c {}"'.format(launchers, ps1_launchers, cmd)
czoido marked this conversation as resolved.
Show resolved Hide resolved
else:
return '{} && {}'.format(launchers, cmd)
elif shs:
launchers = " && ".join('. "{}"'.format(f) for f in shs)
return '{} && {}'.format(launchers, cmd)
elif ps1s:
ps1_launchers = f'{powershell} -Command "' + " ; ".join('&\'{}\''.format(f) for f in ps1s) + '"'
cmd = cmd.replace('"', "'")
return '{} ; cmd /c {}'.format(ps1_launchers, cmd)
ps1_launchers = f'{powershell} -Command "' + " ; ".join('&\'{}\''.format(f) for f in ps1s)
cmd = cmd.replace('"', r'\"')
return '{} ; cmd /c {}"'.format(ps1_launchers, cmd)
czoido marked this conversation as resolved.
Show resolved Hide resolved
else:
return cmd

Expand Down
98 changes: 98 additions & 0 deletions test/functional/toolchains/env/test_virtualenv_powershell.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os
import platform
import sys
import textwrap

import pytest
Expand Down Expand Up @@ -232,3 +233,100 @@ class Pkg(ConanFile):
assert "Boolean values for 'tools.env.virtualenv:powershell' are deprecated" in client.out
else:
assert "Boolean values for 'tools.env.virtualenv:powershell' are deprecated" not in client.out

@pytest.mark.skipif(platform.system() != "Windows", reason="Requires Windows powershell")
@pytest.mark.parametrize("powershell", [True, "powershell.exe", "pwsh", "powershell.exe -NoProfile", "pwsh -NoProfile"])
def test_virtualenv_quoted_command(powershell):
""" This tests a command that contains single quotes through cmd /c via powershell
"""
client = TestClient()
test_package = textwrap.dedent(r"""
import sys
from conan import ConanFile
from conan.tools.files import save
class Test(ConanFile):
def requirements(self):
self.requires(self.tested_reference_str)
def generate(self):
# Emulates vcvars.bat behavior
save(self, "myenv.bat", "echo MYENV!!!\nset MYVC_CUSTOMVAR1=PATATA1")
self.env_scripts.setdefault("build", []).append("myenv.bat")
save(self, "myps.ps1", "echo MYPS1!!!!\n$env:MYVC_CUSTOMVAR2=\"PATATA2\"")
self.env_scripts.setdefault("build", []).append("myps.ps1")
def layout(self):
self.folders.build = "mybuild"
self.folders.generators = "mybuild"
def test(self):
py_exe = sys.executable
py_command = "from __future__ import print_function; import sys; print('{}.{}'.format(sys.version_info.major, sys.version_info.minor))"
command = f'"{py_exe}" -c "{py_command}"'
self.run(command)
self.run("set MYVC_CUSTOMVAR1")
self.run("set MYVC_CUSTOMVAR2")
""")
client.save({"conanfile.py": GenConanfile("pkg", "1.0"),
"test_package/conanfile.py": test_package})
client.run("create .")
assert f"{sys.version_info.major}.{sys.version_info.minor}" in client.out
assert "MYENV!!!" in client.out
assert "MYPS1!!!!" in client.out
assert "MYVC_CUSTOMVAR1=PATATA1" in client.out
assert "MYVC_CUSTOMVAR2=PATATA2" in client.out
# This was crashing because the .ps1 of test_package was not being cleaned
client.run(f"create . -c tools.env.virtualenv:powershell='{powershell}'")
assert "MYENV!!!" in client.out
assert "MYPS1!!!!" in client.out
assert "MYVC_CUSTOMVAR1=PATATA1" in client.out
assert "MYVC_CUSTOMVAR2=PATATA2" in client.out

@pytest.mark.skipif(platform.system() != "Windows", reason="Test for powershell")
@pytest.mark.parametrize("powershell", [True, "powershell.exe", "pwsh", "powershell.exe -NoProfile", "pwsh -NoProfile"])
def test_concatenated_build_and_run_with_quoted_command(powershell):
# this tests that if we have both build and run env, they are concatenated correctly when using
# powershell
client = TestClient(path_with_spaces=True)
compiler_bat = "@echo off\necho MYTOOL {}!!\n"
conanfile = textwrap.dedent("""\
import os
from conan import ConanFile
from conan.tools.files import copy
class Pkg(ConanFile):
exports_sources = "*"
package_type = "application"
def package(self):
copy(self, "*", self.build_folder, os.path.join(self.package_folder, "bin"))
""")

num_deps = 2
for i in range(num_deps):
client.save({"conanfile.py": conanfile,
"mycompiler{}.bat".format(i): compiler_bat.format(i)})
client.run("create . --name=pkg{} --version=0.1".format(i))

conanfile = textwrap.dedent("""\
from conan import ConanFile
class Pkg(ConanFile):
settings = "os"
tool_requires = "pkg0/0.1"
requires = "pkg1/0.1"
generators = "VirtualBuildEnv", "VirtualRunEnv"
""")

client.save({"conanfile.py": conanfile}, clean_first=True)
client.run(f'install . -c tools.env.virtualenv:powershell="{powershell}"')
conanfile = ConanFileMock()
conanfile.conf.define("tools.env.virtualenv:powershell", powershell)
py_exe = sys.executable
py_command = "from __future__ import print_function; import sys; print('{}.{}'.format(sys.version_info.major, sys.version_info.minor))"
quoted_command = f'"{py_exe}" -c "{py_command}"'
cmd = environment_wrap_command(conanfile,["conanrunenv", "conanbuildenv"],
client.current_folder,f"{quoted_command} ; mycompiler0.bat")
client.run_command(cmd)
assert "MYTOOL 0!!" in client.out
assert f"{sys.version_info.major}.{sys.version_info.minor}" in client.out

cmd = environment_wrap_command(conanfile,["conanrunenv", "conanbuildenv"],
client.current_folder,f"{quoted_command} ; mycompiler1.bat")
client.run_command(cmd)
assert "MYTOOL 1!!" in client.out
assert f"{sys.version_info.major}.{sys.version_info.minor}" in client.out
Loading