Skip to content
This repository was archived by the owner on Oct 24, 2025. It is now read-only.

Conversation

@AndrewFarley
Copy link
Contributor

@AndrewFarley AndrewFarley commented Sep 14, 2018

Fixes #242

Problem

Could not use this plugin with any path that contains a space character due to missing/insufficient shell escaping/quoting.

Solution

I'm sorry I couldn't find a simple "quote-all" solution similar to what was in place pre-merge. These solutions fall apart because certain things you can't quote or docker freaks out, for example features like the 'slim' would fall apart... eg: (see the last line)

   [ '-m',
     'pip',
     'install',
     '-t',
     '\'/root/test me/serverless-python-requirements-farley/tests/base/.serverless/requirements\'',
     '-r',
     '\'/root/test me/serverless-python-requirements-farley/tests/base/.serverless/requirements/requirements.txt\'',
     '" && find /root/test me/serverless-python-requirements-farley/tests/base/.serverless/requirements -name \\"*.so\\" -exec strip {} \';\'"' ]

The "simple" single test you added for testing in a subfolder only tests one of many use-cases that use various inputs for volumes, caching, etc. This MR I tested (as you see above) running ALL tests from a folder with a space in it, confirmed working. Just, not as clean as I would like, but I spent 4 hours trying to make it work with a quote all args type mechanism. If you don't like this MR's style, please let me know, but I'm out of effort/time/etc. I can post my diff for my attempts as using a simpler quote-all if interested. I did not, however, go test if this breaks anything on Windows. :\ We really need automated testing on Windows somehow @dschep.

@AndrewFarley
Copy link
Contributor Author

Ah crap, idk what is up with that failing test, but I'll look into it later this evening. :P

@dschep
Copy link
Contributor

dschep commented Sep 14, 2018

The error was docker not found which I assumed was a CircleCI issue, and upon retrying, tests pass, so it was!

@dschep dschep merged commit 8a32692 into serverless:master Sep 14, 2018
@AndrewFarley AndrewFarley deleted the bug-space-paths branch February 28, 2020 04:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants