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

Optional dependency on opam-overlays and mirage-opam-overlays #43

Open
Julow opened this issue May 25, 2023 · 8 comments
Open

Optional dependency on opam-overlays and mirage-opam-overlays #43

Julow opened this issue May 25, 2023 · 8 comments

Comments

@Julow
Copy link
Contributor

Julow commented May 25, 2023

opam-nix have a lot of dependencies but two seem unnecessary: opam-overlays and mirage-opam-overlays.
They are added to the list of repositories used for solving, regardless of whether the packages being solved need these or not. Since #18

This adds overhead to the build but what worries me the most is the extra maintenance caused by these. I actively fight Nix tendency to use several version of each dependency and end up with a large flake.nix:

{
  inputs.nixpkgs.url = "github:nixos/nixpkgs/nixos-unstable";
  inputs.opam-nix = {
    url = "github:tweag/opam-nix";
    inputs.nixpkgs.follows = "nixpkgs";
    inputs.flake-utils.follows = "flake-utils";
    inputs.opam-repository.follows = "opam-repository";
    inputs.opam-overlays.follows = "opam-overlays";
    inputs.mirage-opam-overlays.follows = "mirage-opam-overlays";
  };
  inputs.flake-utils = {
    url = "github:numtide/flake-utils";
  };
  inputs.opam-repository = {
    url = "github:ocaml/opam-repository";
    flake = false;
  };
  inputs.opam-overlays = {
    url = "github:dune-universe/opam-overlays";
    flake = false;
  };
  inputs.mirage-opam-overlays = {
    url = "github:dune-universe/mirage-opam-overlays";
    flake = false;
  };
}

These dependencies could be optional, at the cost of extra setup for users that need them and are not fighting against Nix.

@balsoft
Copy link
Collaborator

balsoft commented May 25, 2023

They are added to the list of repositories used for solving, regardless of whether the packages being solved need these or not. Since #18

They shouldn't be; AFAIU they are only added to queryToMonorepo or buildMonorepo, in which case they appear useful.

This adds overhead to the build but what worries me the most is the extra maintenance caused by these. I actively fight Nix tendency to use several version of each dependency

AFAIU the only downside to having them in opam-nix is slightly bigger lockfiles downstream; users of downstream projects that don't call queryToMonorepo or buildMonorepo shouldn't need to actually download those repositories.

@Julow
Copy link
Contributor Author

Julow commented May 25, 2023

Indeed, I misread the code.

It's more than bigger lockfiles, it's also bigger flake.nix files for people that like to control their dependency and more time lost not understanding what they are doing.

Is it a good practice to override opam-nix's inputs the way I did ?
If yes, I think monorepo users might want to control their dependency to the overlays too, making the defaults an unnecessary burden. (same for the dependency to opam-repository)
It's entirely possible that my workflow is wrong.

@balsoft
Copy link
Collaborator

balsoft commented May 25, 2023

Is it a good practice to override opam-nix the way I did ?

I think it's ok, but when you override opam-repository (or any other input for that matter) there might be incompatibilities which you might need to fix with your own overlays. The most common painpoint is findlib, which gets broken on almost every update and requires a new patch (see https://github.com/tweag/opam-nix/tree/main/patches/ocamlfind) every time. You might have to write this patch yourself if you update opam-repository. There are also other common packages that can be broken by opam-repository or nixpkgs updates. Therefore, I think you should only override inputs if you absolutely have to and your package doesn't build otherwise, or if you know what you're doing and don't care about default overlays/patches.

If yes, I think monorepo users might want to control their dependency to the overlays too, making the defaults an unnecessary burden. (same for the dependency to opam-repository)

AFAIK there's no way to make flake inputs without "default" values. Do you mean removing the inputs completely, I'll defer to @RyanGibb, who introduced them in the first place.

it's also bigger flake.nix files for people that like to control their dependency

In your case, opam-overlays and mirage-opam-overlays aren't really your dependencies, they are never used by your flake. As such, I would recommend removing that override, since it is functionally useless. This cleans up flake.nix, at least.

@Julow
Copy link
Contributor Author

Julow commented May 26, 2023

The most common painpoint is findlib, which gets broken on almost every update and requires a new patch

Good to know! Though, the point of overriding the input is not necessarily to get the most uptodate version but to have more control (eg. easily switch to an other branch than master, update some flake inputs but not all).

Do you mean removing the inputs completely,

That's what I mean.

I would recommend removing that override

That's what I'll do, thanks!

@RyanGibb
Copy link
Contributor

If yes, I think monorepo users might want to control their dependency to the overlays too, making the defaults an unnecessary burden. (same for the dependency to opam-repository)

AFAIK there's no way to make flake inputs without "default" values. Do you mean removing the inputs completely, I'll defer to @RyanGibb, who introduced them in the first place.

I think if the monorepo functions aren't used then the only downside is a slightly bigger lockfile.
Having opam-overlays by default greatly improves usability as without it many common packages would be broken: it patches packages to be built with dune, which is required for the monorepo workflow.
And as mirage unikernels are normally built with a monorepo of dependency sources for cross-compilation having mirage-opam-overlays by default is useful, although I'd be happy to push this functionality into https://github.com/RyanGibb/hillingar if you'd prefer to remove it.

@Julow
Copy link
Contributor Author

Julow commented May 26, 2023

I'm not suggesting to go without the overlays, but instead to ask the user provide them. Examples/templates are already huge and could host the management of these two overlays, which wouldn't be the most complicated part.

To go further, I would suggest to do the same for opam-repository. Like with the overlays, it's important to control its version and it's not used inside opam-nix except as the input of the solver.

@RyanGibb
Copy link
Contributor

I understand what you mean, and I do a similar thing for my own nixos config.

I don't have a good intuition for what is best for new users.

@balsoft
Copy link
Collaborator

balsoft commented May 26, 2023

To go further, I would suggest to do the same for opam-repository. Like with the overlays, it's important to control its version and it's not used inside opam-nix except as the input of the solver.

I'm against this: it will break virtually all downstream flakes, of which there are quite a few already.

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