Skip to content

Conversation

Naxdy
Copy link
Contributor

@Naxdy Naxdy commented Oct 15, 2025

flake-utils is currently only used for outputting system-specific dev shells. This can actually be achieved only using functionality already present within nixpkgs, thus there is no need for an extra dependency.

Additionally, we move to use the packages and env args for mkShell to more clearly outline what they are used for.


Further reading: https://determinate.systems/blog/best-practices-for-nix-at-work/#avoid-flake-helper-libraries-if-possible

As a side note, using with to import large scopes is discouraged by official Nix resources, so an alternative approach to list installed packages could be something like this:

packages =
  (builtins.attrValues {
    inherit (pkgs)
      # generic
      git
      git-lfs
      gnumake
      gnused
      gnutar
      gzip
      zip

      # frontend
      cairo
      pixman
      pkg-config

      # linting
      uv

      # backend
      gofumpt
      sqlite
      ;

    inherit
      # frontend
      nodejs
      pnpm

      # linting
      python3

      # backend
      go
      ;
  })
  ++ linuxOnlyInputs;

But I saw this as too pedantic to include in the initial PR.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 15, 2025
@techknowlogick techknowlogick requested a review from 6543 October 17, 2025 04:56
@techknowlogick techknowlogick added the dependencies Pull requests that update a dependency file label Oct 17, 2025
@6543
Copy link
Member

6543 commented Oct 19, 2025

i dont see inputs as bad as determined systems here ... because i see no reason for somebody to use this flake as input ...

but as less dpes are better and if the flake e.g. learns to build gitea ... it is likely going to be used esternally ... so no blocker on my side ;)

Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

the env={} i was going to do that ... thanks for doint it now

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Oct 19, 2025
@Naxdy
Copy link
Contributor Author

Naxdy commented Oct 19, 2025

@6543 Thanks for the feedback. I do have a private fork with a build script for gitea (including frontend + pnpm working). I was assuming there wasn't any interest in having one here since you already have a different build process, but if you want I can open a PR and work to upstream it?

It does require updating dependency hashes for go & pnpm deps on package updates however.

@techknowlogick
Copy link
Member

@Naxdy maybe not in this repo, but the pnpm build would be very appreciated in nixpkgs when 1.25 comes out

@techknowlogick
Copy link
Member

@Naxdy sorry, I should've said: could you share the nix build you have, as then I could port it to nixpkgs when the 1.25 release comes out for you. Nix is optional to build this repo, and we don't want to make it a hard requirement for contributors, but as youve already done the heavy lifting for pnpm that's wonderful as that's work that we don't need to replication. Thanks again:)

@Naxdy
Copy link
Contributor Author

Naxdy commented Oct 19, 2025

@techknowlogick I pushed my derivation to my nixpkgs fork here: Naxdy/nixpkgs@ba04041

Feel free to take what you want from it; you can also ping me on the nixpkgs PR once it's up :)

PS: You'll probably want to pin nodejs and/or pnpm, as is convention in nixpkgs, i.e. nodejs_22 instead of nodejs, pnpm_9 instead of pnpm or something similar.

@6543 6543 requested a review from techknowlogick October 20, 2025 13:26
@techknowlogick
Copy link
Member

@Naxdy 🙏💕 thank you so much!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file lgtm/need 1 This PR needs approval from one additional maintainer to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants