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

Are binary checks the responsibility of nixpkgs-update? #260

Closed
raboof opened this issue Apr 16, 2021 · 7 comments
Closed

Are binary checks the responsibility of nixpkgs-update? #260

raboof opened this issue Apr 16, 2021 · 7 comments

Comments

@raboof
Copy link
Contributor

raboof commented Apr 16, 2021

Shouldn't we encourage projects to add those in their installCheckPhase, like NixOS/nixpkgs#119636 ?

@ryantm
Copy link
Collaborator

ryantm commented Apr 16, 2021

@raboof I agree that it would be better for those checks to be upstreamed. If there was some default version check that checked the version of everything in the bin directory, that would probably be quite useful.

It may be preferable to use passthru.tests instead of installCheckPhase, since the later introduces overhead of every build. Also, passthru.tests can be executed independently of the build.

@ryantm
Copy link
Collaborator

ryantm commented Apr 16, 2021

nixpkgs-update and OfBorg run passthru.tests

@raboof
Copy link
Contributor Author

raboof commented Apr 17, 2021

It may be preferable to use passthru.tests instead of installCheckPhase, since the later introduces overhead of every build. Also, passthru.tests can be executed independently of the build.

Took me a while to figure out how exactly, so proposed NixOS/nixpkgs#119731

@raboof
Copy link
Contributor Author

raboof commented Apr 18, 2021

It may be preferable to use passthru.tests instead of installCheckPhase, since the later introduces overhead of every build. Also, passthru.tests can be executed independently of the build.

let's discuss this in NixOS/nixpkgs#119731 (comment)

raboof added a commit to raboof/nixpkgs that referenced this issue Apr 25, 2021
passthru.tests instead of installCheckPhase as recommended in
nix-community/nixpkgs-update#260 (comment)

Inspired by
https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/science/logic/key/default.nix#L54-L57

This is basically the extent of testing I usually do when an upgrade
is proposed, so this takes that manual step away.
tomberek pushed a commit to NixOS/nixpkgs that referenced this issue Apr 28, 2021
passthru.tests instead of installCheckPhase as recommended in
nix-community/nixpkgs-update#260 (comment)

Inspired by
https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/science/logic/key/default.nix#L54-L57

This is basically the extent of testing I usually do when an upgrade
is proposed, so this takes that manual step away.
raboof added a commit to raboof/nixpkgs that referenced this issue Aug 14, 2021
And mention you can have either lightweight 'package' or
more heavyweight 'NixOS' (module) tests.

This was suggested at
nix-community/nixpkgs-update#260 (comment)
and discussed further at
NixOS#119731
@ryantm
Copy link
Collaborator

ryantm commented Sep 2, 2021

I think I will get rid of the binary checks. I don't think they work that well, are looked at much, and operationally they stall updates sometimes the programs hang around in a way it doesn't know how to kill.

@Artturin
Copy link
Member

I look at the binary checks, and I can merge the pr without even building it if those pass and the upstream commits are okay.

@ryantm
Copy link
Collaborator

ryantm commented Jul 30, 2022

I removed binary checking.

@ryantm ryantm closed this as completed Jul 30, 2022
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

3 participants