Skip to content

Test perl bindings with nix-serve#11531

Closed
roberth wants to merge 2 commits intomasterfrom
test-perl-bindings
Closed

Test perl bindings with nix-serve#11531
roberth wants to merge 2 commits intomasterfrom
test-perl-bindings

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

@Mic92
Copy link
Member

Mic92 commented Mar 12, 2025

Downstream maintainer of nix-serve is against this idea.

@Mic92 Mic92 closed this Mar 12, 2025
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