Document attribute ordering in package expressions

Anyone have any tips to explain this in the coding conventions documentation?
Look at this expression https://github.com/NixOS/nixpkgs/blob/4107aa98ddedae87b8b483d91ba1f05d3fe93248/pkgs/desktops/pantheon/granite/default.nix, notice how I’ve separated most of the attributes with whitespace except pname and version. Also notice how all the expressions there, and especially in python-modules, have a very predictable hierarchy that actually has no meaning.

The hierarchy almost always looks like the following, and I even have a vscode snippet/template I use to write new expressions

{ stdenv 
, fetchurl
, fetchpatch
}:

stdenv.mkDerivation rec {
  pname = "";
  version = "";

  src = fetchurl {
    url = "";
    sha256 = stdenv.lib.fakeSha256;
  };

  # I do this because of the associativity with src
  patches = [
    (fetchpatch {
    })
  ];

  # Inputs
  # Yes, there's definitely more than these, but I generally think of this.
  nativeBuildInputs = [
  ];

  buildInputs = [ 
  ];

  propagatedNativeBuildInputs = [
  ];

  propagatedBuildInputs = [
  ];

  checkInputs = [
  ]

  installCheckInputs = [
  ];

  # configuring flags
  configureFlags = [
  ];
  
  cmakeFlags = [
  ];

  # do*something attrs
  doCheck = true;
  doInstallCheck = true;
  
  # dont attrs
  dontBuild = true;

  # Phases attrs are ordered exactly as they're executed in setup.sh
  # Seen at:
  # * https://github.com/NixOS/nixpkgs/blob/18f47ecbac1066b388e11dfa12617b557abeaf66/pkgs/stdenv/generic/setup.sh#L1261
  # unpackPhase 
  # patchPhase 
  # ${preConfigurePhases:-}
  # configurePhase 
  # ${preBuildPhases:-} 
  # buildPhase 
  # checkPhase
  # ${preInstallPhases:-} 
  # installPhase 
  # ${preFixupPhases:-} 
  # fixupPhase 
  # installCheckPhase
  # ${preDistPhases:-} 
  # distPhase 
  # ${postPhases:-}

  # Metadata
  passthru = {
  };

  meta = {
    # I don't separate these with whitespace
    # I don't think ordering matters here at all, it's metadata.
    # But my template makes it so these attributes are usually present in that order.
    description = "";
    # Kinda optional
    longDescription = ''
    '';
    homepage = "";
    license = licenses.
    platforms = platforms.
    maintainers = [];
  };
}

When it comes to my peers, we’ve basically spread this around via word of mouth, and to contributors they usually look at the expressions and adopt this “style”.
@jtojnar and I have actually adopted the same practices. Expressions we write get formatted with nixpkgs-fmt and all listitems are one per line, and function arguments are the same.

All of this does sound a bit ridiculous to me, but things seem to happen like this. It’s just usually unsaid.

Thoughts? I’d probably just add the suggestions with an example similar to this, since it’s probably best expressed this way.

5 Likes

Related:

1 Like

I like having consistent ordering so I can quickly locate the things I need to change. We should also mention rationales for the more controversial parts – for example, the one per line rule is to make git diffs, as well as merges cleaner.

I think adding that example should be enough, people are pretty good at pattern matching. And adding an opinionated second level to nixkpgs-fmt would help a lot. Ideally, people should not have to think about this much.

Regarding the actual ordering, I like to order the attributes in meta alphabetically. Thankfully, it keeps the common one reasonably ordered (project info on top, nixpkgs-only stuff on the bottom, though the platforms kind of belong to the project too, now that I am thinking about it).

Few more things we might want to show in the example:

  • fetchFromGitHub (though the ordering of arguments is pretty intuitive)

  • environment variables like NIX_CXX_FLAGS (__structuredAttrs will mandate placing them into env attribute)

For sure will mention special rationales.

