Skip to content

Conversation

@detjonmataj
Copy link

@detjonmataj detjonmataj commented Jul 13, 2025

Summary

Make subprocess calls safer when using commands containing paths with spaces and ensure they run with the current Python interpreter.

Changes

  • Use argument lists instead of interpolated command string so paths with spaces are escaped properly.
  • Replace hard‑coded "python"/"python3" with sys.executable for interpreter consistency.
  • Add a shell parameter to try_execute() to set shell=False in subprocess.Popen when passing commands as a list of args.

Related Issue

Closes #2950.

@detjonmataj
Copy link
Author

detjonmataj commented Jul 13, 2025

I think we should set shell=False in Popen for the changed commands in this PR, but it doesn’t seem to work on Windows (which is odd). We’ll probably need a different approach to escaping file paths. Any thoughts?

Edit:
I’ve added logic to use shell=False when passing the command as a list of arguments. Also, instead of hardcoding the Python executable name, it’s better to use sys.executable so we stay within the active environment (otherwise it might pick up the global Python rather than our .venv). Finally, the calls weren’t uniform as some used python3 and others just python which could lead to failures (depending on the environment or OS). Let me know what you think!

@detjonmataj detjonmataj marked this pull request as ready for review July 13, 2025 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] llama.cpp GGUF converter command fails on paths with spaces (Unsloth: Quantization failed for)

1 participant