YosysHQ / yosys

Yosys Open SYnthesis Suite
https://yosyshq.net/yosys/
ISC License
3.32k stars 859 forks source link

Begin supporting nix flakes build system #4238

Closed RCoeurjoly closed 2 months ago

RCoeurjoly commented 4 months ago

So far there is:

Follow up actions would be to:

povik commented 4 months ago

Does it make sense to have a flake without the lock file? Is that even possible?

RCoeurjoly commented 4 months ago

Does it make sense to have a flake without the lock file? Is that even possible?

I think it doesn't make much sense, because one of the main purposes of nix is to provide reproducibility. Without the lock file, every time a user builds the package, a new version of nixpkgs would get pulled. Most notably, the version of the compilers and dependencies would not be version controlled.

That's why this PR provides both flake.nix and flake.lock

povik commented 4 months ago

I think it doesn't make much sense, because one of the main purposes of nix is to provide reproducibility. Without the lock file, every time a user builds the package, a new version of nixpkgs would get pulled. Most notably, the version of the compilers and dependencies would not be version controlled.

I understand the value of reproducibility, but I am not convinced we want to maintain a pinned version of compilers and other dependencies as part of the source tree. I worry that would mean a collection of stale hashes which will get updated irregularly.

If we only include the flake without the lock file, that would still mean others can supply their lock files easily, if they wish to pin the dependencies and establish reproducibility. We will just supply the Nix building instructions as part of the source tree.

RCoeurjoly commented 4 months ago

I think it doesn't make much sense, because one of the main purposes of nix is to provide reproducibility. Without the lock file, every time a user builds the package, a new version of nixpkgs would get pulled. Most notably, the version of the compilers and dependencies would not be version controlled.

I understand the value of reproducibility, but I am not convinced we want to maintain a pinned version of compilers and other dependencies as part of the source tree. I worry that would mean a collection of stale hashes which will get updated irregularly.

If we only include the flake without the lock file, that would still mean others can supply their lock files easily, if they wish to pin the dependencies and establish reproducibility. We will just supply the Nix building instructions as part of the source tree.

Updating the lock file is easy with nix flake update (https://nixos.org/manual/nix/stable/command-ref/new-cli/nix3-flake-update.html). You can even update one input only.

If no lock file is committed to the repo, and there is github action for building with nix, that building process would pull a different nixpkgs every time, because nixpkgs (https://github.com/NixOS/nixpkgs) gets a lot of activity.

For example at the time of writing this comment, the last commit was only 46 minutes ago.

So I would propose committing the lock file, and maybe creating a bot that updates the lock from time to time?

whitequark commented 4 months ago

The key question is: What happens when the build breaks?

If YosysHQ has someone who can and will fix the build, that makes it no different than any other source of breakage, whether it's broken immediately after a nixpkgs update, or at night when the cronjob runs. (For example upstream changes in iverilog recently broke our builds.)

If YosysHQ does not, then the flake should probably just not be in the repo.

whitequark commented 4 months ago

Actually, one advantage of having the flake lockfile be committed that I think no one brought up yet is that it enables people to keep building historical revisions of Yosys in the exact environment that passed CI.

@povik Is this something that will change how you feel about committing the lockfile?

RCoeurjoly commented 4 months ago

I have created a repo to maintain the flake out of tree: https://github.com/RCoeurjoly/yosys_flake/tree/main.

I think @whitequark is right in that adding flake to this repo would create a burden on YosysHQ maintainers, so better to maintain it out of tree, kind of like how yosys is in nixpkgs right now.

Thoughts?

povik commented 4 months ago

We have discussed this among Yosys maintainers and have come up with a plan to:

povik commented 4 months ago

We have discussed this among Yosys maintainers and have come up with a plan to:

Just to be sure @RCoeurjoly, I am not asking you to implement all of it, @mmicko and I can look into some of it too, though any work you would do towards the plan is welcome, and would bring the flake getting merged closer.

RCoeurjoly commented 4 months ago

We have discussed this among Yosys maintainers and have come up with a plan to:

  • do commit the lock file into the repo
  • have two Nix CI jobs, one building with the version-controlled lock file and the other with the latest version of inputs
  • adapt the automation we already have for periodic "Bump version" commits to also update the lock file to a version it can pull from a successfully-completed CI job

Sounds reasonable. One problem I am encountering when trying to build yosys with flakes is the following.

[100%] Building abc/abc-896e5e7 Pulling ABC from https://github.com/YosysHQ/abc:

  • test -d abc
  • git clone https://github.com/YosysHQ/abc abc Cloning into 'abc'... fatal: unable to access 'https://github.com/YosysHQ/abc/': Could not resolve host: github.com
  • cd abc /nix/store/5l50g7kzj7v0rdhshld1vx46rf2k5lf9-bash-5.2p26/bin/bash: line 7: cd: abc: No such file or directory make: *** [Makefile:797: abc/abc-896e5e7] Error 1

This behavior from nix is expected, since nix tries to be hermetic and one of the parts of that is not accessing the network, because cloning a repo at two different time points would result in two different packages, or derivations (in nix lingo).

Options available of the top of my head:

whitequark commented 4 months ago
  • Add YosysHQ/abc as a submodule

It should be a submodule. It only isn't a submodule because abc used to be version controlled in Mercurial, which (obviously) prevented that.

I once tried to convert it to a submodule but some other horrors in the Yosys Makefile caused me to waste three days on it, after which I swore I will never attempt to improve Yosys Makefile again. I wish you luck.

povik commented 4 months ago

Options available of the top of my head:

  • Add YosysHQ/abc as a flake input

  • Add YosysHQ/abc as a submodule

While it may make sense to make abc a submodule from a conceptual sense, I think this is too much of change affecting downstream users of Yosys to couple it to the work on the Nix flake.

If abc can be made an input to the flake, I would go with that for now.

whitequark commented 4 months ago

While it may make sense to make abc a submodule from a conceptual sense, I think this is too much of change affecting downstream users of Yosys to couple it to the work on the Nix flake.

I disagree with this reasoning. I think it should have been made a submodule years ago instead of whatever the hell it is right now. The current situation has been negatively affecting users of Yosys for long enough that it should just be fixed, and the current time is as good as any.

povik commented 4 months ago

I suppose we shouldn't not try fixing the status quo because it is the status quo, but it seems valid not to want to couple this to another large task.

whitequark commented 4 months ago

@RCoeurjoly How do you feel about converting abc to a submodule?

@povik Generally speaking yes, but it's not like we're under pressure to ship flakes ASAP.

RCoeurjoly commented 4 months ago

@RCoeurjoly How do you feel about converting abc to a submodule?

@povik Generally speaking yes, but it's not like we're under pressure to ship flakes ASAP.

Yes, my strong preference would be for abc to be a submodule, because it would enable us to compile inside the devShell. Otherwise, if abc is a flake input, working inside the devShell would be very inconvenient.

whitequark commented 4 months ago

Then I'd say let's make it into a submodule. I will have to redo a bunch of my build scripts and I will do it with joy.

(I think mainly just replacing git submodule update with git submodule update -r really...)

povik commented 4 months ago

No contest here. Roland, if you want to tackle this, then please go ahead!

whitequark commented 4 months ago

I think adding abc as a submodule would be best done in another PR.

mmicko commented 4 months ago

Please be aware that use case where user downloads abc.tar.gz from release page and unpacks it in abc directory should also continue to work.

widlarizer commented 3 months ago

I've written my own shell.nix before noticing this PR. I notice some differences between it and this flake:

I have tested this with nix-shell --pure to verify it still builds yosys & docs succesfully

RCoeurjoly commented 3 months ago

I've written my own shell.nix before noticing this PR. I notice some differences between it and this flake:

  • I only use clang stdenv, I don't explicitly get pkgs.clang, which may be why I don't need libcxx (but this may just be a shell.nix vs flake thing)
  • I have some extra packages bc I just copied deps from CI or docs, namely, gawk, pkg-config, boost
  • For show to work, I have graphviz and xdot
  • To build docs, I have libfaketime pdf2svg texliveFull (python3.withPackages (ps: [ps.furo ps.sphinxcontrib-bibtex]))

I have tested this with nix-shell --pure to verify it still builds yosys & docs succesfully

How do you deal with the abc dependency? When trying to create the flake, it was decided that it was better to convert abc to a submodule.

I am working on that here https://github.com/YosysHQ/yosys/pull/4243

widlarizer commented 3 months ago

I didn't deal with it probably because I built it first with an impure shell which pulled in the dependency. I retried with a pure shell in a fresh checkout and it does indeed fail. I'm definitely in favor of a submodule