Skip to content

nix flake show: Support meta attribute for apps#11297

Merged
roberth merged 3 commits intoNixOS:masterfrom
shivaraj-bh:flake-apps-description
Aug 17, 2024
Merged

nix flake show: Support meta attribute for apps#11297
roberth merged 3 commits intoNixOS:masterfrom
shivaraj-bh:flake-apps-description

Conversation

@shivaraj-bh
Copy link
Copy Markdown
Member

@shivaraj-bh shivaraj-bh commented Aug 14, 2024

Motivation

Metadata information for flake apps will be useful while exploring a
flake using nix flake show

Context

The outputs that follow, uses the flake:

{
  inputs.nixpkgs.url = "github:nixos/nixpkgs/nixpkgs-unstable";
  outputs = { nixpkgs, ... }: {
    apps.aarch64-darwin.hello = {
      program = "${nixpkgs.lib.getExe nixpkgs.legacyPackages.aarch64-darwin.hello}";
      type = "app";
      meta.description = "I say hello!";
    };
  };
}

Output without the description:

❯ nix flake show
path:/Users/shivaraj/oss/tmp?lastModified=1723627908&narHash=sha256-D89aZ1hTqNCzvUIHLD6meJVNiApUoGvyMzEiowdf/dU%3D
└───apps
    └───aarch64-darwin
        └───hello: app

Output with the description:

❯ ../nix/outputs/out/bin/nix flake show
path:/Users/shivaraj/oss/tmp?lastModified=1723627908&narHash=sha256-D89aZ1hTqNCzvUIHLD6meJVNiApUoGvyMzEiowdf/dU%3D
└───apps
    └───aarch64-darwin
        └───hello: app: I say hello!

The description is also included in the json output of nix flake show:

{
  "apps": {
    "aarch64-darwin": {
      "hello": {
        "description": "I say hello!",
        "type": "app"
      }
    }
  }
}

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

Copy link
Copy Markdown
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

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

It'd be lovely to have

  • a characterization test for nix flake check
  • warning for unknown attribute in app

but this is already a good change.

@shivaraj-bh
Copy link
Copy Markdown
Member Author

  • a characterization test for nix flake check
  • warning for unknown attribute in app

I can add them. Do you suggest I do this here or in a different PR?

@roberth
Copy link
Copy Markdown
Member

roberth commented Aug 14, 2024

Awesome. Should be fine to do here in new commits.

  • a characterization test for nix flake check

Grep diffAndAcceptInner for examples

  • warning for unknown attribute in app

I think we already have some checks like that, so you could adapt it to work the same.

@kjeremy
Copy link
Copy Markdown
Contributor

kjeremy commented Aug 14, 2024

Is this a dupe of #10980

@roberth
Copy link
Copy Markdown
Member

roberth commented Aug 14, 2024

@kjeremy It's complementary, because this is about apps as opposed to packages and other "derivations".

Partially it kind of should be a duplicate because this PR doesn't solve the terminal layout issue. We could merge either one first and then apply your rendering solution to apps, but I'd slightly prefer to merge your PR first.

@roberth
Copy link
Copy Markdown
Member

roberth commented Aug 14, 2024

A question was raised on matrix whether this should be meta.description.
apps aren't derivations (or package attrsets), so it's not necessary to separate this into meta, but perhaps it'd be good for consistency nonetheless?

@shivaraj-bh shivaraj-bh force-pushed the flake-apps-description branch from 4f60b36 to 0bfc73b Compare August 15, 2024 22:03
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Aug 15, 2024
@shivaraj-bh
Copy link
Copy Markdown
Member Author

nix flake check now throws an error when an unknown attribute is present:

❯ ../nix/outputs/out/bin/nix flake check
error:
       … while checking flake output 'apps'
         at /nix/store/n8v51gyfjrhjrfbabxwy0hh4q68q2v7v-source/flake.nix:5:6:
            4|      # formatter.aarch64-darwin = "foo";
            5|      apps.aarch64-darwin.hello = {
             |      ^
            6|       program = "${nixpkgs.lib.getExe nixpkgs.legacyPackages.aarch64-darwin.hello}";while checking the app definition 'apps.aarch64-darwin.hello'
         at /nix/store/n8v51gyfjrhjrfbabxwy0hh4q68q2v7v-source/flake.nix:5:6:
            4|      # formatter.aarch64-darwin = "foo";
            5|      apps.aarch64-darwin.hello = {
             |      ^
            6|       program = "${nixpkgs.lib.getExe nixpkgs.legacyPackages.aarch64-darwin.hello}";

       error: app 'apps.aarch64-darwin.hello' has unsupported attribute 'unknown-attr'

It also throws error when type or program attributes are missing. For meta you get a warning:

❯ ../nix/outputs/out/bin/nix flake check
warning: app 'apps.aarch64-darwin.hello' lacks attribute 'meta'

@shivaraj-bh
Copy link
Copy Markdown
Member Author

  • nix run doesn’t yet report a warning when it detects an unknown attribute, should it? Also, should nix flake check throw this as an error or just a warning?
  • nix flake check for apps doesn’t resolve the app, like it is done when nix fmt or nix run are called, thus we don’t check if the program is actually present in the store. (Side note: I did try doing sanity on apps by resolving them, but couldn’t find a way to do so, I will have to try again if that’s what is desired)

@shivaraj-bh shivaraj-bh changed the title flake: Show description attribute for apps if present nix flake show: Support meta attribute for apps Aug 15, 2024
src/nix/run.md Outdated
apps.x86_64-linux.blender_2_79 = {
type = "app";
program = "${self.packages.x86_64-linux.blender_2_79}/bin/blender";
description = "Run Blender, a free and open-source 3D creation suite.";
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess this should be meta.description?

However I'm not sure we need the meta prefix. It's necessary for derivations to distinguish derivation input attributes from metadata, but that's not the case for apps.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Though I suppose meta is useful for distinguishing between attributes that should be an error if they're unknown, and attributes that should at most be a warning.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think that's a good argument.
Also users are likely to write meta because they learned it for packages, and it's nice that x.meta.description or "" works for both, if that's going to be relevant.

Copy link
Copy Markdown
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

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

If you could add the warning and sync the docs, this looks good to me 🚀

@shivaraj-bh shivaraj-bh force-pushed the flake-apps-description branch from 0bfc73b to f91d872 Compare August 16, 2024 19:53
@shivaraj-bh
Copy link
Copy Markdown
Member Author

If you could add the warning and sync the docs

Done!

@shivaraj-bh shivaraj-bh force-pushed the flake-apps-description branch from f91d872 to 2ab93fd Compare August 17, 2024 10:20
@roberth roberth enabled auto-merge August 17, 2024 11:04
@roberth roberth merged commit c458598 into NixOS:master Aug 17, 2024
@shivaraj-bh shivaraj-bh deleted the flake-apps-description branch August 17, 2024 11:13
shivaraj-bh added a commit to shivaraj-bh/flake-parts that referenced this pull request Aug 17, 2024
This allows specifying metadata info about the flake app. This has been
standardized in Nix at NixOS/nix#11297
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants