Hmm, not quite sure I see the benefit here but no strong objections either.

Yeah, the benefit is limited, but in cases of the function fixedWidthString which yes does belong in strings.nix but no doesn't have any business being "exported" by default to lib.fixedWidthString. This, and fewer important statements seems like all the benefit there is. Pretty anemic set of reasons.

I mostly put it up in case someone else had a convincing reason :)

If a function shouldn't be available from outside, isn't it better to put it inside of a let?

An example is lib.strings.fixedWidthString has every right to exist and be usable externally, but has no business being hoisted up to lib.fixedWidthString. This is the spirit of

!!! don't include everything at top-level; perhaps only the most
commonly used functions.

but the reality is that comment is impossible to accomplish with the current structure.

@aszlig I added a commit where I tried your suggestion of moving the inherits to the bottom.

👍 after clarification on IRC what this PR actually improves:

gchristensen: a concrete example is lib.strings.tail exists because of https://github.com/NixOS/nixpkgs/pull/27797/files#diff-c6f540a4f3bfa4b0e8b6bafd4cd54e8bR11
gchristensen: another gross thing is types.nix has types = {} inside it

On IRC, @edolstra said that the massive list of inherits is ugly, but probably unavoidable due to the lack of laziness in with lib. I clarified with him and he's okay with this change and the ugliness isn't a blocking problem for the merge.

Unless there is further feedback, I'm planning on merging this later today or tomorrow.

@fpletz what sort of docs and changelog would you like to see?

@grahamc I haven't looked at the manuals yet but this change at least deserves a paragraph in the nixpkgs manual. We should also mention this in the NixOS 17.09 release notes.

I suppose I'm having a hard time writing the docs because not much has changed. Maybe:

It is now required to import the top level lib library instead of individual library sets. Before you could import <nixpkgs/lib/attrsets.nix> and you must now (import <nixpkgs/lib>).attrsets.

I've used the following to verify the same attributes are being exported on lib:

[nix-shell:~/projects/nixpkgs]$ git checkout fixed-lib
Already on 'fixed-lib'

[nix-shell:~/projects/nixpkgs]$ echo "import ./lib" | nix-repl 2>&1 | tr ' ' $'\n' | sort | uniq > ../fixed-lib

[nix-shell:~/projects/nixpkgs]$ git checkout origin/master
Note: checking out 'origin/master'.
HEAD is now at 87b215d5f72... android-studio-preview: 3.0.0.7 -> 3.0.0.8

[nix-shell:~/projects/nixpkgs]$ echo "import ./lib" | nix-repl 2>&1 | tr ' ' $'\n' | sort | uniq > ../master

[nix-shell:~/projects/nixpkgs]$ diff ../fixed-lib ../master

[nix-shell:~/projects/nixpkgs]$ 

unfortunately this isn't recursive, looks like I have more testing to do:

10,11d9
<  defaultFunctor = «lambda»
<  defaultTypeMerge = «lambda»
16,17d13
<  isOptionType = «lambda»
<  isType = «lambda»
25d20
<  mkOptionType = «lambda»
31d25
<  setType = «lambda»
36d29
<  types = { ... }

on types.

An update, and good news!

Using the test script (later in this commenet) I got the following diff
on the API:

  + diff master fixed-lib
  11764a11765,11766
  > .types.defaultFunctor
  > .types.defaultTypeMerge
  11774a11777,11778
  > .types.isOptionType
  > .types.isType
  11781a11786
  > .types.mkOptionType
  11788a11794
  > .types.setType
  11795a11802
  > .types.types

This means that this commit adds to the API, however I can't find a
way to fix these last remaining discrepancies. At least none are
removed.

The test script recursively explores attribute keys (with crude loop prevention) and produces the same diff for 10 recursions (though the pasted script does 4).

Test script (run with nix-repl in the PATH):

  #!/bin/sh

  set -eux

  repl() {
      suff=${1:-}
      echo "(import ./lib)$suff" \
          | nix-repl 2>&1
  }

  attrs_to_check() {
      repl "${1:-}" \
          | tr ';'  $'\n' \
          | grep "\.\.\." \
          | cut -d' ' -f2 \
          | sed -e "s/^/${1:-}./" \
          | sort
  }

  summ() {
      repl "${1:-}" \
          | tr ' ' $'\n' \
          | sort \
          | uniq
  }

  deep_summ() {
      suff="${1:-}"
      depth="${2:-4}"
      depth=$((depth - 1))
      summ "$suff"

      for attr in $(attrs_to_check "$suff" | grep -v "types.types"); do
          if [ $depth -eq 0 ]; then
              summ "$attr" | sed -e "s/^/$attr./"
          else
              deep_summ "$attr" "$depth" | sed -e "s/^/$attr./"
          fi
      done
  }

  (
      cd nixpkgs

      #git add .
      #git commit -m "Auto-commit, sorry" || true
      git checkout fixed-lib
      deep_summ > ../fixed-lib
      git checkout master
      deep_summ > ../master
  )

  if diff master fixed-lib; then
      echo "SHALLOW MATCH!"
  fi

  (
      cd nixpkgs
      git checkout fixed-lib
      repl .types
  )

Heads up: I'm planning on merging this on Tuesday unless someone asks me not to :)

Final warning: I'll be merging this in between 12 and 24 hours.