nix_rs: Inherit `stderr` while running `nix` as a child process

shivaraj-bh commented 4 weeks ago

resolves #209

We also use flake-schemas in this PR to fetch the om config from the flake output instead of using nix eval (since inheriting stderr means it's no longer captured for om to process). An additional, unintended benefit of using flake-schemas is caching the output of nix flake show.

Changes in nix CLI

https://github.com/shivaraj-bh/nix/commit/1d23c1e871981f5666a12c4409bd0574fc1e1e02

Demo

❯ om show github:nammayatri/nammayatri
🐚 nix --extra-experimental-features 'nix-command flakes' show-config --json
warning: unknown setting 'upgrade-nix-store-path-url'
🐚 /nix/store/n02w2ybg9fc78grzz9i2aj49q3rysp7m-nix-2.24.0pre20240801_af10904/bin/nix flake show --legacy --allow-import-from-derivation --json --default-flake-schemas /nix/store/xzalq6mcw0ahyaccab6k98dbx3ll53y6-source github:nammayatri/nammayatri
warning: unknown experimental feature 'repl-flake'
[0.6 MiB DL] downloading 'https://github.com/nammayatri/nammayatri/archive/a3e472efe24e731268d20a31f75a1519ded50220.tar.gz'^C
[0.6 MiB DL]

TODO

[x] Update nix_rs changelog
[x] Update history.md
[x] flake-schemas as local flake (for omSchema) while importing majority of schemas from DetSys/flake-schemas
[x] move find_qualified_attr_in_flake_output to FlakeOutputs
[x] simplify FlakeOutputs serialization… use untagged for flattening
[x] refactor(DRY) in nix/flake-schemas/flake.nix

shivaraj-bh commented 3 weeks ago

Upon inheriting stderr of child process (to display progress of underlying nix command), we can no longer capture it, breaking nix_eval_attr.

Do we need nix_eval_attr to rely on stderr to conclude that an attribute is missing? Could we use flake-schemas here?

Making flake-schemas recognise om flake output isn’t that hard: https://github.com/shivaraj-bh/flake-schemas/commit/037434d5bad1f4f66598e5925265f2036ca2a1ba

shivaraj-bh commented 3 weeks ago

Oh no, It doesn’t:

"om": {
    "doc": "Configuration for `omnix` CLI.\n",
    "output": {
      "children": {
        "ci": {
          "leaf": true
        },
        "health": {
          "leaf": true
        }
      }
    }
  },

I will have to recursively mkChildren to include the entire configuration in the schema.

shivaraj-bh commented 3 weeks ago

Upon recursively creating these children, it seems to sorta work:

"om": {
    "doc": "Configuration for `omnix` CLI.\n",
    "output": {
      "children": {
        "ci": {
          "children": {
            "default": {
              "children": {
                "cli-test-dep-cache": {
                  "children": {
                    "dir": {
                      "leaf": true
                    }
                  }
                },
                "doc": {
                  "children": {
                    "dir": {
                      "leaf": true
                    }
                  }
                },
                "flakreate-registry": {
                  "children": {
                    "dir": {
                      "leaf": true
                    }
                  }
                },
                "omnix": {
                  "children": {
                    "dir": {
                      "leaf": true
                    }
                  }
                }
              }
            }
          }
        },

But if you notice, along with “leaf” we don’t have the attribute mentioning the “value” of the leaf node, even though it is added in the schema:

{
  let
    omSchema = {
      version = 1;
      doc = ''
        Configuration for `omnix` CLI.
      '';
      inventory = output:
      let
        recurse = prefix: attrs: self.lib.mkChildren (builtins.mapAttrs
          (attrName: attrs:
            if (builtins.typeOf attrs) != "set" then
              {
                value = attrs;
                what = "omnix config";
              }
            else
              recurse (prefix + attrName + ".") attrs
          )
          attrs);
      in
        recurse "" output;
    };
  in
}

shivaraj-bh commented 3 weeks ago

That is an easy fix in nix, see: https://github.com/shivaraj-bh/nix/commit/1d23c1e871981f5666a12c4409bd0574fc1e1e02

Note to self: Mention this use case in flake-schemas PR along with examples.

shivaraj-bh commented 3 weeks ago

voila

"om": {
    "doc": "Configuration for `omnix` CLI.\n",
    "output": {
      "children": {
        "ci": {
          "children": {
            "default": {
              "children": {
                "cli-test-dep-cache": {
                  "children": {
                    "dir": {
                      "leaf": true,
                      "value": "crates/omnix-cli/tests",
                      "what": "omnix config"
                    }
                  }
                },
                "doc": {
                  "children": {
                    "dir": {
                      "leaf": true,
                      "value": "doc",
                      "what": "omnix config"
                    }
                  }
                },
                "flakreate-registry": {
                  "children": {
                    "dir": {
                      "leaf": true,
                      "value": "crates/flakreate/registry",
                      "what": "omnix config"
                    }
                  }
                },
                "omnix": {
                  "children": {
                    "dir": {
                      "leaf": true,
                      "value": ".",
                      "what": "omnix config"
                    }
                  }
                }
              }
            }
          }
        },

srid commented 3 weeks ago

See also #211 (omnix flake-module)

We also want to support the scenario of there not being any om configuration in the flake (so as to default back to the Default::default value).

srid commented 3 weeks ago

Is flake-schemas really relevant here? Consider the maintenance burden we would be taking on. Keeping all 3 places -- Rust types, flake-parts module, and flake-schema -- in sync.

shivaraj-bh commented 3 weeks ago

Is flake-schemas really relevant here? Consider the maintenance burden we would be taking on. Keeping all 3 places -- Rust types, flake-parts module, and flake-schema -- in sync.

Rust types

https://github.com/juspay/omnix/issues/214 should solve the burden of maintaining Rust types?

flake-parts module

Aren’t we maintaining the flake-parts module in omnix itself?

flake-schema

That would require maintaining a fork until the upstream merge happens, I can keep this up-to-date, now that I am slowly getting familiarised with the nix codebase.

shivaraj-bh commented 3 weeks ago

I did try to find different ways to not rely on the stderr output, but unless there is a way to print flake output as json, I don’t see anything better than using flake-schemas.

srid commented 3 weeks ago

Feel free to explore the flake-schemas route. I'd be curious to see how it looks, in the end. Right now, I don't have a clear of idea of what is entailed.

That would require maintaining a fork until the upstream merge happens, I can keep this up-to-date, now that I am slowly getting familiarised with the nix codebase.

Can't the flake-schema live in this repo itself? I thought flake-schemas are meant to be decentralized, anyway (i.e., downstream flakes can specify their own schemas).

srid commented 3 weeks ago

Rust types

214 should solve the burden of maintaining Rust types?

For om show, yes.

Not necessarily for stuff like #216

Here's an idea: keep Rust types as canonical source of truth, and then generate the Nix expression necessary for creating flake-parts module and flake-schema from it ... in the same vein as https://hackage.haskell.org/package/purescript-bridge

shivaraj-bh commented 1 week ago

This PR changes a bunch of things, to make the review simpler here are the primary changes:

flake configs are no longer fetched using nix eval, making nix_eval_attr, nix_eval_qualified_attr and omnix-common::OmConfig::from_flake_url redundant, should we still support them along with supporting missing_attribute check via nix eval?
Leaf enum has been removed for simplicity while defining custom serializer for FlakeOutputs

srid commented 1 week ago

(Had to merge from main because I just wanted to try running it on some flakes..)

srid commented 1 week ago

Functionally, it works quite well:

shivaraj-bh commented 1 week ago

@srid added a couple of TODO’s to the description, feel free to add more if I missed any

shivaraj-bh commented 1 week ago

Update the usage example in show.md

@srid do you mean to update the example itself? i.e to show something with less of “N/A” or to show the progress of “nix flake show” due to inherited stderr?

srid commented 1 week ago

Former. Less of N/A.

srid commented 1 week ago

Update the usage example in show.md

Nevermind; I thought we are doing om show on this repo itself, but this is happening on nixos-config.

srid commented 1 week ago

Anyway, I pushed some nix changes to master. Refactors, moving of clippy drv to checks, removing nix_health package, and adding doc/clippy for omnix-common. om show on this repo now displays:

shivaraj-bh commented 1 week ago

I don’t see the description for the apps in the screenshot

srid commented 1 week ago

That was run from main branch.

shivaraj-bh commented 1 week ago

description to apps in main branch was added in https://github.com/juspay/omnix/pull/251?

srid commented 1 week ago

Huh, ignore my last screenshot. I think I ran it using om from my nixos-config. Here's the latest:

(This was run from the omnix-init branch, but that one is up to date with main).

shivaraj-bh commented 1 week ago

Apart from Refactoring flake-schemas/flake.nix, this PR is almost ready. @srid should we bother keeping eval.rs, now that it isn’t being used anywhere?

srid commented 1 week ago

We can get rid of eval.rs. By the way, I don't understand the Filtered output types much. But I'll take a look a bit later.

(You can also drop by huddle to pair on it)

juspay / omnix