@rht, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rvl to be a potential reviewer.

Hi @rht, thanks for the PR. I don't know if this change is for the better or not. I also don't know whether an auto-generated all-packages.nix will be better or not.

I see. I have checked that both are used extensively.

$ ag buildinput | ag '\[.*packages' | wc -l
139
$ ag inherit | ag 'packages\)' | wc -l
206

With .*Packages\.packagename in inherit being used 1.5x more often.

Thank you for your contribution. For the time being, I don't know of any plans to make all-packages.nix auto-generatable. I think both versions (old a new) of this code are acceptable, so I will close this for now as I believe that the current version is acceptable, this PR has had no activity for a long time and now has merge conflicts. If you feel like I have missed an important issue here, please feel free to re-open this pull request.

Wait, what? I am still around to discuss. I haven't even run this through all-packages.nix yet. It's a choice between standardization (enforce one scoping method to avoid future confusion) and allowing all the options in (submodule, inherit, ...).

@rht I agree that having a single style would be great. However:

• historically, it's been quite hard to get large scale sweeping changes merged
• i personally think that the gain here is quite minor. We could add documentation about a "preferred" style though, to make adding new packages easier.

Or is there some huge benefit this change brings that I overlooked?

historically, it's been quite hard to get large scale sweeping changes merged

This is solved with breaking the one monolithic PR into manageable pieces.

i personally think that the gain here is quite minor. We could add documentation about a "preferred" style though, to make adding new packages easier.

This 'field' has been solved through linters, e.g. gofmt, or several standards in js (due to the scale and scope of the .nix language, I think there will be just one for a long time). I have been able to figure out the "preferred" style through reading how the core packages are written, or packages that have been heavily patched, or via statistics like in #21906 (comment).
More than 'adding new packages easier', the process is to be leveraged to machines.

@rht ok. I actually prefer the old way, as it makes it more obvious what packages a certain package depends on. Your statistics seems to show that both are used about the same amount.

I don't have any hard opinions on this and I also don't feel like I am in the position to decide this one.

Reopening for further comments.

Another reason to prefer the variant with explictly passing the argument in all-packages.nix instead of passing the set is that it makes overriding easier. Compare:

fetchBower.override {
  bower2nix = ...
}

to

fetchBower.override (old: {
  nodePackages = old.nodePackages // {
    bower2nix = ...
  };
})

Although the second variant is probably better if you need a "transitive" override, where all the other packages taken from nodePackages by fetchBower use the new bower2nix as well.

The usage for the first variant is harder to capture, \.override { is used ~660 times, most usage is for changing some params, but can't be further narrowed down for changing the version of a package.

The latter, = old\. 40 times, mostly in development/r-modules/default.nix, but are mostly for changing params than changing a module version, e.g.

BayesXsrc = old.BayesXsrc.overrideDerivation (attrs: {
      patches = [ ./patches/BayesXsrc.patch ];
    });

Although the second variant is probably better if you need a "transitive" override, where all the other packages taken from nodePackages by fetchBower use the new bower2nix as well.

Did you mean here is no way to ensure this with the first variant? It is counterintuitive, given that bower2nix is specified at all-packages.nix. I have gone through the ~40 ag result, none is "transitive" override.

e: s/grep/ag/

I prefer to be specific about dependencies, so think it's fine as it is.

It seems that consistency is reinforced only within each language ecosystem.

Though globally,

$ ag buildinput | ag '\[.*packages' | wc -l
222
$ ag inherit | ag 'packages\)' | wc -l
241

the former has caught up in usage.

Are there any updates on this pull request, please?

This is just a stylistic change, and doesn't seem to have consensus for change, so leaving it as is seems like the best solution for now. If we want to refactor this on a greater scale, this might be suitable for https://github.com/NixOS/rfcs.

If anyone feels deeply about this, feel free to reopen.