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

Create app-target for each executable cabal package #36

Closed
HariAmoor-professional opened this issue Nov 1, 2022 · 9 comments · Fixed by #137
Closed

Create app-target for each executable cabal package #36

HariAmoor-professional opened this issue Nov 1, 2022 · 9 comments · Fixed by #137
Labels
enhancement New feature or request

Comments

@HariAmoor-professional
Copy link
Contributor

This ticket corresponds to @roberth's comment on #34. To restate, we need to create a flake output in apps'.* for each executable cabal package. The logic to filter executable packages from non-executable ones is non-trivial.

@srid
Copy link
Owner

srid commented Feb 5, 2023

@roberth @HariAmoor-professional Why do we need this? nix run already looks in packages.* if apps are not defined.

@roberth
Copy link
Collaborator

roberth commented Feb 5, 2023

I think it's relevant for multi-executable packages and packages whose executable has a different name than the package name.

@srid srid added the enhancement New feature or request label Feb 23, 2023
@srid
Copy link
Owner

srid commented Mar 29, 2023

How do we approach this?

  • Proper way: parse the .cabal file, and extract executable stanzas.
  • Quick-and-dirty way: look under ${drv}/bin and use those executables (could include more than what cabal produced).

@roberth
Copy link
Collaborator

roberth commented Apr 1, 2023

parse the .cabal file

A regex on the cabal file contents may actually go a long way.
Isn't something like ^executable ([a-zA-Z0-9_.-]*)$ on any line a decent way to find all executables? Haven't checked the grammar, but this probably catches over 99% in practice.

  • look under ${drv}/bin

We shouldn't do IFD to evaluate attrNames apps. That would slow down all apps evaluations, even if they're not from a haskell project.

@srid
Copy link
Owner

srid commented Apr 1, 2023

Yea, we should parse the .cabal file.

What should the interface be? In the context of #62, to begin with I think we could add an option under outputs, like config.haskellProjects.default.outputs.localApps that points to the apps themselves. Or do we want to expose them as per-package sets, like outputs.localPackageInfo.<package-name>.<exe-name> = <nix-app>?

EDIT: Or we can put this in the passthru of packages?

@roberth
Copy link
Collaborator

roberth commented Apr 1, 2023

EDIT: Or we can put this in the passthru of packages?

Putting it in passthru merges it with an IFD-based attrset, incurring the IFD cost, so that's not good enough for generating an apps attrset.

That shouldn't stop us from also adding information from passthru, although perhaps consistency with the rest of the ecosystem is a reason to make such improvements in cabal2nix instead.

outputs.localPackageInfo.. =

Here everything except <nix-app> is computable without IFD or even evaluating any packages. If some code evaluates up to <exe-name> ignoring the values, it will be instant. This format does not lose any information learned from the cabal files, and is unopinionated. I think it's a good idea to have this information around in an option. Something like localApps seems a bit more "opinionated" as it needs to handle name collisions or decide what the app names should be. Nonetheless, it's likely that the default behavior will be good enough, so this is also worth adding.
tl;dr both?

@srid
Copy link
Owner

srid commented Apr 3, 2023

Here everything except <nix-app> is computable without IFD or even evaluating any packages. If some code evaluates up to <exe-name> ignoring the values, it will be instant. This format does not lose any information learned from the cabal files, and is unopinionated. I think it's a good idea to have this information around in an option.

So, let's go with the outputs.localPackageInfo.<package-name>.<exe-name> approach while documenting the caveats of evaluating the values. I suppose in the future, we could store more than the executable metadata here; haskell.nix exports a bunch of things from the cabal file:

image

So maybe (for future expansion): outputs.localPackageInfo.<package-name>.exes.<exe-name>


Do we still need a separate outputs.localPackages and outputs.localPackageInfo? Why not merge them both, viz.:

{
  outputs.localPackages = {
    foo = {
      package = <the drv>;
      exes.foo-exe = <the app>;
    };
  };
}

@roberth
Copy link
Collaborator

roberth commented Apr 3, 2023

Good points, and doing it all merged in localPackages sgtm.

I confused packages and localPackages for a sec. If you add it to packages, the implementation of the reading part becomes local to the package submodule. Now I wonder if we have a reason why localPackages.<name> isn't packages.<name>.package. That's a bit off-topic though.

@srid
Copy link
Owner

srid commented Apr 5, 2023

Now I wonder if we have a reason why localPackages.<name> isn't packages.<name>.package. That's a bit off-topic though.

We can do it as part of this change (#137). It would also be consistent with the haskellProjects.<name>.packages option naming. So we have, basePackages + packages = finalPackages. Not sure if those are the best names, though. Or maybe we should just call it localPackages through out.

@srid srid closed this as completed in #137 Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants