Use designspace source names (replacement PR for #367)

[verbosus]

Here’s a first stab at an updated PR from @moyogo’s #367. I’ve added a bunch of tests to excercise all the various code paths.

[verbosus]

Oooops, need to lint the code with black, please hold.

[verbosus]

I don’t understand the lint error message. I ran black on the sources a few times but it still fails.

[anthrotype]

Maybe the version on black which is running on the CI is different from the one you have installed locally

[anthrotype]

It's isort that is failing apparently, not black. You can run all the lint checks locally using tox -e lint. You can edit the sources manually to accommodate the linter. Not sure if isort can automatically reformat sources

[verbosus]

Thanks, will do.

[verbosus]

I’m on Python 3.9.5, here are my Python packages including isort which is the same version as the server, 5.9.3:

appdirs==1.4.4
attrs==21.2.0
backports.entry-points-selectable==1.1.0
black==21.7b0
booleanOperations==0.9.0
cffsubr==0.2.8
click==8.0.1
compreffor==0.5.1
coverage==5.5
cu2qu==1.6.7
defcon==0.8.1
distlib==0.3.2
filelock==3.0.12
flake8==3.9.2
flake8-bugbear==21.4.3
fonttools==4.26.1
fs==2.4.13
iniconfig==1.1.1
isort==5.9.3
lxml==4.6.3
mccabe==0.6.1
mypy-extensions==0.4.3
packaging==21.0
pathspec==0.9.0
platformdirs==2.2.0
pluggy==0.13.1
py==1.10.0
pyclipper==1.3.0
pycodestyle==2.7.0
pyflakes==2.3.1
pyparsing==2.4.7
pytest==6.2.4
pytz==2021.1
regex==2021.8.21
six==1.16.0
skia-pathops==0.6.0.post2
toml==0.10.2
tomli==1.2.1
tox==3.24.3
-e git+ssh://git@github.com/verbosus/ufo2ft.git@52c3c0d456f86558fe6d3f42c6776b69f5f16433#egg=ufo2ft
ufoLib2==0.7.1
virtualenv==20.7.2

Here’s the isort output on my Mac if ran on the root directory:

$ isort --check-only --diff .
Skipped 3 files

If I run the command from inside the Lib/ufo2ft directory I get no output.

Can you repro the issue @anthrotype @moyogo?

[verbosus]

Here’s something funny: if I run tox -e lint on my branch everything is fine and the test passes. If I run it on main, the test fails.

I looked into what the server is actually doing, and it seems to be doing the right thing: it’s checking out main, merging my branch in and running the tests on that. I reproduced the exact commands from the Test + Deploy / lint (pull_request) action:

(ufo2ft):~/S/ufo2ft-orig:main$ 
git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +591693e2fe7d1fef4dcbbedb7d28685d2f7c5a36:refs/remotes/pull/527/merge
remote: Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
(ufo2ft):~/S/ufo2ft-orig:main$ git checkout --progress --force refs/remotes/pull/527/merge
Note: switching to 'refs/remotes/pull/527/merge'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 591693e Merge 52c3c0d456f86558fe6d3f42c6776b69f5f16433 into 0e8edb5a4c20648a52f314103e2c1f0cdb0d2cf9
(ufo2ft):~/S/ufo2ft-orig:(HEAD detached at pull/527/merge)$ tox -e lint
lint installed: appdirs==1.4.4,attrs==21.2.0,black==21.7b0,click==8.0.1,coverage==5.5,flake8==3.9.2,flake8-bugbear==21.4.3,iniconfig==1.1.1,isort==5.9.3,mccabe==0.6.1,mypy-extensions==0.4.3,packaging==21.0,pathspec==0.9.0,pluggy==0.13.1,py==1.10.0,pycodestyle==2.7.0,pyflakes==2.3.1,pyparsing==2.4.7,pytest==6.2.4,regex==2021.8.21,toml==0.10.2,tomli==1.2.1
lint run-test-pre: PYTHONHASHSEED='973146273'
lint run-test: commands[0] | black --check --diff .
All done! ✨ 🍰 ✨
52 files would be left unchanged.
lint run-test: commands[1] | isort --check-only --diff .
Skipped 2 files
lint run-test: commands[2] | flake8
________________________________________________________________ summary _________________________________________________________________
  lint: commands succeeded
  congratulations :)

[verbosus]

Still don’t quite understand what’s happening, in the meantime I found this: inconsistent isorting sorting between local and CI execution #1790 which seems not far from what I’m experiencing.

[verbosus]

I figured it out. The root directory must be called ufo2ft for the tests to pass. I often run multiple parallel clones of the same repo side-by-side for easy comparisons, etc. and was working from a directory called ufo2ft-debug. I found this whole process rather confusing. Admittedly, it’s the first time I’ve ever been exposed to isort and so it’s all very likely due to my inexperience, but I wouldn’t have thought my root directory name to matter. It shouldn’t, especially because our code lives inside Lib/ufo2ft.

[verbosus]

I think this PR is now ready for review. Let me know @anthrotype @moyogo

[anthrotype]

The root directory must be called ufo2ft for the tests to pass

I didn't know that either (@madig hooked up isort to ufo2ft linters). Yeah, that's a bit weird but I guess we can live with it.

I'll review the rest of the PR now

[verbosus]

@anthrotype I addressed a few of your comments. I need to look into refactoring the tests without using the UFOs I had already made.

[verbosus]

Converting PR back to draft as I refactor the tests.

[verbosus]

Back to ready for review. Can you take a look @anthrotype? Thanks.

[verbosus]

Looks like some of the designspace v5 work has addressed this elsewhere, because it’s now working correctly. So no need to keep the PR up anymore. Brilliant!

[verbosus]

Aaaaand we’re back. The family / style name overriding still does not work in case of a variable font, and it’s dubious whether it will in case of a static instance. I got confused because it works in fontmake thanks to the overriding taking places in its instancer, but it might still be broken here. So we need to revise the tests and still send this PR through, I think. I’ll do that and report back.