rl2009.xml: Mention wrapMpv's new interface

[doronbehar]

Motivation for this change

#88620 (comment) cc @Profpatsch .

Things done

I haven't tested the build of the docs and how they look like...

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• 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 nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

[Profpatsch]

Can we change the abort in mpv-with-scripts to a deprecation warning plus a rewrite to the new module?

[doronbehar]

I won't mind, but I can't find an example I can follow. I mean, I could do this:

diff --git i/pkgs/top-level/aliases.nix w/pkgs/top-level/aliases.nix
index ff405131074..f084ab534ba 100644
--- i/pkgs/top-level/aliases.nix
+++ w/pkgs/top-level/aliases.nix
@@ -306,7 +306,9 @@ mapAliases ({
   msf = metasploit; # added 2018-04-25
   libmsgpack = msgpack; # added 2018-08-17
   mssys = ms-sys; # added 2015-12-13
-  mpv-with-scripts = throw "Use wrapMpv for editing the environment of mpv"; # added 2012-05-22
+  mpv-with-scripts = lib.warn "Use wrapMpv for editing the environment of mpv" (
+    super.wrapMpv super.mpv-unwrapped { }
+  ); # added 2020-05-22
   multipath_tools = multipath-tools;  # added 2016-01-21
   mupen64plus1_5 = mupen64plus; # added 2016-02-12
   mysqlWorkbench = mysql-workbench; # added 2017-01-19

But according to my tests, overlays don't apply to it. If we can't find a way to make overlays apply as well, I think this behavior is worse then a harsh throw error. It might take longer for a user to debug "why mpv behaves as if it didn't load my script" vs to notice and deal with a throw error. Because, the user will run mpv most likely only a while after their update, and a warning might get away from their attention.

[Profpatsch]

diff --git a/pkgs/top-level/aliases.nix b/pkgs/top-level/aliases.nix
index 03b03ff2a84..b647face390 100644
--- a/pkgs/top-level/aliases.nix
+++ b/pkgs/top-level/aliases.nix
@@ -309,7 +309,9 @@ mapAliases ({
   msf = metasploit; # added 2018-04-25
   libmsgpack = msgpack; # added 2018-08-17
   mssys = ms-sys; # added 2015-12-13
-  mpv-with-scripts = throw "Use wrapMpv for editing the environment of mpv"; # added 2012-05-22
+  mpv-with-scripts = lib.warn "Use wrapMpv for editing the environment of mpv" (
+    self.wrapMpv self.mpv-unwrapped { }
+  ); # added 2020-05-22
   multipath_tools = multipath-tools;  # added 2016-01-21
   mupen64plus1_5 = mupen64plus; # added 2016-02-12
   mysqlWorkbench = mysql-workbench; # added 2017-01-19

works just fine for me

[doronbehar]

@Profpatsch if you'll apply that diff and run:

nix-build -A mpv-with-scripts

You should be able to get a working mpv wrapper. But if you will add this overlay to your nixpkgs' config:

self: super:

{
  test-mpv-with-scripts = super.mpv-with-scripts.override {
    scripts = [ super.mpvScripts.mpris ];
  };
}

And run:

nix-build -A test-mpv-with-scripts

You should be able to see that:

cat result/bin/mpv | grep mpris.so

is empty - meaning the .override wasn't applied.

[Profpatsch]

Note that I switched from `super.wrapMpv` to `self.wrapMpv` in my example above. It applied the override script just fine here. To be perfectly clear, I don’t think it’s optional that we provide a backwards-compatible `mpv-with-scripts` symbol. We can’t just make breaking changes like these if there is a viable migration path.

…

On Wed, Jun 10, 2020 at 12:55 PM Doron Behar ***@***.***> wrote: @Profpatsch <https://github.com/Profpatsch> if you'll apply that diff and run: nix-build -A mpv-with-scripts You should be able to get a working mpv wrapper. But if you will add this overlay to your nixpkgs' config: self: super: { test-mpv-with-scripts = super.mpv-with-scripts.override { scripts = [ super.mpvScripts.mpris ]; }; } And run: nix-build -A test-mpv-with-scripts You should be able to see that: cat result/bin/mpv | grep mpris.so is empty - meaning the .override wasn't applied. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#89208 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAYB5ZVOPNDQBBUWCH7SM23RV5RB3ANCNFSM4NOUK23Q> .

[doronbehar]

Note that I switched from super.wrapMpv to self.wrapMpv in my example
above. It applied the override script just fine here.

Didn't notice. nice 👍.

I've updated the PR to include your diff and my release notes changes rebased on latest master.

[Profpatsch]

LGTM

[adisbladis]

This broke ofborg eval, fixed in 9de29f6.

[dotlambda]

This broke umpv: #90549