Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deprecate mkApp in favor of direct definitions #103

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

soupglasses
Copy link

Now that nixpkgs has its own idea of passthru.exePath under the name meta.mainProgram, deprecate our solution in favor of the new standard.

This subsequently removes the value of which mkApp brings, and therefore a full deprecation for its removal has begun.

Closes: #65

@soupglasses soupglasses force-pushed the main branch 4 times, most recently from 015166e to 2a9b808 Compare August 20, 2023 01:24
@soupglasses
Copy link
Author

That became a few more force pushes than desired, but it should all be good now! 😅

Now that nixpkgs has its own idea of `passthru.exePath` under the
name `meta.mainProgram`, deprecate our solution in favor of the new
standard.

This subsequently removes the value of which `mkApp` brings,
and therefore a full deprecation for its removal has begun.
Copy link
Member

@zimbatm zimbatm left a comment

Choose a reason for hiding this comment

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

Thanks! Left a few comments, the PR should be ready once these have been addressed.

README.md Outdated Show resolved Hide resolved
examples/each-system/flake.nix Outdated Show resolved Hide resolved
@@ -12,7 +12,10 @@
default = hello;
};
apps = rec {
hello = flake-utils.lib.mkApp { drv = self.packages.${system}.hello; };
Copy link
Member

Choose a reason for hiding this comment

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

Don't you think it would be nicer to deprecate drv.passthru.exePath and use nixpkgs.lib.getExe in the helper instead?

This is still a bit simpler to use.

Copy link
Member

Choose a reason for hiding this comment

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

That being said, I don't see a lot of value in definining those apps.

Copy link
Author

@soupglasses soupglasses Aug 20, 2023

Choose a reason for hiding this comment

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

Yeah. There is a problem where all these apps can be rewritten as packages."<system>"."<name>" since nix3-run runs against meta.mainProgram as well.

Unsure how to show examples without removing apps outright.

README.md Outdated Show resolved Hide resolved
lib.nix Outdated Show resolved Hide resolved
@zimbatm
Copy link
Member

zimbatm commented Aug 23, 2023

I forgot; the main use-case for apps is for packages that output multiple binaries. For example mkApp { drv = coreutils; name = "ls"; }.

So I propose to just deprecate any uses of drv.passthru.exePath.

@zimbatm
Copy link
Member

zimbatm commented Aug 23, 2023

ok, that seems ready to me. have a look and tell me what you think.

@soupglasses
Copy link
Author

soupglasses commented Aug 23, 2023

I am honestly still quite a bit for deprecating the entire mkApp still. And recommending the use of { type = "app"; program = "${drv}/bin/name"; } or using packages. with mainProgram instead.

As there is little gain having a mkApp function compared to just using the direct definition it creates. For example:

{
  outputs.apps.x86_64-linux = {
    with-lib = flake-utils.lib.mkApp { drv = pkgs.coreutils; name = "ls"; };
    no-lib = { type = "app"; program = "${pkgs.coreutils}/bin/ls"; };
  };
}

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.

Use meta.mainProgram by default in mkApp
2 participants