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

pick, harvest and winnow functions exported from std should respect available systems #337

Open
truelecter opened this issue Aug 11, 2023 · 4 comments

Comments

@truelecter
Copy link

truelecter commented Aug 11, 2023

Currently, using either of these methods will uplift to all 4 systems ({darwin,linux}-{aarch64,x86_64}), while some block types may not support some of the systems.

A good example will be the "installable" block type, which are packages with meta.platforms attribute that is not respected.

For example, GraalVM EE 21 (LTS) does not have a distribution for darwin-aarch64, but is available for other 3 platforms from the list.

The expected result would be that nix flake show would yield the correct output, respecting some meta attribute (meta.platforms in case of installable/packages). However, now it will try to evaluate the package that is not possible to be built on some platforms.

Another reasonable solution may be expanding winnow (or another special "harvester") will have an expanded predicate that will have a "system" attribute as well, so some functionality may be excluded for some systems where they do not make sense

@blaggacao
Copy link
Collaborator

blaggacao commented Aug 11, 2023

packages with meta.platforms attribute t

That's what #41 attempted to capture, as well.

The expected result would be that nix flake show

This is one of the nix quirks that contributed to the need to have a special purpose CLI a la std. Newer versions of nix have fixed this and all systems are only evaluated with nix flake show --all-systems.

├───devShells
│   ├───aarch64-darwin
│   │   ├───cardano omitted (use '--all-systems' to show)

We may reopen #41 again, but similar to Hive's collectors, we'd need a generic enough interface that can be implemented as part of paisano-nix/core. (i.e. it's not about making a defective nix CLI happy, but about not to engage in "false advertising")

@truelecter
Copy link
Author

truelecter commented Aug 11, 2023

all systems are only evaluated with

Does not help when using nix flake show on unsupported system, sadly

I think we can make and overridable variant of winnow, for frameworks like std and hive to be able to add some context to the predicate, as now the predicate is being directly passed to the filterAttrs, or just to expand predicate parameters with more context

Here is super-naive implementation of passing more context to winnow's predicate:
paisano: paisano-nix/core@main...truelecter:paisano-core:testing
consumed in soil: https://github.com/truelecter/hive/blob/dcf68cf09507e1f08ff6b41c6ab5d65c7755d8d8/flake.nix#L198-L214

Expanded winnow suits well into the paisano-nix/core, while the predicate I've used can be stored in std lib

@blaggacao
Copy link
Collaborator

blaggacao commented Aug 11, 2023

Hm not sure about this. The harvest function family is designed for compatibility with the nix CLI, but "fraudulent advertising" (compat with systems that isn't true) is actually an issue of its own and unrelated to the nix CLI.

meta.platforms (if a derivation) or config.nixpkgs.system (if a module system) would be more suitable contracts to filter on.

The problem I have is just that they aren't currently very strong contracts / guarantees inside the Nix community. But there may be a way that we can enforce them inside Paisano-based projects.

If we do that, encouraging / enforcing the use of lib.lazyDerivation might be a good choice alongside to keep a check on evaluation times for that filtering.

@truelecter
Copy link
Author

The harvest function family is designed for compatibility with the nix CLI, but "fraudulent advertising" (compat with systems that isn't true) is actually an issue of its own and unrelated to the nix CLI.

Problem is that harvest family currently does not provide a functionality to exclude entities from getting into wrong system scope. Providing required context will make filtering possible, at least.

Also, I do not think that filterting should be done on exclusively on paisano's side. To me, paisano provides solid abstractions, while std contains more specific implementations (blocktypes for example). As such, I do not think that paisano should differentiate between the types of harvested blocks at all and if we want to enforce some kind of contracts, frameworks like std and hive are a better place, because they have the blocktypes implementations.

If we do that, encouraging / enforcing the use of lib.lazyDerivation might be a good choice alongside to keep a check on evaluation times for that filtering.

Did not knew about the existence of lazyDerivation, but I do not think that we can enforce its usage, as lazyDerivations are indistinguishable from plain derivations.

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

2 participants