television: add wrapper helper

[RossSmyth]

Things done

1. makeWrapper -> makeBinaryWrapper
2. strictDeps = true
3. installManPage
4. installShellCompletion
5. Add "share" output
6. Wrap program to inject the "cable-dir" argument so that the user doesn't have to download the cables
7. Add withPkgs wrapper helper to bring television in line with other packages like python, hunspell, typst, and others.
8. Add test for the wrapper
9. Add a backwards compatible warning for the previous override-based package injection

• Built on platform:
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
• Ran nixpkgs-review on this PR. See nixpkgs-review usage.
• Tested basic functionality of all binary files, usually in ./result/bin/.
• Nixpkgs Release Notes
  • Package update: when the change is major or breaking.
• NixOS Release Notes
  • Module addition: when adding a new NixOS module.
  • Module update: when the change is significant.
• Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

---

Add a 👍 reaction to pull requests you find important.

[louis-thevenet]

I tested it in my NixOS config, the wrapping works and I get the cables without needing to download them. But the man pages and the shell completions don't seem to get installed. I made a naive suggestion but I'm not sure this is the real issue.

[louis-thevenet]

I would assume the completions to be in /nix/store/xxxx/share// but it seems to get install elsewhere due to the wrapping?

[getchoo]

I'm on board with something like this to avoid full rebuilds for simply adding external programs to $PATH (and it's something I've actually been putting off for a while...😆), but the current implementation has a good few problems I think we need to sort out

[RossSmyth]

I am poking at the checkPhase rn

[RossSmyth]

Ok my findings are that if it is desired for tests to run, probably a nixos vm test needs to be created. The start is:

  preCheck = ''
    substituteInPlace tests/common/mod.rs \
      --replace-fail '"./target/debug/tv"' '"./target/${stdenv.hostPlatform.rust.cargoShortTarget}/${finalAttrs.buildType}/tv"'
  '';

then there are many failures with os error 13 due to a pty not being available. And most tests require a pty.

So in conclusion, I think disabling tests right now is a good idea.

[SuperSandro2000]

Can you please rebase?

[RossSmyth]

Rebased, deleted the second commit that bumps the version since that what the rebase was against.

[getchoo]

I'm pretty happy with this! The only suggestion I would have is to maybe keep some backwards compatibility with something like this:

assert
  (extraPackages == null)
  || lib.warn "Overriding television with the 'extraPackages' attribute is deprecated. Please use `television.withPackages (p: [ p.fd ...])` instead." true;

let
  television = rustPlatform.buildRustPackage { ... };
in

if extraPackages == null then television else television.withPackages (lib.const extraPackages)

[louis-thevenet]

lgtm

[RossSmyth]

Alright the backwards compat is in