Skip to content

Streaming toPretty#98761

Closed
infinisil wants to merge 6 commits intoNixOS:masterfrom
infinisil:streaming-toPretty-clean
Closed

Streaming toPretty#98761
infinisil wants to merge 6 commits intoNixOS:masterfrom
infinisil:streaming-toPretty-clean

Conversation

@infinisil
Copy link
Member

Motivation for this change

This changes the implementation of lib.generators.toPretty significantly

Output streaming

Adds the ability to stream its output in a generic way. This can be done using the new initialState and nextState arguments, which thread some arbitrary state through the execution. nextState is a function that takes two arguments:

  • line: A string, this is the line that was generated and should be processed
  • state: The state as returned by the previous invocation of this function (from the previous line, or initialState for the first invocation)

The function should then return the new state, incorporating the given line into it. The function may also abort execution by returning an attribute set with a return attribute.

The result of the toPretty call will be the last state.

Examples:

  • tracing lines:
    {
      nextState = builtins.trace;
    }
  • tracing only 10 lines:
    {
      nextState = line: left:
        if left == 0 then { return = null; }
        else builtins.trace line (left - 1);
      initialState = 10;
    }
  • collecting a list of lines:
    {
      nextState = line: list: list ++ [ line ];
      initialState = [];
    }
  • Tracing lines but trimmed to 80 chars wide
    {
      nextState = line: builtins.trace (builtins.substring 0 80 line);
    }

Limiting recursion depth

Allows the function to stop recursing at a certain depth using the recursionLimit argument. Attribute sets that aren't recursed into are shown as { <N attributes> }, while lists are shown as [ <N elements> ]. The number of items is shown because that information is available without recursing.

Handling failing values

Values that fail to evaluate are shown as <failure>. This was a bit tricky to get right, but the implementation here should be very solid.

Printing of string-coercible attribute sets

Attribute sets that have an outPath or __toString attribute are now printed by using toString on it.

Ping @Profpatsch @edolstra

Things done

  • Write tests

@infinisil infinisil changed the title Streaming to pretty clean Streaming toPretty Sep 25, 2020
@infinisil
Copy link
Member Author

As an example, the following call:

toPretty {
  recursionLimit = 2;
  nextState = line: left:
    if left == 0 then { return = null; }
    else builtins.trace line (left - 1);
  initialState = 300;
} pkgs.lib

when evaluated using Nix master (for this fix), prints the following within a fraction of a second:

