fetchtorrent: add flatten argument for rqbit (#432330)

doc/release-notes/rl-2511.section.md

@@ -50,6 +50,8 @@
 
 - `tooling-language-server` has been renamed to `deputy` (both the package and binary), following the rename of the upstream project.
 
+- `fetchtorrent`, when using the "rqbit" backend, erroneously started fetching files into a subdirectory in Nixpkgs 24.11.  The original behaviour – which matches the behaviour using the "transmission" backend – has now been restored.  Users reliant on the erroneous behaviour can temporarily maintain it by adding `flatten = false` to the `fetchtorrent` arguments; Nix will produce an evaluation warning for anyone using `backend = "rqbit"` without `flatten = true`.
+
 - `webfontkitgenerator` has been renamed to `webfont-bundler`, following the rename of the upstream project.
   The binary name remains `webfontkitgenerator`.
   The `webfontkitgenerator` package is an alias to `webfont-bundler`.

pkgs/build-support/fetchtorrent/default.nix

@@ -18,20 +18,20 @@ in
       "bittorrent"
     else
       "bittorrent-" + builtins.head (builtins.match urlRegexp url),
-  config ?
+  config ? { },
-    if (backend == "transmission") then
-      { }
-    else
-      throw "json config for configuring fetchFromBitorrent only works with the transmission backend",
   hash,
   backend ? "transmission",
   recursiveHash ? true,
+  flatten ? null,
   postFetch ? "",
   postUnpack ? "",
   meta ? { },
 }:
 let
-  afterSuccess = writeShellScript "fetch-bittorrent-done.sh" ''
+  # Default to flattening if no flatten argument was specified.
+  flatten' = if flatten == null then true else flatten;
+
+  transmissionFinishScript = writeShellScript "fetch-bittorrent-done.sh" ''
     ${postUnpack}
     # Flatten the directory, so that only the torrent contents are in $out, not
     # the folder name
@@ -43,7 +43,52 @@ let
     kill $PPID
   '';
   jsonConfig = (formats.json { }).generate "jsonConfig" config;
+
+  # https://github.com/NixOS/nixpkgs/issues/432001
+  #
+  # For a while, the transmission backend would put the downloaded torrent in
+  # the output directory, but whether the rqbit backend would put the output in
+  # the output directory or a subdirectory depended on the version of rqbit.
+  # We want to standardise on a single behaviour, but give users of
+  # fetchtorrent with the rqbit backend some warning that the behaviour might
+  # be unexpected, particularly since we can't know what behaviour users might
+  # be expecting at this point, and they probably wouldn't notice a change
+  # straight away because the results are fixed-output derivations.
+  #
+  # This warning was introduced for 25.11, so we can remove handling of the
+  # `flatten` argument once that release is no longer supported.
+  warnings =
+    if backend == "rqbit" && flatten == null then
+      [
+        ''
+          `fetchtorrent` with the rqbit backend may or may not have the
+          downloaded files stored in a subdirectory of the output directory.
+          Verify which behaviour you need, and set the `flatten` argument to
+          `fetchtorrent` accordingly.
+
+          The `flatten = false` behaviour will still produce a warning, as this
+          behaviour is deprecated.  It is only available with the "rqbit" backend
+          to provide temporary support for users who are relying on the
+          previous incorrect behaviour.  For a warning-free evaluation, use
+          `flatten = true`.
+        ''
+      ]
+    else if flatten == false then
+      [
+        ''
+          `fetchtorrent` with `flatten = false` is deprecated and will be
+          removed in a future release.
+        ''
+      ]
+    else
+      [ ];
 in
