Skip to content
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

Thoughts on isInOldestRelease #291939

Open
infinisil opened this issue Feb 27, 2024 · 5 comments
Open

Thoughts on isInOldestRelease #291939

infinisil opened this issue Feb 27, 2024 · 5 comments

Comments

@infinisil
Copy link
Member

This isInOldestRelease function is super confusing, but I'm pretty sure it's meant to be used when you introduce a new feature (substitutions here) to deprecate a feature (replacements here).

So the "is in oldest release" terminology refers to the new feature being in the oldest supported release.

If the warning was unconditional, the problem is that third-party projects that aim to support all stable NixOS releases will struggle during the 1 month period in which there's two stable releases, because:

  • The new feature isn't usable because the previous release didn't support it
  • The old feature would always give a warning and users would complain about this

So the third-party project could either:

  • Ignore the user complaints about the warning until the previous release isn't supported anymore anyways
  • Implement some logic to use the new feature conditionally on the new release. But this seems barely worth it for that 1 month period.

By using isInOldestRelease here, there's no warning for the old feature during the 1 month period where both releases are stable. Therefore no users will be upset.

Though this does push the problem around a bit, because as soon as the previous release isn't supported anymore, the warning will start appearing. So in order to actually avoid warnings for users, the third-party project has to synchronise releases to NixOS, which doesn't seem ideal.

It might be better to have a version of isInOldestRelease that is delayed by one more release, such that third-party projects have an entire release cycle to start using the new feature.

Originally posted by @infinisil in #287364 (comment)

Ping @roberth

@roberth
Copy link
Member

roberth commented Feb 27, 2024

You're suggesting a slower deprecation cycle of up to 14 months.

in order to actually avoid warnings for users, the third-party project has to synchronise releases to NixOS, which doesn't seem ideal.

They have implement the changes at some point.
Without a way to notify them early of the otherwise silent but ready-to-go migration, further delays would have no use.
I doubt that many projects are willing to implement the necessary mechanisms to receive these warnings. The complexity of that, in terms of more complicating Nix wiring and other extra setup, may not be worthwhile.

This isInOldestRelease function is super confusing

I'd be excited to have a better name for it. Probably part of the problem is talking about EOL while passing the release in which it's implemented instead. I've tried to make it do too much, trying to bake policy instead of just mechanism into the function.
How about isKnownEndOfLife <latest release before change is merged>? It's the same information, but perhaps with less confusion.

@roberth
Copy link
Member

roberth commented Feb 28, 2024

Or we could make a proper high-level interface: mkDeprecationCycle { introducedIn = <upcoming release>; message = ...; } which can return attributes like warns (bool), and addMessage, which can warn and/or throw.

@infinisil
Copy link
Member Author

Ohh I was missing a crucial part:

nixpkgs/lib/trivial.nix

Lines 175 to 177 in 273e190

oldestSupportedRelease =
# Update on master only. Do not backport.
2311;

I thought that oldestSupportedRelease was being backported, but it's not. So on stable releases you'll never suddenly get deprecation warnings, instead this only happens on unstable.

So this means that there's a nice step ladder of:

  • Release 23.05 doesn't support the new feature
  • Release 23.11 supports both the old and new features without a warning
  • Release 24.05 supports both the old and new features, but the old one gives a warning

And during release 23.11, third-party projects will get reports of the warning from unstable users, prompting them to switch to the new feature ahead of release 24.05. So this is looking good to me after all :)

@infinisil
Copy link
Member Author

Or we could make a proper high-level interface: mkDeprecationCycle { introducedIn = <upcoming release>; message = ...; } which can return attributes like warns (bool), and addMessage, which can warn and/or throw.

Something like that would be much better imo. This very much reminds me of my old RFC 33, though I'm not sure if it took this into account.

I don't think this is high-priority issue, but let's leave it open to track isInOldestRelease interface improvements

@roberth
Copy link
Member

roberth commented Feb 29, 2024

missing a crucial part:

I suppose that's easy to miss. Certainly the name didn't help. A good name would probably be 80 characters... or like 3 so that everyone must read the docs...
Something like a mkDerpecationCycle might be best then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants