Could you bring these into mkDerivation for consistency with other derivations?

The reason I put it outside so that mkDerivation doesn't need rec. Which other derivations are you talking about?

According to this approximate count, most of them:

$ ag -G '\.nix$' -l '(?s)version =.*mkDerivation' | wc -l
678
$ ag -G '\.nix$' -l '(?s)mkDerivation rec .*version =' | wc -l
3494

and, in particular, the previous PR #29886. There is no need to remove rec from mkDerivation.

But wouldn't you say it's better without rec? I'm bit hesitant because this would mean it's inconsistent with most other guile-modules that do not need to use rec: 3579d7e

I guess my point is why use rec when it's not needed?

#29886 doesn't remove rec because it needs it.

I think that in general these attrs are put into mkDerivation because rec version is shorter, and they can be used from other derivations — for example, grep .version} in the repository.

Oh that seems really useful, I didn't think about that.

So I adopted this style after seeing it somewhere, mainly just because of version and how putting it in let can remove the need for rec in a lot of cases. And otherwise if this was not done, rec seemed to nearly always be needed, which didn't seem right.

Anyway in that case I'll switch to setting version too in mkDerivation, thanks for mentioning this. :)

I do not have strong feelings about the rec but just so you know: there is a function called builtins.parseDrvName to get version info from derivation name. Which makes putting version into the derivation a second time not necessary. Because parseDrvName should work with far more derivations than asking for a version attribute using the builtin function would be more desirable anyway, at least in my opinion.

@seppeljordan Oh right that's even better, thanks for letting me know. :)

This does not appear to do anything.

It just gets rid of compilation warnings/errors such as (WARNING, ERROR, etc):

;;; note: auto-compilation is enabled, set GUILE_AUTO_COMPILE=0
;;;       or pass the --no-auto-compile argument to disable.
;;; compiling /nix/store/1djqc42mqb1hacj40228cm2wq50i8c1d-guile-2.2.0/bin/guild
;;; WARNING: compilation of /nix/store/1djqc42mqb1hacj40228cm2wq50i8c1d-guile-2.2.0/bin/guild failed:
;;; ERROR: failed to create path for auto-compiled file "/nix/store/1djqc42mqb1hacj40228cm2wq50i8c1d-guile-2.2.0/bin/guild"
  GEN      sdl2/config.go
wrote `sdl2/config.go'
;;; note: auto-compilation is enabled, set GUILE_AUTO_COMPILE=0
;;;       or pass the --no-auto-compile argument to disable.
;;; compiling /nix/store/1djqc42mqb1hacj40228cm2wq50i8c1d-guile-2.2.0/bin/guild
;;; WARNING: compilation of /nix/store/1djqc42mqb1hacj40228cm2wq50i8c1d-guile-2.2.0/bin/guild failed:
;;; ERROR: failed to create path for auto-compiled file "/nix/store/1djqc42mqb1hacj40228cm2wq50i8c1d-guile-2.2.0/bin/guild"

I can still remove it though if you'd like?

Like it prevents Guile from doing unnecessary things so it's useful to an extent, in my opinion, but idm not having it either.

You are right, I did not notice this because there were no warnings when building in a terminal.

Oh I was building in a terminal too (not emacs or anything if that's what you mean?), strange. 😕