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

feat: add extra_packages input #37

Closed
wants to merge 1 commit into from
Closed

Conversation

mrcjkb
Copy link
Member

@mrcjkb mrcjkb commented Mar 14, 2023

Fixes #35.

@mrcjkb mrcjkb force-pushed the nixpkgs_build_deps branch from 568c71e to c12041a Compare March 14, 2023 22:21
@mrcjkb mrcjkb marked this pull request as draft March 14, 2023 22:22
@mrcjkb mrcjkb force-pushed the nixpkgs_build_deps branch 11 times, most recently from 27c8c76 to b74ad99 Compare March 15, 2023 00:07
@mrcjkb
Copy link
Member Author

mrcjkb commented Mar 15, 2023

I hate Bash!

@mrcjkb mrcjkb force-pushed the nixpkgs_build_deps branch from b74ad99 to 9381c2c Compare March 15, 2023 00:10
@mrcjkb mrcjkb force-pushed the nixpkgs_build_deps branch from 9381c2c to 70e464d Compare March 15, 2023 00:11
@mrcjkb
Copy link
Member Author

mrcjkb commented Mar 15, 2023

It works!

@mrcjkb mrcjkb marked this pull request as ready for review March 15, 2023 00:23
@mrcjkb mrcjkb requested a review from teto March 15, 2023 00:23
@teto
Copy link
Member

teto commented Mar 18, 2023

I dont think this covers enough cases:

  • what if the software isn't in nixpkgs
  • what if I want an override for it
  • what if I dont know nix ?

I think it's best to let the users in control of the setup, can't they install packages with a pre: step thing ?

@mrcjkb
Copy link
Member Author

mrcjkb commented Mar 18, 2023

I dont think this covers enough cases:

* what if the software isn't in nixpkgs

I don't think it would be feasible to try to cover all edge cases with this workflow.
This is a simple way to add basic support for external build dependencies.

* what if I want an override for it

* what if I dont know nix ?

Users shouldn't need to know nix or write package overrides. All this input does is add the package to the PATH so it can be used by make. No nix involved.
If a plugin needs an override to build for some reason, it should probably be done in a custom workflow. The script is available as a luarocks package, so it can be installed in custom workflows.

I think it's best to let the users in control of the setup, can't they install packages with a pre: step thing ?

If this were a JavaScript action, it would be possible to install packages using other github actions dedicated to installing them. I'm pretty sure there are far less github actions for installing packages than there are packages available on nixpkgs.

@teto
Copy link
Member

teto commented Mar 19, 2023

Users shouldn't need to know nix or write package overrides. All this input does is add the package to the PATH so it can be used by make. No nix involved.

User must use the derivation path used by nix. Granted there is https://search.nixos.org/packages?channel=unstable&size=50&sort=relevance&type=packages&query=neovim

I don't think it would be feasible to try to cover all edge cases with this workflow.

depending on how much it covers is it worth adding ?

Right now users do something like

  luarocks-upload:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - uses: nvim-neorocks/luarocks-tag-release@fix-treesitter
        env:
          LUAROCKS_API_KEY: ${{ secrets.LUAROCKS_API_KEY }}
        with:
          name: nvim-treesitter-test
          version: 0.1
          license: apache 2
          detailed_description: |
            The goal of nvim-treesitter is both to provide a simple and easy way to use the interface for tree-sitter in Neovim
            and to provide some basic functionality such as highlighting based on it.
          build_type: "make"
          template: nvim-treesitter-luarocks.template

Our action runs in docker if I trust action.yaml:

  using: "docker"
  image: "Dockerfile"

is there a way to have it run on the host instead ? so that users can install dependencies on the host of their choice (ubuntu/fedora etc) then run this action which can use the previously installed software in PATH. We would need to install nix on that host though.

Installing software is hard (only nix does it well IMO) so adding an action-specific way of doing it will end up being a lot of work + confusion for plugin authors IMO.

@mrcjkb
Copy link
Member Author

mrcjkb commented Mar 19, 2023

User must use the derivation path used by nix.

What do you mean by derivation path? The nix store path or the nixpkgs attribute set path, e.g. lua51Packages.dkjson?

is there a way to have it run on the host instead ?

We could try out a composite action that uses cachix/install-nix-action.
I initially wanted to avoid depending on other github actions, but if we can hide the dependency implementation from users (so they don't have add the install-nix-action to their workflow files), maybe it's a good idea.

@mrcjkb
Copy link
Member Author

mrcjkb commented Mar 19, 2023

Alternate approach in #38

@mrcjkb
Copy link
Member Author

mrcjkb commented Mar 19, 2023

Closing in favour of #38

@mrcjkb mrcjkb closed this Mar 19, 2023
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.

nixpkgs_build_deps input for make build dependencies
2 participants