line-awesome: init at 1.3.0 #83537
line-awesome: init at 1.3.0 #83537
Conversation
|
|
||
| postFetch = '' | ||
| mkdir -p $out/share/fonts | ||
| unzip -j $downloadedFile ${version}/fonts/\*.ttf -d $out/share/fonts/truetype |
There was a problem hiding this comment.
Why is this necessary? Shouldn't fetchzip take care of the unpacking?
There was a problem hiding this comment.
Thanks for reviewing!
Hm. I modeled the PR after #78409 and other fonts do the same thing, e.g. yanone-kaffeesatz or ubuntu-fonts. But that doesn't mean it's correct 🤔
I think this overrides the postFetch defined by fetchzip, which is where the unpacking takes place. So it has to be done manually. But I concur, it's weird.
There was a problem hiding this comment.
Yeah, it seems very weird. If we're unzipping manually anyway, we might as well use fetchurl.
Maybe this should use mkDerivation instead, disable the unnecessary phases (mostly build) and do the installation in installPhase. I don't have much experience with fonts in nixpkgs though. @marsam, what are your thoughts on this?
There was a problem hiding this comment.
(Its possible I won't be able to react to emails over the rest of the weekend. So if someone else wants to merge, no need to wait for my approval)
There was a problem hiding this comment.
@puzzlewolf according to the docs, you should just add unzip to nativeBuildInputs, see: https://nixos.org/nixpkgs/manual/#ssec-unpack-phase .
There was a problem hiding this comment.
@doronbehar Do you want me to model this after e.g. nafees? It's quite different from e.g. ubuntu-fonts, it uses mkDerivation instead of fetchzip. Do you know of a community-agreed default way of doing this kind of font packages?
There was a problem hiding this comment.
I can see that we have many fonts installed with fetchzip which is a bit unfortunate since this has the disadvantage that it makes it impossible to override these derivations with <font>.overrideAttrs. nafees is written great so yes, please.
There was a problem hiding this comment.
Done, now it uses mkDerivation.
|
|
||
| postFetch = '' | ||
| mkdir -p $out/share/fonts | ||
| unzip -j $downloadedFile ${version}/fonts/\*.ttf -d $out/share/fonts/truetype |
There was a problem hiding this comment.
@puzzlewolf according to the docs, you should just add unzip to nativeBuildInputs, see: https://nixos.org/nixpkgs/manual/#ssec-unpack-phase .
puzzlewolf
force-pushed
the
line-awesome
branch
2 times, most recently
from
c7ccf1f
to
4b06f32
Compare
|
And now it's fixed-output. It should be, right? The manual seems pretty clear: https://nixos.org/nix/manual/#sec-advanced-attributes, but fixed-output derivations seem to be their own can of worms NixOS/nix#2270. |
puzzlewolf
force-pushed
the
line-awesome
branch
from
4b06f32
to
8930a67
Compare
|
If there's anything else you would like me to take a look at (a PR of yours or one you have reviewed), let me know. No guarantees of course. |
|
Thanks for reviewing, explaining and your receptiveness on discourse 🚀. Currently, there is only #83028, which should probably also go the I wonder if we should document somewhere in the manual that |
|
Yes, I think #83028 could be modeled after this PR. Documentation would be great! Are you up to writing that? I think the ´fetchX` functions are pretty much just intended to be used as part of a package, but never as the top-level package. If you feel up to it, you could write some documentation. We can then look for some people who have contributed |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/guidelines-on-packaging-fonts/7683/1 |
Motivation for this change
Line Awesome is a free alternative to Font Awesome, in line icon style.
https://icons8.com/line-awesome/
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)