Skip to content

fontbakery: fix build failures#300400

Merged
mweinelt merged 13 commits intoNixOS:masterfrom
danc86:fix-fontbakery
Apr 7, 2024
Merged

fontbakery: fix build failures#300400
mweinelt merged 13 commits intoNixOS:masterfrom
danc86:fix-fontbakery

Conversation

@danc86
Copy link
Contributor

@danc86 danc86 commented Mar 31, 2024

Description of changes

The bulk update of Python packages in PR #294305 introduced build failures in fontbakery and several of its dependencies. This fixes the whole chain to get fontbakery building again.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

This package was failing to build because of some missing dependencies.

Upstream added a new dependency on PyYAML in v0.4.1:
googlefonts/shaperglot@94a9b30
and switched from poetry to setuptools in v0.4.2:
googlefonts/shaperglot@2c76ffa
@danc86 danc86 requested a review from mweinelt March 31, 2024 03:03
@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Mar 31, 2024
Copy link
Member

@mweinelt mweinelt left a comment

Choose a reason for hiding this comment

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

A few changes, that should be done across all packages touched.

Copy link
Member

Choose a reason for hiding this comment

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

If you want to leak this into the environment, change it to

Suggested change
gitTag = "v2024.02.05";
env.gitTag = "v2024.02.05";

Otherwise, I don't see any usage, please remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's used below in meta.changelog:

changelog = "https://github.com/googlefonts/nam-files/releases/tag/${gitTag}";

Unfortunately because of upstream's chosen versioning scheme, the git tag and PyPI versions don't match. I didn't want to try and craft some complicated expression to predict the git tag from the PyPI version or vice versa, I was intending to just use this variable here as a reminder to keep these two versions in sync with each other in future.

Admittedly, that means the normal Python package update script won't work correctly on this package. I'm not sure what the best solution is in this case.

Given that the release notes on Github contain very little useful information, maybe the simplest option is to just leave meta.changelog unspecified?

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Mar 31, 2024
@mweinelt
Copy link
Member

Result of nixpkgs-review pr 300400 run on x86_64-linux 1

4 packages failed to build:
  • python312Packages.fontbakery
  • python312Packages.fontbakery.dist
  • python312Packages.glyphsets
  • python312Packages.glyphsets.dist
16 packages built:
  • fontbakery (python311Packages.fontbakery)
  • fontbakery.dist (python311Packages.fontbakery.dist)
  • python311Packages.gflanguages
  • python311Packages.gflanguages.dist
  • python311Packages.gfsubsets
  • python311Packages.gfsubsets.dist
  • python311Packages.glyphsets
  • python311Packages.glyphsets.dist
  • shaperglot (python311Packages.shaperglot)
  • shaperglot.dist (python311Packages.shaperglot.dist)
  • python312Packages.gflanguages
  • python312Packages.gflanguages.dist
  • python312Packages.gfsubsets
  • python312Packages.gfsubsets.dist
  • python312Packages.shaperglot
  • python312Packages.shaperglot.dist

danc86 added 12 commits April 1, 2024 13:24
This reverts commit 8566364.

The 5.0.4 release was not a real release, it was a typo for 0.5.4 and
has since been yanked from PyPI.
The patch is no longer needed because upstream relaxed the version
range.
Upstream started depending on gflanguages and requests from v0.6.12:
googlefonts/glyphsets@ba341dc
googlefonts/glyphsets@8ca303d
This patch no longer applies cleanly, and upstream has already
completely refactored everything since the v0.11.2 release so my
updated PR won't apply cleanly either.

Just explicitly skip all tests requiring network access for now.
Hopefully in a later release, this list of tests to skip can be
shortened or removed.
@danc86
Copy link
Contributor Author

danc86 commented Apr 1, 2024

Thanks for the review. I've amended this PR to use pyproject = true; and the PEP-517 attributes for all four packages touched in the PR. I've also explicitly disabled tests for the gfsubsets package (there are indeed no unit tests) and added an import check, which helped me to find an undeclared dependency.

@danc86
Copy link
Contributor Author

danc86 commented Apr 1, 2024

Just for the record, the build failures reported by nixpkgs-review are on Python 3.12, due to an unrelated build failure in openstep-plist. It's failing on master also. The Python 3.11 builds are successful.

@danc86 danc86 requested a review from mweinelt April 7, 2024 05:44
@mweinelt mweinelt merged commit 80c0bfb into NixOS:master Apr 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: python Python is a high-level, general-purpose programming language. 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants