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

[RFC 0164] Nim library lockfiles #164

Closed
wants to merge 1 commit into from

Conversation

ehmry
Copy link

@ehmry ehmry commented Oct 13, 2023

This RFC describes a refactoring of Nim packages within Nixpkgs.

Rendered

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/rfc-0164-nim-library-lockfiles/34140/1


- Lockfiles must be checked into Nixpkgs which would bloat the repository.
- Checks on Nim libraries would not be run (unless implemented in `buildNimPackage`).
- Nim programs would be updated as usual but their dependencies would not. Updating lockfiles would be non-trivial for automated tools such as `r-ryantm/nixpkgs-update`.
Copy link

@pbsds pbsds Oct 13, 2023

Choose a reason for hiding this comment

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

This could be solved by making buildNimPackage set a default passthru.updateScript

@FRidh
Copy link
Member

FRidh commented Oct 13, 2023

Good to see this proposal. Given the impact I don't think there is a need to have an rfc at this place, and that instead having a regular issue in Nixpkgs is sufficient, potentially announced via discourse. It would still be an rfc, but without the process that is involved here.

@AndersonTorres
Copy link
Member

I like the idea, but I believe it does not need an RFC. A team would suffice.

@ehmry
Copy link
Author

ehmry commented Oct 15, 2023

This is an issue of policy rather than a request to move some files around in Nixpkgs. I want to know what the long term goals are for languages like this and what the policies are for checking non-nix files into Nixpkgs.

I could have already merged everything I've proposed but I expect that I would get zero review before merging and some complaints afterwards.

Should I merge first like we've done with the other RFCs?

@alyssais
Copy link
Member

Here's a concern I have in general with lockfile-based packaging in Nixpkgs, but I don't know if it applies to Nim:

If there's a security vulnerability in curl, we update our curl package, and then thousands of packages that use our libcurl all have the vulnerability patched. If there's a security vulnerability in a Rust library, we have to update every leaf package individually to pick up the new dependency, and this would usually mean patching the lockfile. In practice, nobody does this.

How easy would it be to update a Nim library across the board, if it was found to have a vulnerability or annoying bug?

@ehmry
Copy link
Author

ehmry commented Oct 15, 2023

@DavHau recommended that we pass the libraries selected by lockfiles though a function that applies overrides according to version constraints, which normally would be used to add buildInputs or add some compiler flags. The same mechanism could also detect libraries with known vulnerabilities and propagate a breakage.

I should prototype something to do this in a language agnostic way before undrafting this PR.

@alyssais
Copy link
Member

Yeah, that sounds like a good idea, and what we already do for Ruby. Would it allow straight up overriding the source to point to a newer version? I don't like how reliant we are on upstreams to update their lockfiles.

@ehmry
Copy link
Author

ehmry commented Nov 6, 2023

I've implemented lockfile overriding by creating an attrset of overlays that take the lockfile data as a parameter, so that should be powerful enough to implement version constraints. - https://github.com/ehmry/nixpkgs/blob/nim-annotations/pkgs/top-level/nim-annotations.nix

@ehmry ehmry marked this pull request as ready for review November 10, 2023 12:39
@tomberek tomberek added the status: open for nominations Open for shepherding team nominations label Nov 15, 2023
@infinisil infinisil changed the title nim-library-lockfiles [RFC 0164] Nim library lockfiles Nov 16, 2023
@ehmry ehmry closed this Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: open for nominations Open for shepherding team nominations
Projects
None yet
7 participants