Skip to content

feat(nix): added package definitions and development environment#96

Merged
m3nu merged 6 commits into
borgbase:mainfrom
Sntx626:main
Apr 9, 2026
Merged

feat(nix): added package definitions and development environment#96
m3nu merged 6 commits into
borgbase:mainfrom
Sntx626:main

Conversation

@Sntx626
Copy link
Copy Markdown
Contributor

@Sntx626 Sntx626 commented Apr 9, 2026

This PR adds nix files as described in #95 .

CLOSES #95

Copy link
Copy Markdown
Contributor

@m3nu m3nu left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together — the overall Nix structure is solid. Crane, shared deps in .nix/common.nix, pre-fetched Skia binaries — all good choices. A few things I'd like addressed before merging:

Root directory clutter

  • .envrc — please remove. This is a personal direnv workflow file, not a project requirement. The dotenv_if_exists .env line is also unrelated to Nix. Happy to mention use flake in CONTRIBUTING docs instead, but it shouldn't be committed.
  • shell.nix — also prefer to remove. The builtins.getFlake shim is impure and won't work with nix-shell --pure. Anyone on flakes already has nix develop.
  • flake.nix + flake.lock at root are fine — understood that's a hard flake requirement.

Technical questions

  • No x86_64-darwin in eachSystem — intentional? The CLI and server packages don't need Skia, so Intel Mac support would be cheap to add.
  • pkgs.lib.maintainers.sntx in package meta — is this entry in nixpkgs? If not, evaluation will fail.
  • curl and cacert in GUI nativeBuildInputs — since SKIA_BINARIES_URL uses file://, are these actually needed? If so, does the build still try to fetch something (which would break the sandbox)?
  • Skia binary version coupling — the pinned 0.90.0 matches current Cargo.lock, but every skia-bindings bump will require updating 3 URLs + hashes. Could you add a comment in flake.nix documenting the update procedure?

@Sntx626
Copy link
Copy Markdown
Contributor Author

Sntx626 commented Apr 9, 2026

I'll address the issues as follows:

  • .envrc removed, added to gitignore and CONTRIBUTING docs for nix users
  • shell.nix, I'm going to push back on this. We can remove the shell.nix, but this would stop nix legacy (non flakes) users from utilizing the shell. Instead I'm going to update the shell.nix to work without flakes (which is a current issue). Please let me know if you want it removed entirely anyway.
  • No x86_64-darwin in eachSystem this is intentional, x86_64-darwin is marked as deprecated by nixpkgs and will be removed after the next stable release
  • pkgs.lib.maintainers.sntx in package meta, yes that's me. it's present in nixpkgs.
  • curl and cacert in GUI nativeBuildInputs, yes since the skia build.rs tries to download the latest skia bins. Since nix has sandboxed builders, we need to provide it ourselves
  • curl and cacert are still used by the build.rs
  • Skia binary version coupling is valid, I'll add comments

Do you want seperate commits, or everything squashed?

@m3nu
Copy link
Copy Markdown
Contributor

m3nu commented Apr 9, 2026

Thanks for the detailed response. Agreeing on most points:

  • .envrc removal + gitignore/docs — sounds good.
  • shell.nix — fair pushback, keeping a proper non-flake version makes sense. The current builtins.getFlake shim is the issue, not shell.nix itself.
  • x86_64-darwin — confirmed, nixpkgs is deprecating it in 26.05 with removal in 26.11. No objection.
  • Skia version coupling comments — appreciated.

Two things that still need attention:

pkgs.lib.maintainers.sntx — I searched maintainer-list.nix on nixpkgs master and got zero matches. If that entry doesn't exist, meta.maintainers will fail on evaluation. You'd need to either land a maintainers PR on nixpkgs first, or drop the field from these package definitions for now.

curl/cacert in GUI nativeBuildInputs — looking at the source, utils::download() has an early return for file:// URLs that reads via std::fs::read directly — curl is never invoked on that path. That said, download.rs has a separate resolve_dependencies() that fetches the Skia source from codeload.github.com via the same function with an https URL, so curl might still be needed depending on which build paths are hit. Have you tried building the GUI package with curl and cacert removed from nativeBuildInputs?

Re commits: either squashed or individual is fine — I can squash on merge.

@Sntx626
Copy link
Copy Markdown
Contributor Author

Sntx626 commented Apr 9, 2026

The commits should've resolved all outstanding issues.

I have confirmed the build working for all (x86_64-linux - since that's what I can test) outputs, including the gui app.

Are we ready to merge?

@m3nu m3nu merged commit a832c7e into borgbase:main Apr 9, 2026
@m3nu
Copy link
Copy Markdown
Contributor

m3nu commented Apr 9, 2026

Thanks for the changes and patience! Merged! Thanks for the contribution!

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.

packaging vykar with nix

2 participants