gftools no longer supports spaces in working directory path

[RickyDaMa]

Minor issue of course, seemingly I'm the only person that bothers to make nice looking folder names lol

Updating a project I'm working on from v0.9.36 to v0.9.52

$ pwd
/home/atk/Fonts/Neat Font Name with Spaces

$ venv/bin/gftools builder sources/config.yaml
[1/3] buildVariable
FAILED: /tmp/tmp_leo9if2
/home/atk/Fonts/Neat Font Name with Spaces/venv/bin/python3 -m gftools.builder.jobrunner fontmake --output-path /tmp/tmp_leo9if2 -o variable -m NeatFont.designspace --filter ...  --filter FlattenComponentsFilter --filter DecomposeTransformedComponentsFilter --keep-overlaps
/bin/sh: line 1: /home/atk/Fonts/Neat: No such file or directory
ninja: build stopped: subcommand failed.

If I get some more time I'll try to track down where this is, but for now I'm filing this issue so I don't forget :)

[simoncozens]

The problem is here:

gftools/Lib/gftools/builder/operations/__init__.py

Line 55 in 4f61dcf

sys.executable + " -m gftools.builder.jobrunner " + cmd,

[anthrotype]

I guess sys.executable needs to be quoted ...

you can steal this from nanoemoji: https://github.com/googlefonts/nanoemoji/blob/fb4b0b3e10f7197e7fe33c4ae6949841e4440397/src/nanoemoji/util.py#L167-L176

(we actually have the same bug 😄 )

[RickyDaMa]

I can update my implementation to that, sure 👍

Edit: ah, looks shlex would be a new dependency. Worth adding?

[anthrotype]

I can update my implementation to that

in theory adding the "" should work most of the time, but one could potentially have not only spaces in the sys.executable path, but even quotes! I know, pathologic right?

I actually just tried on my mac to create a new folder named Neat Font Name with "literal quotes" really?!, and created a venv inside it. But when I try to activate the venv even bash complains...

[anthrotype]

looks shlex would be a new dependency. Worth adding?

it's built-in https://docs.python.org/3/library/shlex.html

[simoncozens]

But when I try to activate the venv even bash complains...

I don't see much point complicating the code to support cases which break the environment we're trying to run the code in... :-)

[RickyDaMa]

it's built-in

Story of my life with Python 😆 I'm too used to Rust

I've updated the PR, thanks for providing the reference

I don't see much point complicating the code to support cases which break the environment we're trying to run the code in... :-)

Yeah that's fair, but Marc's reviewed the PR and asked for it so I don't think it hurts here. I'll open a PR for bash next /s

[anthrotype]

I'll open a PR for bash next /s

it's probably not bash itself at fault but the venv/bin/activate shell script which may not be escaping the quotes in its path.
Probably somewhere here https://github.com/python/cpython/blob/main/Lib/venv/scripts/common/activate