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.

Sign up for GitHub

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

Jump to bottom

noto fonts and tools: update to newer tagged releases #31543

Merged
merged 2 commits into from
Nov 22, 2017
Merged

noto fonts and tools: update to newer tagged releases #31543

merged 2 commits into from
Nov 22, 2017

Conversation

benley
Copy link
Member

@benley benley commented Nov 11, 2017

Motivation for this change

Noto emoji had a major update in 2017.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@benley benley requested a review from FRidh as a code owner November 11, 2017 22:50
@benley benley added the 8.has: package (update) This PR updates a package to a newer version label Nov 11, 2017
FRidh
FRidh reviewed Nov 12, 2017
View reviewed changes
pkgs/top-level/python-packages.nix Outdated
@@ -10985,14 +10985,14 @@ in {
};

nototools = buildPythonPackage rec {
Copy link
Member

Choose a reason for hiding this comment

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

We are moving Python expressions out of pkgs/top-level/python-packages.nix into pkgs/development/python-modules/<module>/default.nix.

Please move the expression there, and call it from pkgs/top-level/python-packages.nix using callPackage ../development/python-modules/<package> { };.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done - actually not to that specific location because it's not a library, it's a set of scripts used to assemble one specific font family - but done.

@GrahamcOfBorg GrahamcOfBorg added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Nov 18, 2017
@benley benley merged commit a1e340f into NixOS:master Nov 22, 2017
@benley benley deleted the noto-fonts-2017 branch November 22, 2017 05:19
Mic92 added a commit that referenced this pull request Nov 22, 2017
@Mic92
pythonPackages.nototools: fix name & evaluation
1476962 
nototools was renamed by accident to notoTools in #31543
This broke evaluation of noto-fonts.

cc @benley
@edolstra
Copy link
Member

It appears that this PR increased the size of the noto-fonts closure from 25 MiB to 302 MiB. Since noto-fonts is included by the NixOS plasma module by default, we should either revert this PR or remove the noto-fonts dependency.

Ping @ttuegel who originally added the dependency on noto-fonts in .285061d50c1f0216a420a811d812e31ad4fe88a7.

@Mic92
Copy link
Member

Mic92 commented May 27, 2018

Also related to #31971

@Mic92
Copy link
Member

Mic92 commented May 27, 2018

Looking at the package it seems to come with different language variants:
i.e.; NotoSansHebrew-CondensedMedium.ttf

I don't think we need all of them.

@Mic92
Copy link
Member

Mic92 commented May 27, 2018

The debian package is much smaller: https://packages.debian.org/sid/fonts-noto-hinted

@ttuegel
Copy link
Member

ttuegel commented May 28, 2018

It appears that this PR increased the size of the noto-fonts closure from 25 MiB to 302 MiB.

I agree that this is not going to work. We should see what Debian is doing differently, because KDE upstream would like us to supply all weights and styles of Noto for Plasma.

@ttuegel
Copy link
Member

ttuegel commented May 28, 2018

Debian's package is smaller because they are not including extra weights or unhinted fonts. Originally, Noto only offered the weights {Regular,Bold,Light,Italic,BoldItalic,LightItalic} (46MB), but now we have {,Extra,Semi}Condensed also (260MB). @benley Would it be alright with you if I limited noto-fonts to just the "ordinary" weights and put the extra weights in noto-fonts-extra? I think it would be OK to include the unhinted fonts for better Unicode coverage at the expense of the extra 20MB, but I think the extra weights are prohibitively large for the default configuration.

@benley
Copy link
Member Author

benley commented May 28, 2018

That sounds reasonable to me. My goal with #31543 was mostly just to get multicolor emoji glyphs working, and I guess we picked up a whole lot more font variants as a side effect.

ttuegel added a commit to ttuegel/nixpkgs that referenced this pull request May 28, 2018
@ttuegel
noto-fonts: separate package for unusual weights
8f43bb4 
Noto provides a large number of unusual font weights so that the complete
package of fonts is more than 300 MB in size. Most of these weights are moved
into noto-fonts-extra; noto-fonts retains the ordinary weights and styles.

See also: NixOS#31543
@ttuegel ttuegel mentioned this pull request May 28, 2018
Merged
8 tasks
@Mathnerd314 Mathnerd314 mentioned this pull request Apr 16, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FRidh FRidh FRidh left review comments

Assignees
No one assigned
Labels
8.has: package (update) This PR updates a package to a newer version 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-darwin: 1-10 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@benley @edolstra @Mic92 @ttuegel @FRidh @GrahamcOfBorg