Skip to content

apps: Add meta option#240

Merged
roberth merged 4 commits intohercules-ci:mainfrom
shivaraj-bh:freeform-app-type
Aug 30, 2024
Merged

apps: Add meta option#240
roberth merged 4 commits intohercules-ci:mainfrom
shivaraj-bh:freeform-app-type

Conversation

@shivaraj-bh
Copy link
Copy Markdown
Contributor

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

This allows specifying metadata info about the flake app. This has been standardised in Nix at NixOS/nix#11297

Here’s an example of its usage:

# Inside perSystem
{
  apps.hello = {
    program = pkgs.hello;
    type = "app";
    meta.description = "I say hello!";
  };
}

@roberth
Copy link
Copy Markdown
Member

roberth commented Aug 13, 2024

I'm supportive of the idea, but the Nix manual states

The only supported attributes are:

  • type [...]
  • program [...]

I guess this is somewhat open to interpretation, but allowing any attribute whatsoever does not seem to be the intent.
nix flake check is silent about it, but most likely that's just an oversight.

Could you open a PR to nix that documents the description attribute? That way it can become a standard, and everyone benefits from better metadata and discoverability.

As for this PR, I'd be happy to accept a description option when the Nix PR is accepted, but preferably not a freeformType, because that would let typos in these attribute names to go uncaught, and iirc a freeformType can also cause worse errors when "recursive" definitions are involved (e.g. desrcription = "something ${baseNameOf config.command}" would be an infinite recursion because of the typo+freeformType).

@shivaraj-bh
Copy link
Copy Markdown
Contributor Author

Could you open a PR to nix that documents the description attribute? That way it can become a standard, and everyone benefits from better metadata and discoverability.

Good idea! Here’s the PR: NixOS/nix#11297

This allows specifying metadata info about the flake app. This has been
standardized in Nix at NixOS/nix#11297
@shivaraj-bh shivaraj-bh changed the title apps: Use freeformType in appType apps: Add meta option Aug 17, 2024
@shivaraj-bh
Copy link
Copy Markdown
Contributor Author

@roberth I have made changes based on what was standardised upstream

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.

I was thinking ideally meta could be a submodule with freeform type, but that would cause description to error or have a default like "", neither of which is an improvement, so this is probably the best solution for now.

Related:

shivaraj-bh and others added 2 commits August 17, 2024 17:48
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
@shivaraj-bh
Copy link
Copy Markdown
Contributor Author

@roberth Does it look good now?

@srid
Copy link
Copy Markdown
Contributor

srid commented Aug 26, 2024

@roberth Is there anything to be done here before merging? This would be useful to implement: juspay/omnix#163

@roberth roberth enabled auto-merge August 30, 2024 13:29
@roberth roberth merged commit af510d4 into hercules-ci:main Aug 30, 2024
};
meta = mkOption {
type = types.lazyAttrsOf lib.types.raw;
default = { };
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In hindsight, should we add a default here? Taking from program, that is, if it is a derivation. But coercedTo seems lossy, so this probably is not possible.

Until then, users basically will have to explicit set it like follows:

image

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants