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: publish tagged releases to luarocks #1050

Merged
merged 2 commits into from
Nov 13, 2023
Merged

Conversation

mrcjkb
Copy link
Contributor

@mrcjkb mrcjkb commented Nov 5, 2023

Summary

This PR is part of a push to get (neo)vim plugins on LuaRocks,
so that they can be used with the WIP luarocks-based plugin manager, rocks.nvim.

Things done:

  • Add a workflow that publishes tags to LuaRocks.org when a tag is pushed.

Notes

Adding the API key (screen shot)
github-add-luarocks-api-key

@mrcjkb mrcjkb changed the title feat: publich to luarocks feat: publish tagged releases to luarocks Nov 5, 2023
@L3MON4D3
Copy link
Owner

L3MON4D3 commented Nov 6, 2023

Hey, that's pretty cool!
Especially for luasnip, since we depend on jsregexp, which is available on luarocks, so the error-prone (or, somewhat platform-dependent) compilation-step can be circumvented, or at least outsourced, if luasnip itself is installed via luarock (Right? 😅)

I don't have that much time right away, but I'll look into the blogposts you shared and this PR as soon as I have some free time on my hands :)

@mrcjkb
Copy link
Contributor Author

mrcjkb commented Nov 6, 2023

(Right? 😅)

Kind of. By default, luarocks will use a platform-dependent compilation setup, but it is specified in the package's rockspec (so end users shouldn't have to worry about it) - and luarocks will output an appropriate error message if part of the toolchain required to build a dependency is missing.
That being said, it is possible to host pre-built binaries, which we plan to implement for rocks.nvim, at least for dependencies that are more complicated to build (e.g. if they require a Rust or Node toolchain).

I don't have that much time right away

Take your time.
I don't have much time for my side gigs this month either 😄

@L3MON4D3
Copy link
Owner

Kind of. By default, luarocks will use a platform-dependent compilation setup, but it is specified in the package's rockspec (so end users shouldn't have to worry about it) - and luarocks will output an appropriate error message if part of the toolchain required to build a dependency is missing.
That being said, it is possible to host pre-built binaries, which we plan to implement for rocks.nvim, at least for dependencies that are more complicated to build (e.g. if they require a Rust or Node toolchain).

Cool, ok, as long as C can be compiled we should be fine :D

Is there a way to specify the version for dependencies? We currently don't support the newest release of jsregexp, but only 0.0.5

Also, do we need a rockspec? Or why don't we need one? :D

@mrcjkb
Copy link
Contributor Author

mrcjkb commented Nov 12, 2023

Is there a way to specify the version for dependencies? We currently don't support the newest release of jsregexp, but only 0.0.5

Yes, we can specify something like jsregexp >= 0.0.4, <= 0.0.5. (what's the minimum version required?)

Also, do we need a rockspec? Or why don't we need one? :D

The luarocks-tag-release action generates a rockspec based on GitHub metadata and the specified inputs.
In my experience, that works for most neovim plugins. In special cases, you can specify a rockspec template.

It can still be useful to have a dev rockspec in the repo root for people who want to install the latest head from the main branch, or if you want to use busted for tests.

@L3MON4D3
Copy link
Owner

Ah, great.
We should probably fix jsregex == 0.0.5, it's the newest version with API we support :D

The luarocks-tag-release action generates a rockspec based on GitHub metadata and the specified inputs.
In my experience, that works for most neovim plugins. In special cases, you can specify a rockspec template.

It can still be useful to have a dev rockspec in the repo root for people who want to install the latest head from the main branch, or if you want to use busted for tests.

Makes sense, ty! Don't think we'll need that for now

@mrcjkb
Copy link
Contributor Author

mrcjkb commented Nov 12, 2023

We should probably fix jsregex == 0.0.5, it's the newest version with API we support :D

Done 😄

@L3MON4D3
Copy link
Owner

Nice!
I'll look into making a release in a bit, so there actually is a luarock :D

@L3MON4D3 L3MON4D3 merged commit b8639fe into L3MON4D3:master Nov 13, 2023
@mrcjkb mrcjkb deleted the luarocks branch November 13, 2023 12:02
@L3MON4D3
Copy link
Owner

@L3MON4D3
Copy link
Owner

I guess the rockspec-name should be lowercased?

@mrcjkb
Copy link
Contributor Author

mrcjkb commented Nov 13, 2023

I guess the rockspec-name should be lowercased?

🤔 That looks like a bug either in luarocks or in the luarocks-tag-release action.
I'll look into it after work today.

Update: yup, it has to be the lower case name of the package field

@mrcjkb
Copy link
Contributor Author

mrcjkb commented Nov 13, 2023

I've pushed a fix. It should work if you rerun the workflow.

@L3MON4D3
Copy link
Owner

Looks like that did it!
https://luarocks.org/modules/l3mon4d3/luasnip

Thank you for the PR, and the quick fix :)

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.

2 participants