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

Change to use fetchFromGithub instead of builtins.fetchgit for flake.nix #81

Closed
wants to merge 0 commits into from

Conversation

emilpriver
Copy link
Contributor

Changed to use pkgs.fetchFromGitHub instead of using builtins.fetchgit to make it more "strict"

@omnisci3nce
Copy link

I checked out your PR's branch and ran nix build nix develop then dune test and it seems to work so LGTM

@leostera
Copy link
Collaborator

@emilpriver is this good for merging still?

@emilpriver
Copy link
Contributor Author

emilpriver commented Jun 3, 2024

@leostera yes, when the CI pass should you be able to merge

However I really think we need to look into if we need to follow bytestring, castore and so on here:

bytestring = {
  url = "github:riot-ml/bytestring";
  inputs.nixpkgs.follows = "nixpkgs";
  inputs.rio.follows = "rio";
};

castore = {
  url = "github:suri-framework/castore";
  inputs.nixpkgs.follows = "nixpkgs";
};

config = {
  url = "github:ocaml-sys/config.ml";
  inputs.nixpkgs.follows = "nixpkgs";
};

gluon = {
  url = "github:riot-ml/gluon";
  inputs.nixpkgs.follows = "nixpkgs";
  inputs.bytestring.follows = "bytestring";
  inputs.config.follows = "config";
  inputs.rio.follows = "rio";
};

rio = {
  url = "github:riot-ml/rio";
  inputs.nixpkgs.follows = "nixpkgs";
};

telemetry = {
  url = "github:leostera/telemetry";
  inputs.nixpkgs.follows = "nixpkgs";
};

I wasn't able to use riot flake due to some missmatch between some library so I am prob going to install Riot without the flake. I know @metame said that this is how it's suppose to be and I prob don't understand why this follows is needed but would be great to have a second look and see if they are really needed or not :D

@metame
Copy link
Contributor

metame commented Jun 4, 2024

So actually the minttea follows is needed as well as all of the other ones.
The reason you're having a mismatch in a library is because of not using follows in your own flake properly (the DX on this isn't perfect as a consumer of the flake I'll admit).
I'll explain this here perhaps so that others can reference it as well.

Gluon, bytestring, and config all depend on minttea.spices. If any one of those used a different version, the flake would not build due to a library mismatch. Using follows ensures, that all of the inputs (aka transient deps) are using the same version. This is especially important in OCaml where the builder will only use one version of a package (iirc), which is why nix fails if there are conflicting versions.

@metame
Copy link
Contributor

metame commented Jun 4, 2024

I checked out your PR's branch and ran nix build nix develop then dune test and it seems to work so LGTM

And to bring this home, the reason this worked is because the current state of gluon, bytestring, and config all happen to depend on the same version of minttea.spices in their flake.lock.

@emilpriver
Copy link
Contributor Author

So actually the minttea follows is needed as well as all of the other ones. The reason you're having a mismatch in a library is because of not using follows in your own flake properly (the DX on this isn't perfect as a consumer of the flake I'll admit). I'll explain this here perhaps so that others can reference it as well.

Gluon, bytestring, and config all depend on minttea.spices. If any one of those used a different version, the flake would not build due to a library mismatch. Using follows ensures, that all of the inputs (aka transient deps) are using the same version. This is especially important in OCaml where the builder will only use one version of a package (iirc), which is why nix fails if there are conflicting versions.

Got ya, this make more sense to me :D I prob had some skill issues and didn't understand how everything is connected sat first. Thank you for explaining it

Then I need to fix something on my end instead

@emilpriver
Copy link
Contributor Author

emilpriver commented Jul 12, 2024

Hey @metame this topic was brought up again as @dmmulroy also got this issue. Do you might know a solution to the problem we're having? :D

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.

4 participants