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

Quote conda prefix in install command #13070

Closed
wants to merge 1 commit into from

Conversation

mvdbeek
Copy link
Member

@mvdbeek mvdbeek commented Dec 16, 2021

You can see this fail in our first_startup tests at https://github.com/galaxyproject/galaxy/runs/4547495148?check_suite_focus=true#step:11:1827

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these contributions under Galaxy's current license.
  • I agree to allow the Galaxy committers to license these and all my past contributions to the core galaxy codebase under the MIT license. If this condition is an issue, uncheck and just let us know why with an e-mail to [email protected].

@mvdbeek
Copy link
Member Author

mvdbeek commented Dec 16, 2021

Installing conda into a path with a space doesn't work at all ... https://github.com/mvdbeek/galaxy/runs/4549243765?check_suite_focus=true#step:11:1590
I guess there's no harm merging this, even if we're still out of luck here.

@nsoranzo
Copy link
Member

Installing conda into a path with a space doesn't work at all ...

Yep, I know, I have an open PR to fix that since May: conda/constructor#449

@@ -431,7 +431,7 @@ def install_conda(conda_context, force_conda_build=False):
with tempfile.NamedTemporaryFile(suffix=".sh", prefix="conda_install", delete=False) as temp:
script_path = temp.name
download_cmd = commands.download_command(conda_link(), to=script_path)
install_cmd = ['bash', script_path, '-b', '-p', conda_context.conda_prefix]
install_cmd = ['bash', script_path, '-b', '-p', shlex.quote(conda_context.conda_prefix)]
Copy link
Member

Choose a reason for hiding this comment

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

I'm quite sure this is not necessary, and actually breaks it if conda_context.conda_prefix contains a space. E.g.:

In [1]: import shlex                                                                                                                                                                                       

In [2]: from galaxy.util.commands import shell                                                                                                                                                             

In [3]: shell(["ls", "/tmp/galaxy root"])                                                                                                                                                                  
foo
Out[3]: 0

In [4]: shell(["ls", shlex.quote("/tmp/galaxy root")])                                                                                                                                                     
ls: cannot access "'/tmp/galaxy root'": No such file or directory
Out[4]: 2

@mvdbeek mvdbeek closed this Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants