make: add googlefonts target #678

m4rc1e · 2024-02-06T14:04:59Z

This pull requests adds a googlefonts target to the makefile. It will create a new set of variable fonts that comply with the GF spec. Personally, I feel this approach is cleaner than GF maintaining a fork.

@rsms many people have requested an updated version of Inter on Google Fonts so I'm hoping this solution works for you,
google/fonts#3429. We may end up calling the family Inter 4 if we cannot all agree that the regressions are not going to cause people to complain.

m4rc1e · 2024-03-14T13:56:40Z

Friendly ping @rsms

rsms

Thank you for this. I agree that this is a cleaner solution than GF maintaining a fork :•)

rsms · 2024-03-20T19:22:16Z

Pipfile

@@ -10,15 +10,15 @@ ufo2ft = "==2.30.0"
 fontmake = "==3.5.*"
 fontbakery = "==0.10.4"
 skia-pathops = "==0.8.*"
-gftools = "==0.9.*"
+gftools = "*"


Please pin the version to whatever GF requires.
Accepting "any version" can break CI (it has happened in the past)

rsms · 2024-03-20T19:26:59Z

misc/makezip2.sh

@@ -58,7 +58,8 @@ cp build/fonts/var/InterVariable-Italic.ttf   "$ZIPDIR/InterVariable-Italic.ttf"
 cp build/fonts/static/Inter*.woff2            "$ZIPDIR/web/" &
 cp build/fonts/var/InterVariable.woff2        "$ZIPDIR/web/InterVariable.woff2"
 cp build/fonts/var/InterVariable-Italic.woff2 "$ZIPDIR/web/InterVariable-Italic.woff2"
-cp misc/dist/inter.css                         "$ZIPDIR/web/"
+cp misc/dist/inter.css                        "$ZIPDIR/web/"
+cp build/fonts/googlefonts/Inter*.ttf         "$ZIPDIR/googlefonts/"


(Please revert this change)

Was this accidental? This script (makezip2.sh) is used to create the offician distribution, so it shouldn't contain Google Fonts files.

If the intention was to get a zip file of the googlefonts make target, I suggest adding a custom zip ... invocation to the googlefonts make target.

(Not affiliated with the Google Fonts team)

Why are there makezip.sh and makezip2.sh? Is the former obsolete?

@mon-jai something like that, I forget. None of them are intended to be used directly, just to reduce the complexity of the makefile.

Maybe we could delete the obsolete one?

Thanks for the review.

This change is intentional. In order for the fonts to be included in Google Fonts, they must reside in a zip file that's attached to the release. The easiest way for me to do this is to bundle the gf edition fonts into the existing zip. If you want them in a separate zip, I can tweak the github action. Either way, whenever you cut a new version, we'll need the googlefonts fonts to be generated and included with the release.

Got it! Thanks for clarifying.
Github actions is only used for CI. The actual release I do manually, so, we can add a second zip file e.g. "Inter-VERSION-GoogleFonts.zip" which can then be fetched from github releases (scripted or manually.)
I'll add a patch in a separate comment for you.

rsms · 2024-03-22T18:44:53Z

Patch for making a separate Inter-VERSION-GoogleFonts.zip file

--- a/Makefile	2024-03-22 11:40:10.000000000 -0700
+++ b/Makefile	2024-03-22 11:43:07.000000000 -0700
@@ -424,6 +424,7 @@
 # - step2 runs tests, then makes a zip archive and updates the website (docs/ dir.)
 
 DIST_ZIP = build/release/Inter-${VERSION}.zip
+DIST_ZIP_GF = $(SRCDIR)/build/release/Inter-$(VERSION)-GoogleFonts.zip
 
 dist:
 	@echo "——————————————————————————————————————————————————————————————————"
@@ -443,13 +444,15 @@
 	$(MAKE) -f $(MAKEFILE) -j$(nproc) clean
 	$(MAKE) -f $(MAKEFILE) -j$(nproc) all
 	$(MAKE) -f $(MAKEFILE) -j$(nproc) test
-	$(MAKE) -f $(MAKEFILE) -j$(nproc) dist_zip dist_docs
+	$(MAKE) -f $(MAKEFILE) -j$(nproc) dist_zip dist_zip_gf dist_docs
 	$(MAKE) -f $(MAKEFILE) dist_postflight
 
 dist_zip: | venv
-	@#. $(VENV) ; python misc/tools/patch-version.py misc/dist/inter.css
 	bash misc/makezip2.sh -reveal-in-finder "$(DIST_ZIP)"
 
+dist_zip_gf: | venv
+	cd "$(FONTDIR)/googlefonts" && zip -q -X -r "$(DIST_ZIP_GF)" *.ttf
+
 dist_docs:
 	$(MAKE) -C docs -j$(nproc) dist
 
@@ -468,7 +471,7 @@
 	@echo ""
 	@echo "——————————————————————————————————————————————————————————————————"
 
-.PHONY: dist dist_preflight dist_step1 dist_step2 dist_zip dist_docs dist_postflight
+.PHONY: dist dist_preflight dist_step1 dist_step2 dist_zip dist_zip_gf dist_docs dist_postflight

m4rc1e · 2024-03-26T14:44:28Z

Apologies for the delay. Thank you so much for the patch. I agree that a separate zip is better.

I've just implemented your feeback. I've pinned gftools, regenerated the lockfile and added your patch.

m4rc1e · 2024-03-26T14:45:59Z

I also don't mind deleting makezip.sh since it isn't used anywhere in the chain.

The team at GF still haven't decided whether to rename the family to "Inter 4" due to the amount of regressions it may cause. I should have an answer for you by Friday. Please don't merge the PR until then.

rsms · 2024-03-26T20:26:41Z

@m4rc1e Thank you for the update. I'll hold off on merging until I hear back from you.

Makefile

m4rc1e · 2024-05-15T09:16:06Z

Hey,

So sorry about the delay.

I went on a short break and Dave was off as well. The answer is in, we're not going to rename the family. I've also added . $(VENV) ; and updated gftools. I needed to update gftools because we changed an api.

rsms · 2024-05-15T15:28:54Z

Thank you for all your help @m4rc1e

m4rc1e · 2024-05-16T08:49:49Z

Thank you so much for the help and merge. Hate to be a bother but could you cut another release and include the gf zip? I can then onboard the family to Google Fonts.

rsms · 2024-05-23T22:31:38Z

@m4rc1e unfortunately the change of gftools caused and implicit upgrade of fontTools which in turn has a bug (or is broken or I'm using it wrong; most likely) that messes up the wght values in the VF.

$ make -j var
...
. build/venv/bin/activate ; python misc/tools/bake-vf.py build/fonts/var/.Inter-Roman.var.ttf -o build/fonts/var/InterVariable.ttf
unexpected wght 481.0 (expected 500) <fontTools.ttLib.ttFont.TTFont object at 0x101cc0e50> {'opsz': 14.0, 'wght': 481.0}
unexpected wght 562.0 (expected 600) <fontTools.ttLib.ttFont.TTFont object at 0x101cc0e50> {'opsz': 14.0, 'wght': 562.0}
unexpected wght 643.0 (expected 600) <fontTools.ttLib.ttFont.TTFont object at 0x101cc0e50> {'opsz': 14.0, 'wght': 643.0}
unexpected wght 758.0 (expected 800) <fontTools.ttLib.ttFont.TTFont object at 0x101cc0e50> {'opsz': 14.0, 'wght': 758.0}

I have almost no free time (between raising a newborn baby, running a start-up and sleeping a little bit in between) so I'm not sure when I'll get around to fixing the fontTools issues.

We can try reverting the change to Pipfile but I assume you had a good reason for upgrading it. Alt we could try to pin fontTools to a version that doesn't break the VF STAT table.

m4rc1e · 2024-05-24T07:50:51Z

Thank you very much for the update. I have a nine month old who's teething so my sleep is completely wrecked,. I have no problems looking into this issue and opening another PR. I'll keep you posted.

rsms · 2024-05-24T17:55:24Z

@m4rc1e I had a look this morning and I was wrong — the issue was introduced by an update to Glyphs app. I've got it working and will get you a zip in a moment (will post a link here)

rsms · 2024-05-24T18:08:21Z

@m4rc1e Since I'm not ready to do QA and publish Inter 4.1 yet, here's a GF build of 66647c0 which will be very similar—if not identical—to the final 4.1 release:

Inter-4.1-GoogleFonts.zip

(You can build your own locally like this: make clean && make -j dist_gf)

make: add googlefonts target

6a65452

m4rc1e mentioned this pull request Mar 19, 2024

Update Inter to latest version google/fonts#3429

Closed

rsms requested changes Mar 20, 2024

View reviewed changes

implement Rasmus feedback

ec3c759

rsms reviewed Mar 26, 2024

View reviewed changes

Makefile Outdated Show resolved Hide resolved

m4rc1e added 2 commits May 15, 2024 09:55

make: ensure venv is activated before running gftools

ad0c3d2

Upgrade gftools

9c5b1e0

rsms merged commit bde6fa1 into rsms:master May 15, 2024

shyakadavis mentioned this pull request Jun 7, 2024

Inter 4 fontsource/font-files#55

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make: add googlefonts target #678

make: add googlefonts target #678

m4rc1e commented Feb 6, 2024

m4rc1e commented Mar 14, 2024

rsms left a comment

rsms Mar 20, 2024

rsms Mar 20, 2024

mon-jai Mar 20, 2024

rsms Mar 20, 2024

mon-jai Mar 21, 2024 •

edited

Loading

m4rc1e Mar 21, 2024

rsms Mar 22, 2024

rsms commented Mar 22, 2024 •

edited

Loading

m4rc1e commented Mar 26, 2024

m4rc1e commented Mar 26, 2024 •

edited

Loading

rsms commented Mar 26, 2024

m4rc1e commented May 15, 2024

rsms commented May 15, 2024

m4rc1e commented May 16, 2024

rsms commented May 23, 2024

m4rc1e commented May 24, 2024

rsms commented May 24, 2024

rsms commented May 24, 2024

make: add googlefonts target #678

make: add googlefonts target #678

Conversation

m4rc1e commented Feb 6, 2024

m4rc1e commented Mar 14, 2024

rsms left a comment

Choose a reason for hiding this comment

rsms Mar 20, 2024

Choose a reason for hiding this comment

rsms Mar 20, 2024

Choose a reason for hiding this comment

mon-jai Mar 20, 2024

Choose a reason for hiding this comment

rsms Mar 20, 2024

Choose a reason for hiding this comment

mon-jai Mar 21, 2024 • edited Loading

Choose a reason for hiding this comment

m4rc1e Mar 21, 2024

Choose a reason for hiding this comment

rsms Mar 22, 2024

Choose a reason for hiding this comment

rsms commented Mar 22, 2024 • edited Loading

m4rc1e commented Mar 26, 2024

m4rc1e commented Mar 26, 2024 • edited Loading

rsms commented Mar 26, 2024

m4rc1e commented May 15, 2024

rsms commented May 15, 2024

m4rc1e commented May 16, 2024

rsms commented May 23, 2024

m4rc1e commented May 24, 2024

rsms commented May 24, 2024

rsms commented May 24, 2024

mon-jai Mar 21, 2024 •

edited

Loading

rsms commented Mar 22, 2024 •

edited

Loading

m4rc1e commented Mar 26, 2024 •

edited

Loading