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

Test perl bindings with nix-serve #11531

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Test perl bindings with nix-serve #11531

wants to merge 2 commits into from

Conversation

roberth
Copy link
Member

@roberth roberth commented Sep 18, 2024

Motivation

Avoid being caught by surprise when updating nix in Nixpkgs.

Alternatives:

  • Deprecate the perl bindings in favor of new bindings built on the C API.
    • Not stable yet
    • Virtually no store-layer interface yet in the C API
  • Write a test suite for the perl bindings, so that these tests are local again, possibly with better coverage. I doubt that the effort is worth it.

Context

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

This includes NixOS/nixpkgs#342817

Flake lock file updates:

• Updated input 'nixpkgs':
    'github:NixOS/nixpkgs/c3d4ac725177c030b1e289015989da2ad9d56af0' (2024-08-15)
  → 'github:NixOS/nixpkgs/3a458f7c763ca62c6bf454b8d828bd86b7250671' (2024-09-18)
@Ericson2314
Copy link
Member

There is a tiny test suite for the perl bindings, but it is indeed not useful for seeing if certain apps have upgraded. In this case the breaking change in the Perl bindings was intentional, so a mere test suite would not have caught it.

Comment on lines +338 to +341
passthru = lib.optionalAttrs (stdenv.buildPlatform.canExecute stdenv.hostPlatform) {
perl-bindings = nix-perl-bindings;
};

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 I would rather not do that here (it doesn't work with the split packages so well), and instead just slap on the attribute for the nix-serve override.

Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

+1 on the concept

@edolstra
Copy link
Member

I'm 👎 on this, since nix-serve is a (not very well maintained) downstream on Nix. It shouldn't be tested here but in the nix-serve project. We already have enough "noise" in terms of failing nix:master Hydra jobs, so I don't want to add another job that is perpetually failing. We also don't have an easy way to fix nix-serve here, i.e.

          nix-serve =
            prev.nix-serve.override {
              # undo potential version pinning
              nix = final.nix;
            };

takes the nix-serve from Nixpkgs rather than nix-serve master.

@roberth
Copy link
Member Author

roberth commented Sep 18, 2024

We also don't have an easy way to fix nix-serve here

In other projects, I've added overrides conditioned on e.g. inputs?nix-serve, so that could make it easy to switch to a different version.

We already have enough "noise" in terms of failing nix:master Hydra jobs

👍 let's make sure to fix that.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2024-09-18-nix-team-meeting-minutes-179/52361/1

@Mic92
Copy link
Member

Mic92 commented Sep 20, 2024

I worked on improving testing in nix-serve here: edolstra/nix-serve#61

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 this pull request may close these issues.

5 participants