Details
trace: {
trace:   __unfix__ = <function>;
trace:   add = <function>;
trace:   addContextFrom = <function>;
trace:   addErrorContext = <function>;
trace:   addErrorContextToAttrs = <function>;
trace:   addMetaAttrs = <function>;
trace:   all = <function>;
trace:   and = <function>;
trace:   any = <function>;
trace:   appendToName = <function>;
trace:   applyIfFunction = <function>;
trace:   assertMsg = <function>;
trace:   assertOneOf = <function>;
trace:   asserts = {
trace:     assertMsg = <function>;
trace:     assertOneOf = <function>;
trace:   };
trace:   attrByPath = <function>;
trace:   attrNames = <function>;
trace:   attrNamesToStr = <function>;
trace:   attrVals = <function>;
trace:   attrValues = <function>;
trace:   attrsets = {
trace:     attrByPath = <function>;
trace:     attrNames = <function>;
trace:     attrVals = <function>;
trace:     attrValues = <function>;
trace:     catAttrs = <function>;
trace:     chooseDevOutputs = <function>;
trace:     collect = <function>;
trace:     dontRecurseIntoAttrs = <function>;
trace:     filterAttrs = <function>;
trace:     filterAttrsRecursive = <function>;
trace:     foldAttrs = <function>;
trace:     genAttrs = <function>;
trace:     getAttr = <function>;
trace:     getAttrFromPath = <function>;
trace:     getAttrs = <function>;
trace:     getBin = <function>;
trace:     getDev = <function>;
trace:     getLib = <function>;
trace:     getMan = <function>;
trace:     getOutput = <function>;
trace:     hasAttr = <function>;
trace:     hasAttrByPath = <function>;
trace:     isAttrs = <function>;
trace:     isDerivation = <function>;
trace:     listToAttrs = <function>;
trace:     mapAttrs = <function>;
trace:     mapAttrs' = <function>;
trace:     mapAttrsRecursive = <function>;
trace:     mapAttrsRecursiveCond = <function>;
trace:     mapAttrsToList = <function>;
trace:     matchAttrs = <function>;
trace:     nameValuePair = <function>;
trace:     optionalAttrs = <function>;
trace:     overrideExisting = <function>;
trace:     recurseIntoAttrs = <function>;
trace:     recursiveUpdate = <function>;
trace:     recursiveUpdateUntil = <function>;
trace:     setAttrByPath = <function>;
trace:     toDerivation = <function>;
trace: lib.zip is deprecated, use lib.zipAttrsWith instead
trace:     zip = <function>;
trace:     zipAttrs = <function>;
trace:     zipAttrsWith = <function>;
trace:     zipAttrsWithNames = <function>;
trace:     zipWithNames = <function>;
trace:   };
trace:   bitAnd = <function>;
trace:   bitNot = <function>;
trace:   bitOr = <function>;
trace:   bitXor = <function>;
trace:   boolToString = <function>;
trace:   callPackageWith = <function>;
trace:   callPackagesWith = <function>;
trace:   canCleanSource = <function>;
trace:   catAttrs = <function>;
trace:   checkFlag = <function>;
trace:   checkReqs = <function>;
trace:   chooseDevOutputs = <function>;
trace:   cleanSource = <function>;
trace:   cleanSourceFilter = <function>;
trace:   cleanSourceWith = <function, args: {src, filter?, name?}>;
trace:   cli = {
trace:     toGNUCommandLine = <function, args: {mkBool?, mkList?, mkOption?, mkOptionName?}>;
trace:     toGNUCommandLineShell = <function>;
trace:   };
trace:   closePropagation = <function>;
trace:   collect = <function>;
trace:   commitIdFromGitRepo = <function>;
trace:   compare = <function>;
trace:   compareLists = <function>;
trace:   composeExtensions = <function>;
trace:   concat = <function>;
trace:   concatImapStrings = <function>;
trace:   concatImapStringsSep = <function>;
trace:   concatLists = <function>;
trace:   concatMap = <function>;
trace:   concatMapStrings = <function>;
trace:   concatMapStringsSep = <function>;
trace:   concatStrings = <function>;
trace:   concatStringsSep = <function>;
trace:   condConcat = <function>;
trace:   const = <function>;
trace:   converge = <function>;
trace:   count = <function>;
trace:   crossLists = <function>;
trace:   customisation = {
trace:     callPackageWith = <function>;
trace:     callPackagesWith = <function>;
trace:     extendDerivation = <function>;
trace:     hydraJob = <function>;
trace:     makeOverridable = <function>;
trace:     makeScope = <function>;
trace:     overrideDerivation = <function>;
trace:   };
trace:   debug = {
trace:     addErrorContextToAttrs = <function>;
trace:     attrNamesToStr = <function>;
trace:     runTests = <function>;
trace: Warning: `showVal` is deprecated and will be removed in the next release, please use `traceSeqN`
trace:     showVal = <function>;
trace:     testAllTrue = <function>;
trace:     traceCall = <function>;
trace:     traceCall2 = <function>;
trace:     traceCall3 = <function>;
trace:     traceCallXml = <function>;
trace:     traceIf = <function>;
trace:     traceSeq = <function>;
trace:     traceSeqN = <function>;
trace:     traceShowVal = <function>;
trace:     traceShowValMarked = <function>;
trace:     traceVal = <function>;
trace:     traceValFn = <function>;
trace:     traceValIfNot = <function>;
trace:     traceValSeq = <function>;
trace:     traceValSeqFn = <function>;
trace:     traceValSeqN = <function>;
trace:     traceValSeqNFn = <function>;
trace:     traceXMLVal = <function>;
trace:     traceXMLValMarked = <function>;
trace:   };
trace:   deepSeq = <function>;
trace:   defaultFunctor = <function>;
trace:   defaultMerge = <function>;
trace:   defaultMergeArg = <function>;
trace:   defaultTypeMerge = <function>;
trace:   dischargeProperties = <function>;
trace:   doRename = <function, args: {from, to, use, visible, warn, withPriority?}>;
trace:   dontDistribute = <function>;
trace:   dontRecurseIntoAttrs = <function>;
trace:   drop = <function>;
trace:   elem = <function>;
trace:   elemAt = <function>;
trace:   enableFeature = <function>;
trace:   enableFeatureAs = <function>;
trace:   escape = <function>;
trace:   escapeShellArg = <function>;
trace:   escapeShellArgs = <function>;
trace:   evalModules = <function, args: {modules, args?, check?, prefix?, specialArgs?}>;
trace:   evalOptionValue = <function>;
trace:   extend = <function>;
trace:   extendDerivation = <function>;
trace:   extends = <function>;
trace:   fakeHash = "sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=";
trace:   fakeSha256 = "0000000000000000000000000000000000000000000000000000000000000000";
trace:   fakeSha512 = "00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000";
trace:   fetchers = {
trace:     proxyImpureEnvVars = [ <5 elements> ];
trace:   };
trace:   fileContents = <function>;
trace:   filesystem = {
trace:     haskellPathsInDir = <function>;
trace:     locateDominatingFile = <function>;
trace:   };
trace:   filter = <function>;
trace:   filterAttrs = <function>;
trace:   filterAttrsRecursive = <function>;
trace:   filterOverrides = <function>;
trace:   findFirst = <function>;
trace:   findSingle = <function>;
trace:   fix = <function>;
trace:   fix' = <function>;
trace:   fixMergeModules = <function>;
trace:   fixedPoints = {
trace:     composeExtensions = <function>;
trace:     converge = <function>;
trace:     extends = <function>;
trace:     fix = <function>;
trace:     fix' = <function>;
trace:     makeExtensible = <function>;
trace:     makeExtensibleWithCustomName = <function>;
trace:   };
trace:   fixedWidthNumber = <function>;
trace:   fixedWidthString = <function>;
trace:   fixupOptionType = <function>;
trace:   flatten = <function>;
trace:   flip = <function>;
trace:   fold = <function>;
trace:   foldArgs = <function>;
trace:   foldAttrs = <function>;
trace:   foldl = <function>;
trace:   foldl' = <function>;
trace:   foldr = <function>;
trace:   forEach = <function>;
trace:   fullDepEntry = <function>;
trace:   functionArgs = <function>;
trace:   genAttrs = <function>;
trace:   genList = <function>;
trace:   generators = {
trace:     mkKeyValueDefault = <function, args: {mkValueString?}>;
trace:     mkValueStringDefault = <function>;
trace:     toGitINI = <function>;
trace:     toINI = <function, args: {listsAsDuplicateKeys?, mkKeyValue?, mkSectionName?}>;
trace:     toJSON = <function>;
trace:     toKeyValue = <function, args: {listsAsDuplicateKeys?, mkKeyValue?}>;
trace:     toPlist = <function>;
trace:     toPretty = <function, args: {allowPrettyValues?, initialState?, multiline?, nextState?, recursionLimit?}>;
trace:     toYAML = <function>;
trace:   };
trace:   genericClosure = <function>;
trace:   getAttr = <function>;
trace:   getAttrFromPath = <function>;
trace:   getAttrs = <function>;
trace:   getBin = <function>;
trace:   getDev = <function>;
trace:   getFiles = <function>;
trace:   getLib = <function>;
trace:   getMan = <function>;
trace:   getName = <function>;
trace:   getOutput = <function>;
trace:   getValue = <function>;
trace:   getValues = <function>;
trace:   getVersion = <function>;
trace:   groupBy = <function>;
trace:   groupBy' = <function>;
trace:   hasAttr = <function>;
trace:   hasAttrByPath = <function>;
trace:   hasInfix = <function>;
trace:   hasPrefix = <function>;
trace:   hasSuffix = <function>;
trace:   head = <function>;
trace:   hiPrio = <function>;
trace:   hiPrioSet = <function>;
trace:   hydraJob = <function>;
trace:   id = <function>;
trace:   ifEnable = <function>;
trace:   imap = <function>;
trace:   imap0 = <function>;
trace:   imap1 = <function>;
trace:   importJSON = <function>;
trace:   inNixShell = false;
trace:   info = <function>;
trace:   init = <function>;
trace:   innerClosePropagation = <function>;
trace:   innerModifySumArgs = <function>;
trace:   intersectLists = <function>;
trace:   intersperse = <function>;
trace:   isAttrs = <function>;
trace:   isBool = <function>;
trace:   isDerivation = <function>;
trace:   isFunction = <function>;
trace:   isInt = <function>;
trace:   isList = <function>;
trace:   isOption = <function>;
trace:   isOptionType = <function>;
trace:   isStorePath = <function>;
trace:   isString = <function>;
trace:   isType = <function>;
trace:   kernel = {
trace:     freeform = <function>;
trace:     module = { <2 attributes> };
trace:     no = { <2 attributes> };
trace:     option = <function>;
trace:     whenHelpers = <function>;
trace:     yes = { <2 attributes> };
trace:   };
trace:   last = <function>;
trace:   lazyGenericClosure = <function, args: {operator, startSet}>;
trace:   length = <function>;
trace:   lessThan = <function>;
trace:   licenses = {
trace:     abstyles = { <4 attributes> };
trace:     afl21 = { <4 attributes> };
trace:     afl3 = { <4 attributes> };
trace:     agpl3 = { <4 attributes> };
trace:     agpl3Only = { <4 attributes> };
trace:     agpl3Plus = { <4 attributes> };
trace:     amazonsl = { <4 attributes> };
trace:     amd = { <4 attributes> };
trace:     apsl20 = { <4 attributes> };
trace:     arphicpl = { <3 attributes> };
trace:     artistic1 = { <4 attributes> };
trace:     artistic2 = { <4 attributes> };
trace:     asl20 = { <4 attributes> };
trace:     beerware = { <4 attributes> };
trace:     blueOak100 = { <4 attributes> };
trace:     boost = { <4 attributes> };
trace:     bsd0 = { <4 attributes> };
trace:     bsd2 = { <4 attributes> };

Note that I only limited it to 300 lines for demonstration, but the implementation can handle arbitrary big numbers.

Or alternatively, this call:

toPretty {
  recursionLimit = 2;
  nextState = line: left:
    if left == 0 then { return = null; }
    else builtins.trace line (left - 1);
  initialState = 10;
} pkgs

prints and evaluates only the first couple packages of pkgs, therefore returning within about a second:

trace: {
trace:   AAAAAASomeThingsFailToEvaluate = <failure>;
trace:   AMB-plugins = <derivation /nix/store/5040g6rfg8aijkxh9hnp6v454k4pnqjf-AMB-plugins-0.8.1.drv>;
trace:   ArchiSteamFarm = <derivation /nix/store/5vqny1dkca98ijd9x4asvj1g7vpa5gry-ArchiSteamFarm-4.2.4.0.drv>;
trace:   AusweisApp2 = <derivation /nix/store/h3ab3jxrnrk2a53h0cwcmmvakp45nji5-AusweisApp2-1.20.2.drv>;
trace:   CoinMP = <derivation /nix/store/xmbqnw4s2nb9wy9lsj6hv3iwkgk3vxmj-CoinMP-1.8.4.drv>;
trace:   DisnixWebService = <derivation /nix/store/f5caamhwhlhw24v40zyf80z8sdr9m0gd-DisnixWebService-0.9.drv>;
trace:   EBTKS = <derivation /nix/store/gk6xmh7cjm9qcnh9n8k7fjg3r4jdwrdp-EBTKS-2017-09-23.drv>;
trace:   EmptyEpsilon = <derivation /nix/store/asp0firfapr28jwxn91vj29da14lprz1-empty-epsilon-2020.08.07.drv>;
trace:   FIL-plugins = <derivation /nix/store/akv2ni3czd57z6pdj4yq5k8ixvls9c3i-FIL-plugins-0.3.0.drv>;

So this new implementation stops evaluation as soon as return is returned.

@jonringer
Copy link
Contributor

@GrahamcOfBorg eval

@infinisil
Copy link
Member Author

@jonringer It fails as expected btw, I haven't updated the tests for this change

@jonringer
Copy link
Contributor

@infinisil there was another issue going on around the time you made the PR #98808

@jonringer
Copy link
Contributor

I just ran eval on all PRs that were failing

Copy link
Member

@Profpatsch Profpatsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is basically my comment on the other PR, but times 100, since toPretty is a monster now. I count these responsibilities:

  • handling failing subexpressions
  • recursion limit detection/limiting
  • pretty printing of values (the original functionality)
  • multiline printing & indentation
  • streaming of the output

I don’t think this is viable.

We have a lazy functional language here, so we should be able to split this up into at least 3 different, orthogonal functions that compose nicely, instead of one giant blob of interleaved responsibilities.

@Profpatsch
Copy link
Member

For example, constructing a data structure of streaming values (and consuming those) is a task that is completely orthogonal to how things are formatted. Same goes with indentation/pretty printing of multiline values.

@SuperSandro2000 SuperSandro2000 added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 18, 2021
@stale
Copy link

stale bot commented Jul 21, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 21, 2021
@Profpatsch
Copy link
Member

I’d tentatively close this now, I think my opposition to this change is still the same, we shouldn’t overcomplicate toPretty unless there is a specific need to have a streaming version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants