Skip to content

RFC: Use nix only for tools #156

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

jfly
Copy link
Member

@jfly jfly commented May 2, 2025

This builds upon #154.

This allows us to (impurely) use bun to install dependencies, and
(hopefully) not depend on node. As we're now using bun to install
dependencies, we can get rid of the package-lock.json workaround and
use bun.lock instead. All our various make commands are the source
of truth for both local development and CI. Folks can use nix develop
to get a development shell with all deps pre-installed, or they can
install those tools (just make and bun) some other way, at the risk
of using a different version of those tool than we use in CI.

There's some weirdness here to make nix fmt defer to make format.
make format didn't support formatting individual files, so I had to
add a little bit of janky python+shell glue to make it work. We can
revert this, I just wanted a smooth experience for nix-ers who have gone
to the effort to configure their editor to automatically run nix fmt.

jfly added 3 commits May 2, 2025 13:07
This allows us to (impurely) use bun to install dependencies, and
(hopefully) not depend on node. As we're now using bun to install
dependencies, we can get rid of the `package-lock.json` workaround and
use `bun.lock` instead. All our various `make` commands are the source
of truth for both local development and CI. Folks can use `nix develop`
to get a development shell with all deps pre-installed, or they can
install those tools (just `make` and `bun`) some other way, at the risk
of using a different version of those tool than we use in CI.

There's some weirdness here to make `nix fmt` defer to `make format`.
`make format` didn't support formatting individual files, so I had to
add a little bit of janky python+shell glue to make it work. We can
revert this, I just wanted a smooth experience for nix-ers who have gone
to the effort to configure their editor to automatically run `nix fmt`.
@lgarron
Copy link
Member

lgarron commented May 3, 2025

I appreciate the effort you've put into trying to get nix working for us, but I think at this point I'm pretty reluctant on this approach on practical grounds.

I think the PR title illustrates this: even when trying to use nix "only" for tools it still requires wrapping all the make commands in the workflows.

By contrast, the abstraction we use for other cubing projects is really low compared to other possibilities. For example, when the ttf2woff2 issue occurred, this was the entirety of the GitHub actions output:

clipboard

This shows:

  • The error occurred when running make build-lib.
  • The error occurred immediately after bun run script/build-lib.ts was invoked.

It took me almost no time to figure out what the issue was and how it's triggered. We invoke a make target, it invokes bun, and we get the output of running the code. I think it takes some prior experience to understand the likely meaning of symbol lookup error (and in this case I obviously couldn't reproduce it locally), but if it was a simple logic error I don't think the experience could be any more straightforward.

(As for the actual issue in that case, it took me little time to work around. And bun has invested a lot into node compat — even the little warts. oven-sh/bun#798 is pinned for bun issues and alii has recently reiterated to me that node compat remains a high priority. So I don't expect this to be particularly common thing to have to debug. And if we do have to refactor our toolchain, we start from a point of low abstraction.)

By contrast, consider something like https://github.com/cubing/icons/actions/runs/14323570832/job/40144925277

  • The command is an inscrutable nix command which I can't run locally. This PR is a great improvement in that it splits out the make commands and contains less boilerplate, but I think it is preferable for the command to be "actual" command that failed (i.e. just the make invocation).
  • The failing command has 1268 lines of output. Turns out I could probably have figured it out, and it identified a real consistency issue. But after a minute or two of trying to scroll through it I was pretty scard off.
  • To understand how the project is set up requires poking at multiple configuration files in a custom language. It's not too hard to get an idea of what they're doing, but again it's another layer of abstraction and I feel like I'd be learning a programming language from scratch if I had to modify them.

In addition, it's not easy for me to run nix locally and trying to use nix in codespaces is a gnarly time, which I think is a condition that extends to most others. Now, nix is clearly a superior mechanism to dev containers. I would say it's more accurate the lay the blame at the feet of codespaces for making proprietary decisions that don't encourage reproducibility enough. But it remains that codespaces remain a far more accessible way to get a consistent environment (two clicks and you can run make successfully a minute later). I would really love if codespaces and GH Actions integrated with the kind of functionality nix has (or were even consistent with each other), but at this point adding nix into the mix adds layers of abstraction and debugging. I don't mind having config files for it in the repo, but I don't see myself using or maintaining them unless something in the ecosystem shifts.

So overall I'm not necessarily opposed to nix, but at this point I'd strongly prefer #154 unless the ecosystem has made reproducibility significantly more ergonomic.

@jfly
Copy link
Member Author

jfly commented May 3, 2025

I'd strongly prefer #154 unless the ecosystem has made reproducibility significantly more ergonomic.

I don't see this as an either or. Let's merge #154. This just gets the tools in a different way.

By contrast, consider something like https://github.com/cubing/icons/actions/runs/14323570832/job/40144925277

I'm not going to try to defend that. I agree it's inscrutable. It's also not the sort of thing we're going to get with the approach in this PR, though. We're now letting bun do bun things.

Would it be helpful if I intentionally re-introduce the bug from #152 so we can see what it looks like in CI now?


For the record, I'm not going to die on this hill, but I do want to provide a nix devshell. I would like it if it CI uses that devshell.

@lgarron
Copy link
Member

lgarron commented May 3, 2025

I'd strongly prefer #154 unless the ecosystem has made reproducibility significantly more ergonomic.

I don't see this as an either or. Let's merge #154. This just gets the tools in a different way.

Sounds good, I'll merge!

For the record, I'm not going to die on this hill, but I do want to provide a nix devshell. I would like it if it CI uses that devshell.

Yeah, I'll leave all the nix files as-is for now.

Base automatically changed from normal-ci to main May 3, 2025 21:08
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.

2 participants