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

Move nixpkgs-check-by-name to a separate repository #286559

Closed
infinisil opened this issue Feb 5, 2024 · 7 comments · Fixed by #299301
Closed

Move nixpkgs-check-by-name to a separate repository #286559

infinisil opened this issue Feb 5, 2024 · 7 comments · Fixed by #299301
Milestone

Comments

@infinisil
Copy link
Member

infinisil commented Feb 5, 2024

nixpkgs-check-by-name is the tool used for the pkgs/by-name CI check. Its source code is defined in Nixpkgs itself under pkgs/test/nixpkgs-check-by-name.

While it's somewhat convenient for it to be in the same repository it checks, experience has shown that it's really a separate project, especially after #281374, which started pinning the tooling.

It would be best if this codebase was moved to a separate repository, with benefits such as:

  • A separate issue/PR tracker, making it easy to track issues and watch for updates. Currently I'm using a milestone to do that instead.
  • Improves clarity on the update process, because currently there's always two Nixpkgs PRs needed to apply a change (the first to change the codebase, the second to update the pinned CI tooling)
  • A separate Hydra jobset can significantly shorten turnaround time, since there's no need to wait for the NixOS channel to update anymore (that's what is currently used to avoid having to build the tooling for every CI run triggered in GitHub Actions).
  • Versioning the tool properly improves signalling regarding the significance of updates.

With more details:

  • Move pkgs/test/nixpkgs-check-by-name (except ./scripts) to something under https://github.com/NixOS
    • This codebase is showing promise to also be useful for non-pkgs/by-name checks, so a name not specific to that might be best
    • @philiptaron suggested nixpkgs-vet, since it "vets" the Nixpkgs Architecture :)
  • Create a separate Hydra project for the codebase and set up the Nixpkgs-side pinning to use it
    • It's not necessary to expose the tooling's Nix derivation in Nixpkgs, because pinning for CI is best based directly on the store path, because it makes CI very fast because it doesn't need to download any Nixpkgs
  • Make at least me the owner of the repository
@infinisil infinisil added this to the RFC 140 milestone Feb 5, 2024
@philiptaron
Copy link
Contributor

How can I help move this issue along? I find contributing to the tool in nixpkgs difficult, but if it were extracted as nixdoc is extracted, it would be substantially easier.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/nixpkgs-architecture-team-conclusion-and-prospective/41020/6

@infinisil
Copy link
Member Author

@philiptaron I just asked @zimbatm to create a repo for this so I can do the move. I'd like to just use github.com/NixOS/nixpkgs-check-by-name for now since that's what it's already called, let's have minimal changes at a time, and we can always easily rename it later if it becomes more generic :)

@zimbatm
Copy link
Member

zimbatm commented Mar 16, 2024

Done

@philiptaron
Copy link
Contributor

philiptaron commented Mar 16, 2024

I look forward to contributing there. It's a little too hard to do it in the giant monorepo, I find. @infinisil's skill in doing so is admirable.

@infinisil
Copy link
Member Author

infinisil commented Mar 18, 2024

Steps to complete this:

philiptaron added a commit to NixOS/nixpkgs-vet that referenced this issue Mar 18, 2024
After the move into this new repository (NixOS/nixpkgs#286559), some changes to the readme are necessary.

1.    Fixes Nixpkgs links
2.    Defers to Nixpkgs' workflow to explain how it gets integrated
3.    Add some basic information about the commit permissions.
philiptaron added a commit to NixOS/nixpkgs-vet that referenced this issue Mar 18, 2024
After the move into this new repository (NixOS/nixpkgs#286559), some changes are necessary to make it work, since there's some Nix code that relied on ../../.. to be Nixpkgs.

*  Adds a pinned Nixpkgs using npins
*  Move default.nix to package.nix and move things that are needed outside of it to default.nix
*  Fix shell.nix to use the shell attribute from default.nix.

This only fixes the core `.nix` files.
@infinisil
Copy link
Member Author

Just opened #297901 to complete the move. Still a draft yet though

infinisil added a commit to tweag/nixpkgs that referenced this issue Mar 26, 2024
The nixpkgs-check-by-name tooling is [being moved](NixOS#286559 (comment))
to a [separate repo](https://github.com/NixOS/nixpkgs-check-by-name).

This commit updates Nixpkgs CI to use it instead of the tree inside
Nixpkgs
infinisil added a commit to tweag/nixpkgs that referenced this issue Mar 26, 2024
The nixpkgs-check-by-name tooling is [being moved](NixOS#286559 (comment))
to a [separate repo](https://github.com/NixOS/nixpkgs-check-by-name).

This commit updates Nixpkgs CI to use it instead of the tree inside
Nixpkgs
infinisil added a commit to tweag/nixpkgs that referenced this issue Mar 26, 2024
The nixpkgs-check-by-name tooling is [being moved](NixOS#286559 (comment))
to a [separate repo](https://github.com/NixOS/nixpkgs-check-by-name).

This commit updates Nixpkgs CI to use it instead of the tree inside
Nixpkgs

No changes have been made to the tooling locally since it was moved:
- [Exported history](https://github.com/NixOS/nixpkgs/commits/55bf02190ee57fcf83490fd7b6bf7834e28c9c86/pkgs/test/nixpkgs-check-by-name)
- [Imported history](https://github.com/NixOS/nixpkgs-check-by-name/commits/d579e1821d56c79fd90dab34b991cc7bdab7a5c6/)
infinisil added a commit to tweag/nixpkgs that referenced this issue Mar 26, 2024
The nixpkgs-check-by-name tooling is [being moved](NixOS#286559 (comment))
to a [separate repo](https://github.com/NixOS/nixpkgs-check-by-name).

This commit updates Nixpkgs CI to use it instead of the tree inside
Nixpkgs

No changes have been made to the tooling locally since it was moved:
- [Exported history](https://github.com/NixOS/nixpkgs/commits/55bf02190ee57fcf83490fd7b6bf7834e28c9c86/pkgs/test/nixpkgs-check-by-name)
- [Imported history](https://github.com/NixOS/nixpkgs-check-by-name/commits/d579e1821d56c79fd90dab34b991cc7bdab7a5c6/)
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

Successfully merging a pull request may close this issue.

4 participants