transmission: sent a warning and alias it to transmission_3

[doronbehar]

Motivation for this change

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 22.05 Release Notes (or backporting 21.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[jtojnar]

This looks like transmission: rename from transmission_4. And Transmission 4 contains at least one regression causing data loss, see the original PR: #215316 (comment)

[doronbehar]

Transmission 4 contains at least one regression causing data loss

Why do you consider it as "data loss"? It's not files that were already downloaded are deleted from your storage, but rather it is the "torrent configuration" loss. I personally don't think it is such a bad bug to live with.

[jtojnar]

Yes, it loses torrent configuration in a way that downgrading does not restore. This is quite painful if you have large amount of affected torrents that you have configured. Thankfully, I had backups or I would be very annoyed.

[doronbehar]

Hmm I understand... Your usage of transmission is unique IMO. Perhaps we can add some release notes, with links to upstream bug reports? My personal opinion is that unless upstream released a completely broken package, a distribution shouldn't hold such significant updates from all users just because some users experience a bug that can barely be reproduced... I wasn't aware of this upstream release if I hadn't noticed it by chance by browsing Nixpkgs' pull requests.

BTW I don't understand what is the ofborg-eval-darwin error :/.

[doronbehar]

I'm attempting to fix the darwin build of transmission 4. I pushed several other improvements to both expressions, along with a release-notes line for the possible upstream bug experience.

[doronbehar]

@ofborg build transmission

[doronbehar]

The darwin build seems fixed.

[ThinkChaos]

I've been using 4.0.3 for a while, and lately ran into an issue where data was being downloaded, but not saved to disk. The torrents showed multiple GBs downloaded but still 0% progress. This was happening with all downloading torrents, even newly added ones, and was fixed by restarting Transmission.
I haven't reported it upstream since 4.0.4 is out now and I'm not using it yet.

So even though I'm using v4 I'm against making it the default as in my experience it's not yet as stable as v3.

Your goal seems to be to raise awareness about v4. I can think of a couple ways to do that without risking people's data:

• add a release note mentioning v4 is packaged
• deprecate transmission and push users towards transmission_3 or transsmission_4, similar to Python 2/3.

In both cases, we should explain why v4 isn't the default and link the PRs/upstream bug so users can make an informed decision.

[doronbehar]

Thanks @ThinkChaos for the ideas. I like them overall. As for:

• deprecate transmission and push users towards transmission_3 or transsmission_4, similar to Python 2/3.

Were you thinking of something like this in aliases.nix:

transmission = throw "Please instead of attribute `transmission`, use `transmission_3` or `transmission_4`";

vs:

transmission = lib.warn "The transmission attribute without version suffix is deprecated, please switch over to either transmission_4 or transmission_3. Note the release-notes mentioning upstream bugs." transmission_3;

Or simply (which would not make a difference for most users, unless aliases are disabled in their nixpkgs.config):

transmission = transmission_3;

I'd appreciate your vote on either of these options. Moreover, if you'd report that with 4.0.4, you don't experience the buggy behavior you observed with 4.0.3, that should affect this decision.

[kirillrdy]

Thanks @ThinkChaos for the ideas. I like them overall. As for:

• deprecate transmission and push users towards transmission_3 or transsmission_4, similar to Python 2/3.

Were you thinking of something like this in aliases.nix:

transmission = throw "Please instead of attribute `transmission`, use `transmission_3` or `transmission_4`";

vs:

transmission = lib.warn "The transmission attribute without version suffix is deprecated, please switch over to either transmission_4 or transmission_3. Note the release-notes mentioning upstream bugs." transmission_3;

Or simply (which would not make a difference for most users, unless aliases are disabled in their nixpkgs.config):

transmission = transmission_3;

I'd appreciate your vote on either of these options. Moreover, if you'd report that with 4.0.4, you don't experience the buggy behavior you observed with 4.0.3, that should affect this decision.

sorry for noise
but what does

transmission = lib.warn evaluate to ? null ? eg what is output when you try to build it ?

[doronbehar]

transmission = lib.warn evaluate to ? null ? eg what is output when you try to build it ?

See: https://noogle.dev/?selected=%22lib.trivial.warn%22&term=%22warn%22

[kirillrdy]

transmission = lib.warn evaluate to ? null ? eg what is output when you try to build it ?

See: https://noogle.dev/?selected=%22lib.trivial.warn%22&term=%22warn%22

@doronbehar thank you for you response
@ThinkChaos I like your suggestion, I would be leaning towards throw but that's just my opinion

[jtojnar]

I wasn't aware of this upstream release if I hadn't noticed it by chance by browsing Nixpkgs' pull requests.

I would prefer throwing over a silent migration to a broken version but I do not think there is much of a point. Nixpkgs is method for distributing software (which needs to be working to be useful) before being a mechanism for announcing latest versions of upstream projects. Also, eventually, we are going to remove transmission_3 and then we would probably want to go back to using transmission attribute.

Perhaps we can add some release notes, with links to upstream bug reports?

I do not think many people running unstable read release notes. Personally, I skim the commit log but it is really easy to miss something important there. As such, I would not consider a mention in release notes to be a sufficient precaution for bumping the transmission attribute to a version that eats data.

There is a section for announcing new versions and Transmission 4 definitely deserves a mention (with a warning) but that just would not be enough for the bump IMO.

My personal opinion is that unless upstream released a completely broken package, a distribution shouldn't hold such significant updates from all users just because some users experience a bug that can barely be reproduced...

I would consider any data loss a serious breakage. I would not be against bumping the attribute if it was just the feature that was broken but it clears the torrent status so it is not possible to fix by downgrading once user ran 4.0. And the bug can be reproduced easily – just because it is a niche feature that does not affect many torrent files does not mean it does not seriously affect many peopleʼs workflows.

[ThinkChaos]

I would personally lean more towards warn for backward-compatibility reasons, and since it's similar to the established patern for module options (for instance mkRenamedOptionModule).
That would also allow you to backport the change and reach people using stable.

I do not think many people running unstable read release notes

I think you're right, personally I've started diffing the next release notes md file when updating my flake, works great :)

