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

nixos/tests/misc: support old and new path-info output structure #315857

Merged
merged 1 commit into from
May 30, 2024

Conversation

mweinelt
Copy link
Member

Description of changes

#315262 (comment)

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@mweinelt mweinelt added the 1.severity: channel blocker Blocks a channel label May 30, 2024
@github-actions github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label May 30, 2024
@fricklerhandwerk
Copy link
Contributor

fricklerhandwerk commented May 30, 2024

@nixos/nix-team Why in the world is Nixpkgs even testing against Nix experimental commands? That's exactly how you make them unchangeable or annoy everyone with breakages.

Looking at commit history, this has been added before experimental features were a thing. I suggest removing that test and only adding it back when the command is stabilized.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels May 30, 2024
@K900 K900 removed the 1.severity: channel blocker Blocks a channel label May 30, 2024
@K900
Copy link
Contributor

K900 commented May 30, 2024

Going to untag as channel-blocker as the Nix bump was reverted.

@Mic92
Copy link
Member

Mic92 commented May 30, 2024

@NixOS/nix-team Why in the world is Nixpkgs even testing against Nix experimental commands? That's exactly how you make them unchangeable or annoy everyone with breakages.

Looking at commit history, this has been added before experimental features were a thing. I suggest removing that test and only adding it back when the command is stabilized.

I think we are beyond a point where we can no longer reasonable treat these commands as experimental. Even if it doesn't break in nixpkgs, it will still break other users, since these commands have been cooking for too long in an "experimental" state.

@Mic92
Copy link
Member

Mic92 commented May 30, 2024

@mweinelt thoughts about: #315878 ?

@mweinelt
Copy link
Member Author

If it broke, then yes, passthru.tests seems appropriate. Naming the test misc 🤷

@Mic92
Copy link
Member

Mic92 commented May 30, 2024

Yeah. Terrible name. I will come up with something else.

@lilyinstarlight
Copy link
Member

I think we are beyond a point where we can no longer reasonable treat these commands as experimental. Even if it doesn't break in nixpkgs, it will still break other users, since these commands have been cooking for too long in an "experimental" state.

This command has been present since Nix 2.0 and was added to the NixOS misc tests during the Nix 2.0 bump PR (https://github.com/NixOS/nixpkgs/pull/34636/files#diff-5eb2258233433ba8710976a266f30f16a70d92c633d36706072a095f91dc50a9R38)

I'm just providing context, though, so I won't make any comment either way about whether it's appropriate the command was later made "experimental" when that ability was added (like the nix repl command) or whether it should be removed from the test until the command is "stabilized"

@Mic92
Copy link
Member

Mic92 commented May 30, 2024

Tested that this, fixes the nix upgrade. I currently don't see a good replacement for this command in the old cli interface. If we make sure that we run this NixOS test on every nix version bump, than we can also fix it.

@Mic92 Mic92 merged commit 5da922e into NixOS:master May 30, 2024
23 checks passed
@mweinelt mweinelt deleted the misc-test-pathinfo-variants branch May 30, 2024 14:05
@Ericson2314
Copy link
Member

I will say, I mean for things to be experimental because I intend to make breaking changes. It's quite simple. I caused this breaking change, and I have further changes to the format in NixOS/nix#9995, and probably more after that. Per the issue through, the JSON guidelines need to be further fleshed out, and this (and other interfaces) need to be brought into compliance.

The unstable interface of Nix grew much faster than our ability to maintain it, or make it adequate quality. It's a hard problem to solve without a more developer hours.

@Ericson2314
Copy link
Member

I currently don't see a good replacement for this command in the old cli interface.

There is a mishmash of nix-store commands to query individual fields of the path info JSON. Agreed that is much less nice to use!

If we make sure that we run this NixOS test on every nix version bump, than we can also fix it.

That's true. If we the tests make clear they are not locking down features users should rely on, but just sanity checking Nixpkgs' own internal usage of experimental stuff, it is less bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants