Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ttx_diff] Use pip-compile and requirements.in #1090

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions fontc_crater/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,17 @@ script in the [`fontc_crater` repo][crater-repo] and results are posted to
To run in CI mode locally to play with the html output:

When run in CI, we want to ensure that we have complete control over the set of
python pacakges we are comparing against; to ensure this we use `constraints.txt`
in the root `resources/scripts` directory. To update the set of python packages
in use, you need to update this file:
python pacakges we are comparing against; to ensure this we use `pip-compile`
(part of [`pip-tools`]) to generate a pinned `requirements.txt` from a
`requirements.in` file in the root `resources/scripts` directory. To update the
set of python packages in use, you need to update this file:

- `rm -rf` your existing venv
- create a new venv and activate it
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one doesn't need to delete and create a new venv when using pip-compile, it may well run from a global site-packages (e.g. I install it globally with pipx install pip-tools) as pip-compile does not modify its environment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

installing something in a venv is generally good practice especially when developing onto something, in this case we are simply using a development tool and it's not a requirement that it runs within a venv, so I don't think you should mention this here

- `pip install -r resources/scripts/requirements.txt`
- `pip freeze > resources/scripts/constraints.txt`
- install pip-tools with `python -m pip install pip-tools`
- delete `resources/scripts/requirements.txt`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need to delete it as it will be overwritten, but it's ok

- run `pip-compile resources/scripts/requirements.in`
- commit the new `requirements.txt` file.

```shell
# clone [email protected]:googlefonts/fontc_crater.git somewhere, we'll assume at ../fontc_crater
Expand All @@ -45,5 +48,4 @@ $ cargo run --release -p=fontc_crater -- ci ../fontc_crater/gf-repos-2024-09-23.
[rust-lang/crater]: https://github.com/rust-lang/crater
[crater-repo]: https://github.com/googlefonts/fontc_crater
[crater-results]: https://googlefonts.github.io/fontc_crater/


[`pip-tools`]: https://github.com/jazzband/pip-tools
97 changes: 0 additions & 97 deletions resources/scripts/constraints.txt

This file was deleted.

12 changes: 12 additions & 0 deletions resources/scripts/requirements.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
absl-py
# keep fontmake version pinned to ensure output from ttx_diff.py is stable;
# the 'repacker' option enables faster GSUB/GPOS serialization via uharfbuzz
fontmake[repacker]==3.10.0
# technically fonttools is in turn a dependency of fontmake but a few of
# our scripts import it directly, so we list it among the top-level requirements.
fonttools
lxml
cdifflib
glyphsLib
# our custom branch of gftools
git+https://github.com/googlefonts/gftools.git@fontc-flag-sketch
Loading
Loading