no need to create an environment variable, can just substitute it directly in preBuild

There is a need. You can now easily overrideAttrs it.

If it is intended to be overridable, then make it a parameter of the function. overrideAttrs should be avoided because it overrides the low-level stdenv.mkDerivation.

@FRidh We certainly should not try to maintain options for every case, and, in fact, we don't even provide options for most common overrides, otherwise we would have functions defined like { stdenv, pname ? "matplotlib", version ? "2.3.1", src ? fetchurl { ... }, ... }:. I thought, the accepted solution to this was that we are not standing in the way of the "low-level" plumbing and thus users can still override src. And I was sticking with this approach to defining my derivations and suggesting it when reviewing other PR's. This is the first time I hear that overrideAttrs should be avoided, because we don't really need overhead of overrideDerivation or overridePythonAttrs or a higher level functions in this particular case.

Just in case, I will try to present my original point again. If I have a derivation:

let
  foo = ./bar;
in
stdenv.mkDerivation {
  fooPhase = ''
    # ...
    cp ${foo} foo
    # ...
  '';
}

I am unable to do a lot of things, like easily access the value of foo (unless I use <nixpkgs/path/to/bar>, which is not overlay-friendly), or easily exchange it for something else (for example I'm not even sure if builtins.replaceStrings will discard the context of old value of foo). This is what we loose when we do it this way, but what do we gain for that?

I see your point. We should indeed not make options for everything. The issue at least with overrideAttrs (and worse, overrideDerivation) is that overriding attributes may not always work as intended. It is this issue, not being able to tell whether it works or not without inspecting how buildPython* works, which is why I say, do not use overrideAttrs.

Now, in this case it indeed would work fine because neither buildPython* nor stdenv.mkDerivation does anything with this specific variable. But, if this is deemed something that you want to offer that should work, I'd argue it should be part of the function interface where it is created. Does it happen often people create variables like these so they can override it later?

But, if this is deemed something that you want to offer that should work, I'd argue it should be part of the function interface where it is created.

I'm having trouble understanding you here. Could you, maybe, rephrase?

Does it happen often people create variables like these so they can override it later?

People, who use overrides, will likely be able to quickly learn which kinds of expressions are easy to override and which are not. Once they have learned the good patterns, they will employ them for their contributions to nixpkgs. When I review PR's, I make suggestions based on my experience doing overrides to improve this aspect. Of course, there are many contributions out there that go against my "best practice", but that doesn't immediately invalidate my effort. And, yes, I saw there are few other people that do put $variables in their phase overrides.

I thought I had replied but apparently not.

I'm having trouble understanding you here. Could you, maybe, rephrase?

The $setup_cfg variable is specific to matplotlib, not to stdenv.mkDerivation or buildPythonPackage. Put differently, it's a parameter of the matplotlib builder, and not of stdenv.mkDerivation or buildPythonPackage.

Ok, now I see what the point was.

The setup_cfg is not a part of an interface, but a part of implementation, just like XDG_RUNTIME_DIR. But we want the implementation to be conviniently overridable when we do "crazy" things (like build on a system without /tmp, or adding more stuff to setup_cfg).