Skip to content

ttf_envy_code_r: init at preview7 #27165

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

Merged
merged 1 commit into from
Jul 8, 2017
Merged

Conversation

wheatdog
Copy link

@wheatdog wheatdog commented Jul 6, 2017

Motivation for this change

I want to use Envy Code R.

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
    • Linux
  • 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"
  • This package has no commands but fonts are stored in share/fonts/truetype
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

@mention-bot
Copy link

@wheatdog, thanks for your PR! By analyzing the history of the files in this pull request, we identified @zimbatm, @zraexy and @vcunat to be potential reviewers.

@vbgl
Copy link
Contributor

vbgl commented Jul 6, 2017

It seems that the license is not free. Below the download link, on can read:

These files are free to download and use from damieng.com but CAN NOT be redistributed either by other web sites or be included in your package, download, product or source repository at this time.

@wheatdog
Copy link
Author

wheatdog commented Jul 6, 2017

So, how about changing the license to unfree?

@0xABAB
Copy link
Contributor

0xABAB commented Jul 6, 2017

@wheatdog Best thing is to just ask the author of the package to change the wording of what he said, because it's rather vague.

@vcunat
Copy link
Member

vcunat commented Jul 6, 2017

I believe it's clear that the font is non-redistributable and therefore unfree. I think it can be included in nixpkgs, with licenses.unfree.

@jfrankenau
Copy link
Member

jfrankenau commented Jul 7, 2017

@wheatdog, if you squash those two commits, the PR will be ready to be merged.

However, I am not sure if the license allows redistributing this font, as Hydra will inevitably package it and provide a binary package. Or is there a way to deactivate it for this one package? Maybe by setting preferLocalBuild?

@0xABAB
Copy link
Contributor

0xABAB commented Jul 7, 2017

@jfrankenau That's what the unfree is supposed to be for. Or are you saying that despite it being marked as unfree that it would still be built?

@@ -13096,6 +13096,8 @@ with pkgs;

ttf_bitstream_vera = callPackage ../data/fonts/ttf-bitstream-vera { };

ttf_envy_code_r = callPackage ../data/fonts/ttf-envy-code-r {};
Copy link
Member

Choose a reason for hiding this comment

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

Any particular reason for underscores? Dashes are preferred nowadays: http://nixos.org/nixpkgs/manual/#sec-package-naming

Copy link
Author

Choose a reason for hiding this comment

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

Actually, I prefer dahes, too. I will fix it.

@jfrankenau
Copy link
Member

@0xABAB, ah I didn't know that setting unfree as a side effect would be prevent it from getting in the binary caches. Still so much to learn about NixOS. 😃

@vcunat
Copy link
Member

vcunat commented Jul 7, 2017

Some distros, e.g. one pseudo-fork, don't even allow unfree stuff into the main repo, typically for "ideological reasons" :-) If we distributed non-distributable packages, that could be quite a legal problem.

@jfrankenau
Copy link
Member

If we distributed non-distributable packages, that could be quite a legal problem.

That's why I wanted to make sure that this package is only built by the end user. And I realized that NixOS being source-based and only backed by binary caches is ideal for this kind of situation.

@wheatdog
Copy link
Author

wheatdog commented Jul 8, 2017

I am not sure how nixpkg handle unfree packages. Can someone point out some code or document that explaining the behavior when specifying licenses.unfree? Thanks 😄

@vcunat
Copy link
Member

vcunat commented Jul 8, 2017

@wheatdog: this, for example: http://nixos.org/nixpkgs/manual/#sec-allow-unfree

@wheatdog
Copy link
Author

wheatdog commented Jul 8, 2017

@vcunat: thanks, what about the concern @jfrankenau mentioned? Will it be cached in binary form (by Hydra, maybe)?

@vcunat
Copy link
Member

vcunat commented Jul 8, 2017

Unfree packages aren't touched by Hydra, of course. Not even the redistributable ones ATM.

@jfrankenau
Copy link
Member

@wheatdog, another section in the Nixpkgs manual implies this: https://nixos.org/nixpkgs/manual/#sec-meta-license

@vcunat
Copy link
Member

vcunat commented Jul 8, 2017

OK, I'd push this with some nitpicks atop: 4227b49. Any other ideas?

@wheatdog
Copy link
Author

wheatdog commented Jul 8, 2017

@vcunat: Looks great, but why do you change the location of readme? Will it collide? The reason I put it that way is that terminus-font-ttf seems do the same thing.

@vcunat
Copy link
Member

vcunat commented Jul 8, 2017

Yes, if two packages define the same path, they will collide when getting linked into the same environment.

@vcunat vcunat merged commit 208edec into NixOS:master Jul 8, 2017
vcunat added a commit that referenced this pull request Jul 8, 2017
vcunat added a commit that referenced this pull request Jul 8, 2017
(cherry picked from commit c057098)
It just adds a new package.
@vcunat
Copy link
Member

vcunat commented Jul 8, 2017

Merged to master and 17.03. I just switched unzip to nativeBuildInputs, which would only make a difference when cross-compiling...

@wheatdog wheatdog deleted the ttf-envy-code-r branch July 8, 2017 11:49
@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
adrianpk added a commit to adrianpk/nixpkgs that referenced this pull request May 31, 2024
(cherry picked from commit c057098)
It just adds a new package.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants