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 rockspec + luarocks upload workflow #2276

Merged
merged 1 commit into from
Jan 10, 2023

Conversation

mrcjkb
Copy link
Contributor

@mrcjkb mrcjkb commented Dec 18, 2022

Description

telescope.nvim is a common dependency across Neovim plugins. Using luarocks may alleviate the need for users to specify their plugins' dependencies in their plugin manager.
(e.g., vim-plug or packer).

See also:
https://teto.github.io/posts/2021-09-17-neovim-plugin-luarocks.html

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

luarocks install ./telescope.nvim-scm-1.rockspec --local

Checklist:

  • My code follows the style guidelines of this project (stylua)
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (lua annotations)

@mrcjkb
Copy link
Contributor Author

mrcjkb commented Dec 18, 2022

If this PR is accepted, a package on www.luarocks.org will need to be added, which can be done from only one account. See luarocks/luarocks-site#173.

@mrcjkb mrcjkb marked this pull request as draft December 18, 2022 23:48
@Conni2461
Copy link
Member

we can do this and i can also maintain the luarocks package. i kinda already do that for plenary as well: https://luarocks.org/search?q=plenary.nvim

@Conni2461
Copy link
Member

Is anything missing here? I think it looks good but you marked it as a Draft ?

@mrcjkb
Copy link
Contributor Author

mrcjkb commented Dec 21, 2022

The rockspec itself should be good to go.
I was hoping to add some tests (or at least get my manual test working).

On my NixOS machine, in a nix shell with lua51Packages.luarocks, running

luarocks install ./telescope.nvim-scm-1.rockspec --local

gives me

Missing dependencies for telescope.nvim scm-1:
   plenary.nvim (not installed)

telescope.nvim scm-1 depends on lua 5.1, < 5.4 (5.1-1 provided by VM)
telescope.nvim scm-1 depends on plenary.nvim (not installed)

Error: Could not satisfy dependency plenary.nvim: No results matching query were found for Lua 5.1.

There may also the concern of keeping the luarocks packages up to date (i.e. using github release workflows). For example, the scm-1 package of plenary.nvim on luarocks is missing the filetype module.

@mrcjkb
Copy link
Contributor Author

mrcjkb commented Dec 21, 2022

It looks like the error is because the plenary luarocks package doesn't have a non-dev release.

@teto
Copy link

teto commented Dec 28, 2022

I had removed plenary from luarocks.org so that the plenary maintainers can upload their own version. Seems like they haven't yet so I reuploaded a version: https://luarocks.org/modules/teto/plenary.nvim . I used the tagged version, let me know if it's too old and we can nag the plenary maintainers to push a new tag / maintain the rockspec.

@Conni2461
Copy link
Member

Hey teto, that a funny things because i did upload to luarocks: https://luarocks.org/modules/Conni2461/plenary.nvim

Maybe i did something wrong because its flagged as dev, i need to do some research regarding publishen to luarocks.
Also Tj added a random 0.1 tag last week because someone wants to publish plenary on arch nvim-lua/plenary.nvim#429 not a big fan of both of those things :|

Like i previously said in the plenary repo, i am fine with maintaining it and keeping it up to date.

@teto
Copy link

teto commented Dec 29, 2022

@Conni2461 ha thanks for the report ! sry for the bad rap ^^'' .

Maybe i did something wrong because its flagged as dev

I got hit by this too, so the rockspec you've added is the scm=source control management = dev version which is not installable via a plain luarocks install plenary.nvim (only works if you add --dev e.g luarocks install --dev plenary.nvim).

You want a tagged release to make it work out of the box, which means editing the rockspec as I've done here (rename the extension to rockspec, else github complained):
plenary.nvim-0.1.0-1.txt

Depending on the project, some of them git the plenary.nvim-0.1.0-1.rockspec. I think it's a nice thing to do in case you need to reupload it and for others to fork/patch/contribute.

@mrcjkb
Copy link
Contributor Author

mrcjkb commented Jan 1, 2023

Depending on the project, some of them git the plenary.nvim-0.1.0-1.rockspec. I think it's a nice thing to do in case you need to reupload it and for others to fork/patch/contribute.

Maybe a script that generates the rockspec based on the current git tag or revision (using something like git describe --tags --always --first-parent) could be used with GitHub Actions.

telescope.nvim is a common dependency across Neovim plugins.
Using luarocks may alleviate the need for users to specify their
plugins' dependencies in their plugin manager.
(e.g., vim-plug or packer).

See also:
https://teto.github.io/posts/2021-09-17-neovim-plugin-luarocks.html
@mrcjkb
Copy link
Contributor Author

mrcjkb commented Jan 6, 2023

I think this is ready now. :)

@mrcjkb mrcjkb marked this pull request as ready for review January 6, 2023 22:45
@mrcjkb mrcjkb changed the title feat: add rockspec feat: add rockspec + luarocks upload workflow Jan 6, 2023
@teto
Copy link

teto commented Jan 9, 2023

I can confirm this workflow works (used in https://github.com/neovim/nvim-lspconfig) so looking forward to see this merged :)

@Conni2461
Copy link
Member

of course i am going to merge it (i also merged it in plenary).

Thanks for all your work :)

I'll try to make a new telescope release (0.1.1) in the next couple of days. I wanna merge a couple more fixes, but i will manually upload 0.1.0 in the next 10min

@Conni2461 Conni2461 merged commit e8c01ba into nvim-telescope:master Jan 10, 2023
@Conni2461
Copy link
Member

image

hmm, there seems to be an issue, any idea?

@teto
Copy link

teto commented Jan 10, 2023

the filename of the rockspec has to match the version inside it and apparently it doesn't accept the shortsha as a version number since it contains letters.

@mrcjkb
Copy link
Contributor Author

mrcjkb commented Jan 10, 2023

The script blindly assumes it's run on a tagged release. Anything that isn't tagged will have the shortsha appended to it, which as @teto points out doesn't match the luarock versioning spec.

I would suggest copying the rockspec to a temp directory and manually setting the MODREV to 0.1.0.

@mrcjkb mrcjkb deleted the rockspec branch January 10, 2023 16:25
@Conni2461
Copy link
Member

Conni2461 commented Jan 10, 2023

yeah makes sense. thanks

0.1.0 is now also published

Conni2461 pushed a commit that referenced this pull request Jan 16, 2023
telescope.nvim is a common dependency across Neovim plugins.
Using luarocks may alleviate the need for users to specify their
plugins' dependencies in their plugin manager.
(e.g., vim-plug or packer).

See also:
https://teto.github.io/posts/2021-09-17-neovim-plugin-luarocks.html

(cherry picked from commit e8c01ba)
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