I’d also be nice if people can even copy and paste it too.
I think by design, nixpkgs-fmt doesn’t do a lot of strict things like this

But as a nixpkgs-committer, I already request these things just for package expressions.

It’s good that you mention alphabetization, because I actually do this for all *Inputs as well.
I think it can seem a bit excessive, but my editor has an extension which makes it pretty convenient.
Will adopt that for meta attributes.

I strictly follow this for the following fetchers

What I think we can do there is just add examples in

and people can pick up on this. If they read an expression they’ll be the same anyways.

As for the env attribute, I’m seeing this in the actual PR

env.NIX_CFLAGS_COMPILE

I’d see things looking like either

env.ONE_VAR = "a_value";
env.TWO_VAR = "another_value";
env.YOU_GET_THE_IDEA = "it_goes_on";

or

env = {
 ONE_VAR = "a_value";
 TWO_VAR = "another_value";
 YOU_GET_THE_IDEA = "it_goes_on";
};

The first looks better to me, because it’ll probably be seldom used.
(or just one entry).

Heirarchy-wise, before meta and passthru sounds good.

1 Like

My own personal habits differ from the convention you describe. I tend to group attributes in paragraphs related to the different phases. src, then unpack, then patch, etc. This makes it easy to follow the implicit flow of execution.

For example, configureFlags should be close to preConfigure = '' configureFlagsArray+=...'', and cmakeFlags should come before postFixup. Otherwise you need to go back and forth to find all the fragments that define how a given phase behaves.

I am however always uneasy with nativeBuildInputs, as these relate to all the phases. I tend to directly and explicitly antiquote them when possible.

From my experience, this ordering is also used in other linux distros like arch, debian or opensuse.

2 Likes

As I thought deeper about it and read through the phases documentation and *inputs, I thought of the same thing when it comes to ordering. If we order in paragraphs according to phases src is associated with unpackPhase and configureFlags is associated with configurePhase.

so then the layout seems like

stdenv.mkDerivation rec {
  pname = "";
  version = "";

  # Variables controlling dependencies.
  buildInputs = [
  ];

  # Stdenv Phases and variables that control them in order of execution.
  src = fetchurl {
  };

  postPatch = ''
  '';

  postInstall = ''
  ''; 

  # Metadata
  passthru = [
  };

  meta = {
  };
};

:grin: So I then think the example should be a real example and then a summary of the ordering conventions.

1 Like

Generally I follow: high-level derivation info (pname, version, [disabled]), order they are evaluated (src), then order they are used by builder.sh (inputs then phases), and meta last.

With exception of some the very old expressions, most of them follow the ordering in your first post.

It might be nice to also have some conventions on situational attributes such as: enableParallelBuilding, cmakeDir, __darwinAllowLocalNetworking, sourceRoot, etc

As @layus, my habit is to put flags and do/dont switches with the corresponding phase.

For example:

  [... build things ...]

  doCheck = true;
  checkTarget = "test";

  [... install things ...]

I would find it weird by now if src were not the 3rd attribute, just below pname and version. Out of habit but also because it’s usually the place where pname and version are used.

And additionally, the source of a package is intuitively, for me, one of the first things I want to specify. Interestingly though, Arch PKGBUILDs don’t follow this pattern and have source in the middle. So maybe this argument is not so strong.

I also like to put outputs right after pname and version as they end up in the store path as well.

Also, because many version bumps only require updating version and src, it is nice to have them together.

1 Like

I think I can agree with this. The first portion can just be those common attributes you want to know about the expression pretty much immediately, the high level derivation info, and then the order as you described. I also agree with @c0bw3b that the phases attrs if present should come first followed by any variable that controls the aforementioned phase/s.

1 Like

See also https://github.com/jtojnar/nixpkgs-hammering/blob/0c54b7121a21f79e580773a46fc8d51482dc1d1c/explanations/attribute-ordering.md for a more complete list of attributes and a tool that can check it.

1 Like