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

Improve flake.nix to save build time, make nix run work again, and check builds on CI #312

Merged
merged 18 commits into from
Oct 31, 2023

Conversation

akirak
Copy link
Contributor

@akirak akirak commented Oct 29, 2023

Hello, I am trying to switch from Elixir LS to Next LS. I am a user of NixOS, so I have checked out flake.nix already included in this repository.

I thought the configuration could be better by applying changes proposed in this PR, that will:

  • Improve the build time for Nix users. Downgrade to Erlang R25 because the latest release is not completely ready for NixOS users yet. Major BEAM packages are available from the official binary cache if you pick the previous release or older, but not for R26 yet. Also, the master branch of nixpkgs is somewhat meant for development, so switch to unstable channel where things are less likely to be broken.
  • Specify meta.mainProgram of the package so nix run works. The app seemed to be broken, so I have removed it.
  • Tidy the configuration code to make it more concise.
  • Apply alejandra formatter which is popular in the community. If you don't like this, you can revert only the last commit on flake.nix.
  • Add a CI job to check if it builds both on Linux and macOS.

I am about to incorporate the package into my project, and it seems to be working. Thank you for working on this awesome project!

The master branch is where PRs are merged into. Binary cache is more likely
available on stable and unstable branches, so switch to unstable to use latest
packages.
In NixOS/nixpkgs, the latest version of BEAM packages may not have all packages
built on Hydra. In fact, rebar3 is not available from binary cache, which
requires the package user to rebuild the package. Downgrading to the previous
version of Erlang VM solves this issue.
The path points to a location which apparently no longer exists.
Setting meta.mainProgram is recommended now so `nix run` consistently works.
The user may want to use the same versions as his/her project under development,
so it makes sense to allow overriding of beamPackages and elixir.
@akirak akirak changed the title Improve flake.nix to save build time, make nix run work, and check builds on CI Improve flake.nix to save build time, make nix run work again, and check builds on CI Oct 29, 2023
@mhanberg
Copy link
Collaborator

Wow, this is awesome!

The one thing is that OTP26 is a requirement, would making my own binary cache for Next LS help with the build time?

I'm still a nix newb, so I appreciate the help!

@akirak
Copy link
Contributor Author

akirak commented Oct 29, 2023

The one thing is that OTP26 is a requirement, would making my own binary cache for Next LS help with the build time?

You can sign up an account on Cachix and use cachix-action to upload assets to its server. Cache settings can be added to flake.nix, like in this project.

@akirak
Copy link
Contributor Author

akirak commented Oct 30, 2023

I seem to have messed the ld-linux part, but I think I have fixed it. Sorry about that. It now works, at least on x86_64-linux.

@mhanberg
Copy link
Collaborator

Awesome, i'll pull this down tonight to confirm it works.

I was able to make a binary cache with cachix, just need to figure out how to make the flake use it and get that github action figured out.

@mhanberg
Copy link
Collaborator

@akirak do you happen to know why the flake installs Elixir 1.14 and 1.15?

It already did that before this patch, but I am curious if you know of an explanation

@mhanberg
Copy link
Collaborator

I tried to run this branch locally on my x86 Linux computer and got a hash mismatch

┌─ 07:24:35 AM
│mitchell in 🌐 nublar in next-ls on  flake-nix [?] via  v1.15.5 (OTP 26)
└─  nix build
error: hash mismatch in fixed-output derivation '/nix/store/sl4kqndwjlcvcbdsr1kiy7br724dc08r-next-ls-deps-0.14.2.drv':
         specified: sha256-LCY9ClMG/hP9xEciZUg+A6NQ8V3K7mM2l+6D+WZcubM=
            got:    sha256-MX0X/s2XfnhZS4e3q6E/45tLeY1ie33NNARACtMjE2A=
error: 1 dependencies of derivation '/nix/store/k5z6l3h6nj1jv76w54jf58zy0vamiz42-next-ls-0.14.2.drv' failed to build

@mhanberg
Copy link
Collaborator

Same on my M1 MacBook Air

@akirak
Copy link
Contributor Author

akirak commented Oct 31, 2023

do you happen to know why the flake installs Elixir 1.14 and 1.15?

It seems that fetchMixDeps brings the default version of Elixir, which is currently 1.14:

https://github.com/nixos/nixpkgs/blob/master/pkgs/development/beam-modules/fetch-mix-deps.nix#L12

Fixed at ba5b0ef

@akirak
Copy link
Contributor Author

akirak commented Oct 31, 2023

got a hash mismatch

You have to update the hash value of fetchMixDeps call whenever you update the lock file of Mix.

@akirak
Copy link
Contributor Author

akirak commented Oct 31, 2023

need to figure out how to make the flake use it and get that github action figured out

Here is an instruction:

  1. Add cachix-action to your GitHub workflow, as in this workflow. The action updates all built assets to your account by default.
  2. Add a signing key and/or personal token of your cachix account to the secrets of your repository.

I would also suggest adding extra-substituters and extra-trusted-public-keys to flake.nix as in this example, so the user can locate the binary cache of your packages:

{
  nixConfig = {
    extra-substituters = [ "https://emacs-ci.cachix.org" ];
    extra-trusted-public-keys = [ "emacs-ci.cachix.org-1:B5FVOrxhXXrOL0S+tQ7USrhjMT5iOPH+QN9q0NItom4=" ];
  };
  ... 
}

@mhanberg
Copy link
Collaborator

I would also suggest adding extra-substituters and extra-trusted-public-keys to flake.nix as in this example, so the user can locate the binary cache of your packages:

so if someone does nix profile install github:elixir-tools/next-ls, it will use the binary cache automatically?

@mhanberg mhanberg merged commit ad13581 into elixir-tools:main Oct 31, 2023
5 checks passed
@akirak
Copy link
Contributor Author

akirak commented Oct 31, 2023

so if someone does nix profile install github:elixir-tools/next-ls, it will use the binary cache automatically?

No, the user has to manually approve the extra substituter, unless he/she runs Nix with --accept-flake-config. It is enabled by default with nix-quick-install-action.

Thanks!

@akirak akirak deleted the flake-nix branch October 31, 2023 13:53
@akirak akirak mentioned this pull request Nov 1, 2023
3 tasks
@shanesveller
Copy link
Contributor

Generally speaking, rec/recursive definition syntax is frowned upon by a pretty sizable part of the Nix community as it introduces risks of surprising behavior and worsens troubleshooting. rec appears to have been introduced during this PR.

Do either of you have strong feelings about keeping or removing those recs, with the main alternative being returning to well-scoped let statements like before? I have other planned suggestions for potential improvements to the flake.

@mhanberg
Copy link
Collaborator

mhanberg commented Nov 1, 2023

I have no feelings and will defer to more experienced nix users. PRs are welcome!

@akirak
Copy link
Contributor Author

akirak commented Nov 2, 2023

@shanesveller I would avoid rec in libraries, but I tend to not worry about the problem in flakes. However, I might have gone a bit too far this time, so I will send a PR later to mitigate the issue. Thanks.

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.

3 participants