fonttools / fontbakery

šŸ§ A font quality assurance tool for everyone
https://fontbakery.readthedocs.io
Apache License 2.0
534 stars 99 forks source link

Running TN profile from GitHub action not working #4620

Closed eweracs closed 3 months ago

eweracs commented 3 months ago

Observed behaviour

I am trying to run the TN profile from a GitHub action. The universal profile works fine. However, when I try to run the TN profile, I get the following:

Start ... running 518 individual check executions.

To run the typenetwork profile, one needs to install
fontbakery with the 'typenetwork' extra, like this:

python -m pip install -U 'fontbakery[typenetwork]'

I have the installation line set up as python -m pip install -U 'fontbakery[typenetwork]', but I get the same result.

Expected behaviour

I would expect the TN profile to work following the instructions provided. It works fine from my local Terminal.

Resources and steps needed to reproduce

Try running FB through a GitHub action with the TN profile.

felipesanches commented 3 months ago

I was not able to replicate this behaviour with the latest git revision. But after some inspection, I found out that this only happens on revisions older than March 18, 2024, when @simoncozens committed a change with this log-message:

Screenshot from 2024-03-30 17-29-41

It means you're trying to run it from an old revision. You need to update your local copy of the git repo by running these commands:

git fetch origin git rebase origin/main

note: Here I am assuming that origin is a git-remote pointing to https://github.com/fonttools/fontbakery.git which is the usual state of git after a simple git-clone, which is what I guess you did a couple weeks ago or a bit before that.

felipesanches commented 3 months ago

oh! Now I have re-read your original message and noticed that "from GitHub" does not mean you're running from git repo.

So, my suggestion is that you change the install procedures to do this:

python -m pip install --pre -U 'fontbakery[typenetwork]'

By using the --pre flag, it will install the pre-releases, which are updated more frequently and at this moment already include the commit that fixed this problem.

eweracs commented 3 months ago

Great, thanks, I will give that a try.

eweracs commented 3 months ago

Hello, I tried with the --pre flag, but now I get the following when running with the TN profile:

Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.12.2/x64/bin/fontbakery", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/fontbakery/cli.py", line 423, in main
    profile = profile_factory(get_module(args.profile))
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/fontbakery/fonts_profile.py", line 150, in profile_factory
    add_checks_to_nascent_profile(
  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/fontbakery/fonts_profile.py", line 1[16](https://github.com/mnkytype/ella/actions/runs/8499420275/job/23280223641#step:6:17), in add_checks_to_nascent_profile
    raise ValueError(f"Check {check} not found")
ValueError: Check com.google.fonts/check/family/panose_proportion not found
guidoferreyra commented 3 months ago

Not sure why is complaining about Ā“com.google.fonts/check/family/panose_proportionĀ“ TN profile doesnā€™t use it since it was deprecated some time ago.

eweracs commented 3 months ago

Does running FB with the TN profile through a GitHub action work for you? My basic structure is this:

name: Run FB

on:
  workflow_call:

jobs:
  fontbakery-check:
    runs-on: ubuntu-latest

    steps:
    - name: Checkout Repository
      uses: actions/checkout@v2

    - name: Set up Python
      uses: actions/setup-python@v2
      with:
        python-version: 3.x

    - name: Install Fontbakery
      run: |
        python -m pip install --pre -U 'fontbakery[typenetwork]'

    - name: Run Fontbakery Check
      run: |        
        fontbakery check-typenetwork -F --html "CFF_report.html" "*.otf" || true
guidoferreyra commented 3 months ago

I didnā€˜t tested on a GH action, but I get the same error when installing from pip (-pre included).

felipesanches commented 3 months ago

I'm preparing a new pre-release to include the fixes needed here.

eweracs commented 3 months ago

Brilliant. Thank you!

felipesanches commented 3 months ago

FontBakery v0.12.0a5 is now available. Please re-run your Github Actions job. It should work properly now. Please let me know.

felipesanches commented 3 months ago

@eweracs, we can reopen this issue if anything still fails to work properly for you after this release. Let us know.

eweracs commented 3 months ago

Thanks a lot! I'm not sure whether this is related to the new update, but I now get an ERROR for the FontValidator check:

ERROR Failed with FileNotFoundError: [Errno 2] No such file or directory: 'FontValidator'

  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/fontbakery/checkrunner.py", line 213, in _run_check
    subresults = list(subresults)
                 ^^^^^^^^^^^^^^^^
  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/fontbakery/checks/fontval.py", line 182, in com_google_fonts_check_fontvalidator
    raise error
  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/site-packages/fontbakery/checks/fontval.py", line 160, in com_google_fonts_check_fontvalidator
    subprocess.check_output(fval_cmd, stderr=subprocess.STDOUT)
  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/subprocess.py", line 466, in check_output
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/subprocess.py", line 548, in run
    with Popen(*popenargs, **kwargs) as process:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/subprocess.py", line 1026, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/subprocess.py", line 1953, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
felipesanches commented 3 months ago
eweracs commented 3 months ago

I'm running it as a github action, see my yaml script above. This means that FontValidator is not automatically installed when running

python -m pip install --pre -U 'fontbakery[typenetwork]'

Is this intentional?

felipesanches commented 3 months ago

FontValidator instalation is optional, and must be done manually, if needed, because there's no python package wrapping FontVal

eweracs commented 3 months ago

Odd. This never used to be an issue when running FB. Is this check new?

simoncozens commented 3 months ago

The fontvalidator check has been part of the TypeNetwork profile ever since it was initially added:

https://github.com/fonttools/fontbakery/blob/0908599dcde3742ef5f2381a9291afdb3644e8a7/Lib/fontbakery/profiles/typenetwork.py#L73-L76

eweracs commented 3 months ago

So why am I now getting an error I was not getting previously? This never used to happen, only since I started installing fontbakery with fontbakery[typenetwork]. How can I resolve this?

simoncozens commented 3 months ago

I don't know why you were not getting the error before. Maybe the check wasn't working. As Felipe explained above, you can either skip the FontValidator check (-x fontval on the command line) or you can install FontValidator.

eweracs commented 3 months ago

All right, thank you for the clarification.

guidoferreyra commented 2 months ago

I never had this issue before the last "architecture" change and I never had to install it manually. FontVal was on TN profile since AFAIK it was part of Universal too. Now I see there is no other profile using it, right? I guess we can remove it from TNā€™s profile.

eweracs commented 2 months ago

Hello again. Just a note. Ever since FB 0.12.0, the FontVal check results in an error: No file or directory: "FontValidator". This didn't use to occur.

I don't have FontValidator installed (yet), but I'm wondering why this wasn't an issue previously.