+assert lib.assertMsg (config != { } -> backend == "transmission") ''
+  json config for configuring fetchtorrent only works with the transmission backend
+'';
+assert lib.assertMsg (backend == "transmission" -> flatten') ''
+  `flatten = false` is only supported by the rqbit backend for fetchtorrent
+'';
 runCommand name
   {
     inherit meta;
@@ -75,17 +120,54 @@ runCommand name
         mkdir -p $downloadedDirectory
         mkdir -p $HOME/.config/transmission
         cp ${jsonConfig} $HOME/.config/transmission/settings.json
+        port="$(shuf -n 1 -i 49152-65535)"
         function handleChild {
           # This detects failures and logs the contents of the transmission fetch
           find $out
           exit 0
         }
         trap handleChild CHLD
-        transmission-cli --port $(shuf -n 1 -i 49152-65535) --portmap --finish ${afterSuccess} --download-dir $downloadedDirectory --config-dir "$HOME"/.config/transmission "$url"
+        transmission-cli \
+            --port "$port" \
+            --portmap \
+            --finish ${transmissionFinishScript} \
+            --download-dir "$downloadedDirectory" \
+            --config-dir "$HOME"/.config/transmission \
+            "$url"
       ''
     else
       ''
         export HOME=$TMP
-        rqbit --disable-dht-persistence --http-api-listen-addr "127.0.0.1:$(shuf -n 1 -i 49152-65535)" download -o $out --exit-on-finish "$url"
+      ''
+      + lib.optionalString flatten' ''
+        downloadedDirectory=$out/downloadedDirectory
+        mkdir -p $downloadedDirectory
+      ''
+      + lib.optionalString (!flatten') ''
+        downloadedDirectory=$out
+      ''
+      + ''
+        port="$(shuf -n 1 -i 49152-65535)"
+
+        rqbit \
+            --disable-dht-persistence \
+            --http-api-listen-addr "127.0.0.1:$port" \
+            download \
+            -o "$downloadedDirectory" \
+            --exit-on-finish \
+            "$url"
+
+        ${postUnpack}
+      ''
+      + lib.optionalString flatten' ''
+        # Flatten the directory, so that only the torrent contents are in $out,
+        # not the folder name
+        shopt -s dotglob
+        mv -v $downloadedDirectory/*/* $out
+        rm -v -rf $downloadedDirectory
+        unset downloadedDirectory
+      ''
+      + ''
+        ${postFetch}
       ''
   )

pkgs/build-support/fetchtorrent/tests.nix

@@ -18,41 +18,74 @@ let
   };
 
   # Via https://webtorrent.io/free-torrents
-  httpUrl = "https://webtorrent.io/torrents/sintel.torrent";
+  http.url = "https://webtorrent.io/torrents/sintel.torrent";
-  magnetUrl = "magnet:?xt=urn:btih:08ada5a7a6183aae1e09d831df6748d566095a10&dn=Sintel&tr=udp%3A%2F%2Fexplodie.org%3A6969&tr=udp%3A%2F%2Ftracker.coppersurfer.tk%3A6969&tr=udp%3A%2F%2Ftracker.empire-js.us%3A1337&tr=udp%3A%2F%2Ftracker.leechers-paradise.org%3A6969&tr=udp%3A%2F%2Ftracker.opentrackr.org%3A1337&tr=wss%3A%2F%2Ftracker.btorrent.xyz&tr=wss%3A%2F%2Ftracker.fastcast.nz&tr=wss%3A%2F%2Ftracker.openwebtorrent.com&ws=https%3A%2F%2Fwebtorrent.io%2Ftorrents%2F&xs=https%3A%2F%2Fwebtorrent.io%2Ftorrents%2Fsintel.torrent";
+  magnet.url = "magnet:?xt=urn:btih:08ada5a7a6183aae1e09d831df6748d566095a10&dn=Sintel&tr=udp%3A%2F%2Fexplodie.org%3A6969&tr=udp%3A%2F%2Ftracker.coppersurfer.tk%3A6969&tr=udp%3A%2F%2Ftracker.empire-js.us%3A1337&tr=udp%3A%2F%2Ftracker.leechers-paradise.org%3A6969&tr=udp%3A%2F%2Ftracker.opentrackr.org%3A1337&tr=wss%3A%2F%2Ftracker.btorrent.xyz&tr=wss%3A%2F%2Ftracker.fastcast.nz&tr=wss%3A%2F%2Ftracker.openwebtorrent.com&ws=https%3A%2F%2Fwebtorrent.io%2Ftorrents%2F&xs=https%3A%2F%2Fwebtorrent.io%2Ftorrents%2Fsintel.torrent";
 
-  # All routes to download the torrent should produce the same output and
+  flattened.hash = "sha256-EzbmBiTEWOlFUNaV5R4eDeD9EBbp6d93rfby88ACg0s=";
-  # therefore have the same FOD hash.
+  unflattened.hash = "sha256-lVrlo1AwmFcxwsIsY976VYqb3hAprFH1xWYdmlTuw0U=";
-  hash = "sha256-EzbmBiTEWOlFUNaV5R4eDeD9EBbp6d93rfby88ACg0s=";
 in
-
+# Seems almost but not quite worth using lib.mapCartesianProduct...
-{
+builtins.mapAttrs (n: v: testers.invalidateFetcherByDrvHash fetchtorrent v) {
-  http-link = testers.invalidateFetcherByDrvHash fetchtorrent {
+  http-link = {
-    inherit hash;
+    inherit (http) url;
-    url = httpUrl;
+    inherit (flattened) hash;
-    backend = "transmission";
     inherit (sintel) meta;
   };
-  magnet-link = testers.invalidateFetcherByDrvHash fetchtorrent {
+  http-link-transmission = {
-    inherit hash;
+    inherit (http) url;
-    url = magnetUrl;
     backend = "transmission";
+    inherit (flattened) hash;
     inherit (sintel) meta;
   };
-  http-link-rqbit = testers.invalidateFetcherByDrvHash fetchtorrent {
+  magnet-link = {
-    inherit hash;
+    inherit (magnet) url;
-    url = httpUrl;
+    inherit (flattened) hash;
+    inherit (sintel) meta;
+  };
+  magnet-link-transmission = {
+    inherit (magnet) url;
+    backend = "transmission";
+    inherit (flattened) hash;
+    inherit (sintel) meta;
+  };
+  http-link-rqbit = {
+    inherit (http) url;
     backend = "rqbit";
-    meta = sintel.meta // {
+    inherit (flattened) hash;
-      broken = true;
+    inherit (sintel) meta;
   };
-  };
+  magnet-link-rqbit = {
-  magnet-link-rqbit = testers.invalidateFetcherByDrvHash fetchtorrent {
+    inherit (magnet) url;
-    inherit hash;
-    url = magnetUrl;
     backend = "rqbit";
-    meta = sintel.meta // {
+    inherit (flattened) hash;
-      broken = true;
+    inherit (sintel) meta;
   };
+  http-link-rqbit-flattened = {
+    inherit (http) url;
+    backend = "rqbit";
+    flatten = true;
+    inherit (flattened) hash;
+    inherit (sintel) meta;
+  };
+  magnet-link-rqbit-flattened = {
+    inherit (magnet) url;
+    backend = "rqbit";
+    flatten = true;
+    inherit (flattened) hash;
+    inherit (sintel) meta;
+  };
+  http-link-rqbit-unflattened = {
+    inherit (http) url;
+    backend = "rqbit";
+    flatten = false;
+    inherit (unflattened) hash;
+    inherit (sintel) meta;
+  };
+  magnet-link-rqbit-unflattened = {
+    inherit (magnet) url;
+    backend = "rqbit";
+    flatten = false;
+    inherit (unflattened) hash;
+    inherit (sintel) meta;
   };
 }