I would not consider a mention in release notes to be a sufficient precaution for bumping the transmission attribute to a version that eats data.

I agree, I meant just as a way to say "_4 exists, use it if you want".

[doronbehar]

I opened a PR that does everything done in this file besides the rename, so that the discussion here will stay focused: #259361 . Your review is welcome.

There is a section for announcing new versions and Transmission 4 definitely deserves a mention (with a warning) but that just would not be enough for the bump IMO.

@jtojnar I feel we are in almost full agreement. Could you perhaps voice perhaps a more concrete opinion on the options suggested in my previous comment above?

[bb2020]

I decided to change this PR a bit after the discussion on discourse and these changes maybe don't fit now.

I see 4.00 still has some issues.

Perhaps you can submit a PR where the new settings you introduce change their default value based on the version of cfg.package?

I think you should still cherry pick my changes.
They are the defaults for older version and would minimize confusion if someone switches between V3 and V4.

[jtojnar]

I see 4.00 still has some issues.

Maybe we should create a thread in Discourse Announcements category like the following, and link it in the error message:

https://pad.lassul.us/lHdWr37LRxS-8gEHYWvfuA?both

(Feel free to edit)

They are the defaults for older version and would minimize confusion if someone switches between V3 and V4.

I think this is not our responsibility. And doing so might instead cause confusion for people switching from other distribution to NixOS. At most, we can mention it in the announcement.

[AndersonTorres]

It looks like a nice use case for RFC 127 problem infrastructure!

https://github.com/NixOS/rfcs/blob/master/rfcs%2F0127-issues-warnings.md

[jtojnar]

There is another reference to pkgs.transmission in nixosTests.transmission:

nixpkgs/nixos/tests/all-tests.nix

Line 980 in c94ae9b

transmission = handleTest ./transmission.nix { transmission = pkgs.transmission; };

[doronbehar]

Thanks for the ideas @jtojnar . I added both a notice to the upcoming release notes, based on the text you wrote here.

They are the defaults for older version and would minimize confusion if someone switches between V3 and V4.

I think this is not our responsibility. And doing so might instead cause confusion for people switching from other distribution to NixOS. At most, we can mention it in the announcement.

I agree. I'll write a comment at the discourse thread when this is merged.

resolved

[ThinkChaos]

I ran into another v4 bug that's technically data loss, sharing here for awareness: Downloaded data not saved, might be nice to also add to the release notes.

[doronbehar]

I ran into another v4 bug that's technically data loss, sharing here for awareness: Downloaded data not saved, might be nice to also add to the release notes.

Linked to it in the release notes 👍.

[doronbehar]

So.. @jtojnar do I get your final approval on this change?

[doronbehar]

Once again I fixed the merge conflicts. Since the last revision of this PR, I got only approvals and hadn't seen any objections to it. Hence I'll merge it myself in a day or two.