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

noto-fonts-emoji: Package both color and monochrome variants #254149

Merged
merged 6 commits into from
Sep 12, 2023

Conversation

nbraud
Copy link
Contributor

@nbraud nbraud commented Sep 9, 2023

Description of changes

  • noto-fonts-emoji → noto-fonts-color-emoji
  • noto-fonts-monochrome-emoji: init at v46.2023-09-07

Closes #221181.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • 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/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • Fits CONTRIBUTING.md.

@nbraud
Copy link
Contributor Author

nbraud commented Sep 9, 2023

@mkg20001 since you already saw the sourcery involved.

Copy link
Contributor

@yorickvP yorickvP left a comment

Choose a reason for hiding this comment

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

Thanks for this! I added some feedback.

pkgs/data/fonts/noto-fonts/default.nix Outdated Show resolved Hide resolved
pkgs/data/fonts/noto-fonts/default.nix Outdated Show resolved Hide resolved
pkgs/data/fonts/noto-fonts/default.nix Outdated Show resolved Hide resolved
pkgs/data/fonts/noto-fonts/default.nix Outdated Show resolved Hide resolved
pkgs/data/fonts/noto-fonts/default.nix Outdated Show resolved Hide resolved
pkgs/top-level/aliases.nix Outdated Show resolved Hide resolved
@nbraud nbraud force-pushed the noto-emoji branch 5 times, most recently from 9095109 to dc83279 Compare September 9, 2023 13:26
@nbraud
Copy link
Contributor Author

nbraud commented Sep 9, 2023

I moved the file hashes to a second file (now in SRI format), and added a script to automate the update process, which:

  • fetches the metadata, if an API key is available (and otherwise uses the stale metadata in the JSON file) ;
  • downloads and hashes each new URL (this part does not require an API key) ;
  • atomically updates the JSON files with the metadata and hashes.

@nbraud nbraud force-pushed the noto-emoji branch 3 times, most recently from 1a3a05b to 9b115ff Compare September 9, 2023 15:15
@nbraud nbraud requested a review from yorickvP September 9, 2023 17:40
@nbraud nbraud force-pushed the noto-emoji branch 3 times, most recently from cbef467 to 752ea05 Compare September 9, 2023 18:02
@nbraud
Copy link
Contributor Author

nbraud commented Sep 9, 2023

Changed the version away from unstable-2023-09-07 as I realised upstream does assign proper version numbers, according to their own API.

@nbraud
Copy link
Contributor Author

nbraud commented Sep 9, 2023

It's kind of annoying we cannot set passthru.updateScript to ./noto-emoji.py as things stand. Is there a good reason we can't drop a Google Fonts API key in the repo?

Google's own documentation has an “API Explorer” which does exactly that: some API key is passed to browser-side JS code, though that specific key is bound to the right Referer (which wouldn't stop anyone from using it outside browsers)

@Mathnerd314
Copy link
Contributor

Is there a good reason we can't drop a Google Fonts API key in the repo?

We do have a Chromium key. It looks like @aszlig got permission for it many years ago. As mentioned in that commit, the public Google API TOS does forbid distributing API keys publicly (https://developers.google.com/terms: "Developer credentials may not be embedded in open source projects."), so a Fonts API key would similarly require special permission from Google. I'm not sure who you would reach out to now.

@aszlig
Copy link
Member

aszlig commented Sep 10, 2023

We do have a Chromium key. It looks like @aszlig got permission for it many years ago. As mentioned in that commit, the public Google API TOS does forbid distributing API keys publicly (https://developers.google.com/terms: "Developer credentials may not be embedded in open source projects."), so a Fonts API key would similarly require special permission from Google. I'm not sure who you would reach out to now.

Good luck... we're currently running into quota limits for these mentioned API keys (they're also used in Firefox btw.) and so far we were unable to reach anyone from Google who's in charge of these after multiple attempts. If you do for some reason find the right contact at Google, please let us know (cc: @mweinelt). The person mentioned in that referenced commit no longer works there.

@nbraud
Copy link
Contributor Author

nbraud commented Sep 10, 2023

Thanks @aszlig and @Mathnerd314, for all the information. I'll see what I can do. :3

@nbraud
Copy link
Contributor Author

nbraud commented Sep 10, 2023

The test failure seems unrelated: I did not touch maintainers/*, and team-list.nix fails to evaluate:

error: undefined variable 'ctheune'

at /var/lib/ofborg/checkout/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/ofborg-evaluator-7/maintainers/team-list.nix:300:7:

   299|     members = [
   300|       ctheune
      |       ^
   301|       dpausp

@mweinelt
Copy link
Member

Fixed in #254387. Please rebase.

Clarify that the monochrome font is not included, per NixOS#221181.

The new name is also coherent with the name of the font,
according to `fontconfig`: Noto Color Emoji.
This hedges against a buggy version of the script generating data
that is then blindly reused even after the bug is fixed.
@nbraud
Copy link
Contributor Author

nbraud commented Sep 12, 2023

Done

@mkg20001 mkg20001 merged commit 7163f12 into NixOS:master Sep 12, 2023
24 checks passed
@nbraud nbraud deleted the noto-emoji branch September 12, 2023 14:44
Guanran928 added a commit to Guanran928/nixpak that referenced this pull request Nov 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

noto-fonts-emoji: promises inexistent black-and-white version
7 participants