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

lib: Document stability guarantee #322733

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

infinisil
Copy link
Member

Description of changes

Could be better, but this is a good start :)

Prompted by @Janik-Haag, ping @roberth for review


This work is sponsored by Antithesis ✨

Add a πŸ‘ reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: lib The Nixpkgs function library label Jun 26, 2024
Comment on lines +60 to +63
### Ensure backwards compatibility

`lib` is the effective standard library for Nix, both for Nixpkgs and third-parties.
Because of that it's important to make sure that the interface isn't changed in a breaking way.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhm, this seems a bit extreme. There will always be some factors causing a little change, sometimes even external ones. I think a better policy would be to avoid breaking changes if possible, but allow them for every new stable release if they are documented and reasoned accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for example: @infinisil you do realize that this would have made the markdown transition completely impossible, right? "no breaking changes" is a nonsensical policy for anything that isn't capital d Done (and for those things it's not necessary, because they're done.)

(this includes your revised position of "if not released yet".)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's very rare that a breaking change is necessary, we haven't had a real one in the past 2 releases (and I didn't write release notes before that):

  • 24.05: None at all
  • 23.11: Just removal of deprecated functions and a very minor one to lib.foldl' and friends

I think it's fine to say "no breaking changes", but not insist on it when there's a good case for it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pennae I guess I can clarify that removing deprecated functions is fine after some time (but removing functions without deprecating them is not).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then that should be spelled out very clearly in the document, along with a deprecation process. "No API changes ever EVER" is not something we want, and not something we can achieve even if we did want it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I was simply addressing numinit's usecase.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat, thanks. Still glad to see the latter case too :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splitting lib into a separate repository sounds like a bad idea. The "beauty" of nixpkgs is that you can easily do large refactors, bisecting, etc... there is a reason why most similar projects are structured in a monorepo like fashion (we can and should consider doing regular repository exports, maybe with something like josh). What we should imho do is unify the entry points for lib and not make them gaslight you, currently there are like 4 different ones with at least one changing the behavior of lib.evalModules without telling you. But all of this is off-topic and beside the point, providing a longer stability guarantee for lib than we do for modules is ridiculous.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changing the behavior of lib.evalModules without telling you

πŸ‘€ I think you're referring to the NixOS library nixos/lib, and I'd be guilty of that. We could rename that function there if you think it's confusing. To be clear that library is supposed to be NixOS-as-a-library, and not a clone of Nixpkgs lib.

As for the flake lib, that's definitely a problem. I'd be in favor of moving the Nixpkgs lib to (getFlake "nixpkgs").lib.lib, so that lib.nixos isn't presented as a Nixpkgs lib sublibrary.

What's the other entrypoint?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ‘€ I think you're referring to the NixOS library nixos/lib, and I'd be guilty of that. We could rename that function there if you think it's confusing. To be clear that library is supposed to be NixOS-as-a-library, and not a clone of Nixpkgs lib.

Yes I'm and yes I think having different names would make sense here, e.g. lib.evalModules and lib.evalNixOS or something similar. E.g. I was talking with someone about the minimalModules feature flag some time ago, and differentiating between lib/ and nixos/lib was a bit confusing here.

As for the flake lib, that's definitely a problem. I'd be in favor of moving the Nixpkgs lib to (getFlake "nixpkgs").lib.lib, so that lib.nixos isn't presented as a Nixpkgs lib sublibrary.

What's the other entrypoint?

IIRC there is the flake one, the lib/ one, the lib that gets passed to you inside a nixos module and pkgs.lib

infinisil has done a great job at maintaining lib, and he always listens to contributors, so I really don't get why the tone changed.

I want to disagree with this, I know at least one case where people opted to contribute to nixos/lib/ instead of lib/ and multiple cases where people out right started to avoid contributing to nixpkgs because of the dreadful experiences they had trying to contribute to lib/

@infinisil infinisil force-pushed the lib-stability branch 2 times, most recently from 47e9120 to 5a0cded Compare June 26, 2024 20:54
Could be better, but this is a good start
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jun 26, 2024
The only exceptions are:
- Breaking changes to interfaces that aren't in a release yet
- Breaking changes that are unlikely to cause problems for users
- Removal of functions that have been deprecated for at least one release
Copy link
Contributor

@eclairevoyant eclairevoyant Jun 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean it needs to be deprecated for 6 calendar months, or that functions can be removed the day after the release it was deprecated in?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They can be removed immediately following the release branch-off, so that the release will have the warning, while the next one won't, I'll clarify this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer for deprecations to start after old stable EOL, especially if it doesn't have the new alternative. That adds one month, but provides a guarantee that third party libraries work with all currently supported versions of lib, ensuring that maintainers don't get caught in this sequence of events:

  • users report deprecation warning
  • maintainer acts on it
  • users report evaluation error on a supported Nixpkgs release
  • frustrated maintainer has to revert or write a workaround

Alternatively, we could require a backporting of the new thing to stable / old stable to mitigate compatibility issues, but that's slightly more effort.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/discussion-on-type-functor-changes/57282/3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: lib The Nixpkgs function library 9.needs: community feedback 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants