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

Draft: Add Nix flake, use flake-parts module #1705

Merged
merged 13 commits into from
Jul 13, 2023
Merged

Draft: Add Nix flake, use flake-parts module #1705

merged 13 commits into from
Jul 13, 2023

Conversation

Eisfunke
Copy link
Contributor

@Eisfunke Eisfunke commented Jun 7, 2023

devenv implements and internally uses various features from Nix flakes and flake.parts, but hides a lot of that stuff to keep things easier for beginners. However, for a project like IHP I believe we can get a lot of advantages by using Nix flakes and flake-parts directly, as we don't just need the development environment, but a lot more. E.g. the devenv docs say:

"Using devenv configuration in flakes is useful for projects that need to define other Nix flake features in addition to the development shell. Additional flake features may include the Nix package for the project or NixOS and Home Manager modules related to the project. Using the same lock file for the development shell and other features ensures that everything is based on the same nixpkgs."

I'd be happy about any feedback about this PR and the approach!

Summary

This is still a WIP draft, but should be enough to see what I'm going for.

For simplicity, I also describe the changes from digitallyinduced/ihp-boilerplate#25 here, as they complement each other.

  • I reshuffled a lot of the devenv code from feat: add devenv support #1653.
  • I added a basic flake.nix exporting a flakeModule (see https://flake.parts) from flake-module.nix providing options for IHP, which is what would be imported by individual IHP apps, see my changes to ihp-boilerplate in: TODO
  • The flake module encompasses most of what was previously contained in NixSupport/devenv.nix and in flake.nix on the app side in the ihp-boilerplate repo.
  • devenv.yaml is now replaced by the inputs section of flake.nix. There's no need for a devenv.lock anymore, we have a flake.lock now instead.
  • The flake also exports ihp-boilerplate as a flake template so it can be used through nix flake init.
  • I also updated the flake-compat in default.nix in the boilerplate, because the older one doesn't support the packages.system.default, only the older defaultPackage.system.
  • The flake in the boilerplate is built such that it uses the inputs from the IHP flake instead of its own inputs. This is done in order to let us set all versions for inputs on the IHP side. The user should just need to update the IHP version and all other versions, such as the pinned nixpkgs version, would be updated with that as well.
  • What previously was in the root devenv.nix is now in devenv-module.nix used by flake.nix to provide a devShell for IHP itself.

A thing I noticed: RunDevServer uses nix-shell, which uses default.nix in the boilerplate, which in turn uses flake-compat, which in turn uses defaultPackage / packages.default from the flake. And that's currently set to be the release build, which I copied over from the previous boilerplate code. Is it intended like that?

Advantages

Some advantages of this approach using flakes:

  • this significantly reduces boilerplate: apart from importing the IHP flake module and configuring it there is not much more to do in the app flake
  • as a consequence, this makes it significantly easier to push improvements in the build process in the future: we can update the flake module on the IHP side, and users only have to pull in the new flake with nix flake update to get those improvements, instead of individually patching their local flakes. New features could also be configured by adding options in the module without breaking backward compatiblity.
  • it's extensible, we can provide everything IHP might want to via the flake, e.g. docker images, nixos configurations (e.g. for use with shipnix), the IHP lib itself, ... and we can surface these features as user-friendly flake module config options, similarly to how devenv does it.
  • this only requires a Nix installation, as even devenv is installed through Nix via the flake. It integrates well with standard nix tooling, just use nix develop to enter the devShell (or use direnv, but that's optional), then devenv is available and you can devenv up to run the devserver.

Future

In the future, I would also like to add and refactor some stuff in IHP to use a more flake-centric approach based on this as a start. For example:

  • if possible, get rid of the requirement of --impure builds for better cache-ability, better reproducibility and easier deployments e.g. with Shipnix
  • there are still various scripts involved in building IHP, such as what links IHP to build/ihp-lib, I'd like to stream some of that
  • currently RunDevServer still uses the legacy default.nix through flake-compat, it should use the flake directly instead
  • currently haskell packages are built through the .nix files, cabal files are only provided for tools, but have to be kept in sync (more or less). I'd like to use something like https://haskell.flake.page instead that uses tha cabal file as a single source of truth and automates builds.

@CSchank
Copy link
Contributor

CSchank commented Jun 7, 2023

I wonder if this will help us with #1706

@kodeFant
Copy link
Contributor

kodeFant commented Jun 8, 2023

I don't have a qualified enough opinion about flake.parts itself, other than that I briefly tested haskell-flake a while ago and I really liked it!

Only running nix flake update would be great, and would probably also be a simple solution to always use the version that is in sync with the binary cache, which is a recurring headache for deployment (at least for IHP Pro).

On the other hand, we should maybe consider what to do with breaking changes in IHP here as it will be easy to skip the upgrade guide, and potentially not know what IHP version you are running.

  • Solving the --impure stuff is probably a no-brainer to do at some point and can be solved no matter what type of tooling we go for. And I think the solution is relatively simple, mostly solved by passing nixpkgs as an argument from the flake input instead of adhoc imports.
  • Regarding scripts, if the migration script could be run without nix-shell on a server, I think that also would simplify and speed up deployments

Lots of good points here that should probably separate Github issues to not derail the core discussion here 👍

@Eisfunke
Copy link
Contributor Author

Eisfunke commented Jun 8, 2023

Only running nix flake update would be great, and would probably also be a simple solution to always use the version that is in sync with the binary cache, which is a recurring headache for deployment (at least for IHP Pro).

On the other hand, we should maybe consider what to do with breaking changes in IHP here as it will be easy to skip the upgrade guide, and potentially not know what IHP version you are running.

Good point. The smoothest approach to that problem would probably be the one nixpkgs takes: release branches.

E.g. you would specify github:digitallyinduced/ihp/1.0 as input, we could then push all minor version upgrades / non-breaking changes onto that branch and nix flake update would get those without any further intervention, but breaking changes would then be pushed as a new version on a new branch, so you would then have to change the 1.0 in the input to 1.1, similarly to how you would change from the nixos-22.11 branch to the nixos-23.05 branch in your input.

Would require maintaining such branches here in the repo, but that shouldn't be much work.

Lots of good points here that should probably separate Github issues to not derail the core discussion here +1

Yes, absolutely, I only included the "future" part in here to tease some of the stuff we'll be able to do with flakes etc., I'll definitely open separate issues and PRs for that stuff.

@mpscholten
Copy link
Member

really like the release branches idea 👍

@mpscholten mpscholten marked this pull request as ready for review June 20, 2023 09:25
Copy link
Member

@mpscholten mpscholten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is mostly ready to merge, good job 👍

Does it make sense to already adjust the documentation inside this PR? Especially things in https://github.com/digitallyinduced/ihp/blob/master/Guide/package-management.markdown and the UPGRADE.md

flake-module.nix Outdated

# Set optimized = true to get more optimized binaries, but slower build times
# TODO make configurable via option
optimized = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old Make system has two targets to archive this:

make build/bin/RunOptimizedProdServer # Slow build, optimized binary
make build/bin/RunUnoptimizedProdServer # Fast build, no optimizations

Can we do something similiar here? Like two output binaries in our flake, so the end user can decide to get either an optimized or unoptimized build?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, we should be able to provide two flake outputs for optimized/unoptimized builds, so you can build them with something like nix build .#optimizedServer or nix build .#unoptimizedServer and additionally provide an optional option that controls what is built as default package, i.e. what's build with a plain nix build. I'll figure something out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, there are now separate output packages for the optimized and unoptimized prod servers, build them with nix build .#optimized-prod-server or nix build .#unoptimized-prod-server. The default package built by nix build is the unoptimized one. I've decided to leave out an additional config option for that for now, I think that would probably only confuse people, but I can still add it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great thanks 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should likely add these nix build commands to the documentation later on

@Eisfunke
Copy link
Contributor Author

Does it make sense to already adjust the documentation inside this PR? Especially things in https://github.com/digitallyinduced/ihp/blob/master/Guide/package-management.markdown and the UPGRADE.md

Sounds good, will do!

flake-module.nix Outdated Show resolved Hide resolved
@mpscholten
Copy link
Member

Any updates?

@Eisfunke
Copy link
Contributor Author

Sorry for the delay, my plan is currently to wait with writing documentation for this until we figure out what the cause of #1714 is and know what changes in our devenv/flake stuff we need to fix it

@Eisfunke
Copy link
Contributor Author

Eisfunke commented Jul 6, 2023

I've updated the package management and upgrade documentation accordingly.

@Eisfunke
Copy link
Contributor Author

Eisfunke commented Jul 6, 2023

I've removed the devenv installation from UPGRADE.md, as the flake module pulls in devenv automatically via direnv / nix develop. Instead, I've added instructions for enabling flakes, people using Nix for IHP might not have done that before.

@Eisfunke Eisfunke mentioned this pull request Jul 7, 2023
@mpscholten
Copy link
Member

@Eisfunke can you also fix the failing github actions? Besides that it looks ready to merge 👍

@mpscholten mpscholten merged commit bcf8694 into master Jul 13, 2023
@mpscholten mpscholten deleted the nicolas/flake branch July 13, 2023 19:01
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