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: support rocks.nvim/luarocks #60

Closed
wants to merge 1 commit into from
Closed

Conversation

mrcjkb
Copy link

@mrcjkb mrcjkb commented Jul 21, 2024

Hey 👋

Summary

This PR is part of a push to get neovim plugins on luarocks.org.

See also:

With luarocks/rocks.nvim, it is the plugin authors' responsibility to declare dependencies and build instructions - not the user's.
Installing this plugin becomes as simple as :Rocks install markview.nvim.

Things done:

  • Add a workflow that publishes tags to luarocks.org when a tag or release is pushed.
  • Add a workflow that automatically publishes SemVer releases.
  • Update readme

See also: this guide

Important

As there are no release tags, this workflow won't do anything yet.
If you do not want to maintain SemVer releases, an alternate option would be a scheduled workflow
that publishes an scm rockspec.
I strongly advise against this, because it means users cannot pin versions or roll back.

Notes:

Important

@folke
Copy link

folke commented Jul 22, 2024

@mrcjkb you should remove that example for lazy.nvim as this is no longer valid.
Plugins can declare dependencies themselves for lazy as well.

@mrcjkb
Copy link
Author

mrcjkb commented Jul 22, 2024

@mrcjkb you should remove that example for lazy.nvim as this is no longer valid. Plugins can declare dependencies themselves for lazy as well.

@folke you mean the one in this repo's readme or in the rocks.nvim readme?

In the rocks.nvim readme, I added a footnote that the example is for lazy.nvim pre-11.0.

In this repo, we would need to add a rockspec or a lazy.lua to the repo root.

I'm hesitant to add a rockspec that includes luarocks tree-sitter parsers in the dependencies, because I'm not sure how well that would work with lazy.nvim.

i.e.:

  • Would it cause a failure if the rocks setting is disabled?
    If so, this would be undesirable because you could still use nvim-treesitter to install the parsers manually.
  • With the way that lazy installs luarocks dependencies, would it be able to make use of the tree-sitter parsers (which have the parser installed in a share/<name>/<version>/parser directory)?
  • If it can, would you be willing to deal with potential issues (e.g. conflicts with nvim-treesitter highlighting queries if they're out of sync)?
    The rocks.nvim user base is still quite small and many of our users have dropped nvim-treesitter in favour of the luarocks parser packages (which we bundle with compatible highlighting queries).

With that in mind, I think maybe a lazy.lua would be a better option for plugins that depend on tree-sitter parsers (neorg is an exception because nvim-treesitter dropped it).

@folke
Copy link

folke commented Jul 22, 2024

Nobody sees that footnote. Please remove that info about lazy. It's not correct.

As for luarocks, I personally couldn't care less. I only have negative experience with it so far.
It's a pain to install. On Macos, it's pretty much impossible,

I'm considering of disabling luarocks support in lazy by default, since too many users have issues with it, that honestly don't even need it. It was a nice experiment for sure, but luarocks just isn't there yet.

Edit: I meant the info here https://github.com/nvim-neorocks/rocks.nvim?tab=readme-ov-file#grey_question-why-rocksnvim

@folke
Copy link

folke commented Jul 22, 2024

lazy is the most widely used plugin manager for NeoVim, so that statement is false for most of the Neovim users.

@mrcjkb
Copy link
Author

mrcjkb commented Jul 22, 2024

On Macos, it's pretty much impossible

rocks.nvim has quite a few macos users; it's just a pita if you use hererocks or homebrew.

luarocks just isn't there yet.

We've discussed this before. Please don't shed a bad light on luarocks because your users are having a bad experience with lazy.nim's implementation.

so that statement is false for most of the Neovim users.

In theory, but not in practice (yet), until most plugins that have dependencies have a rockspec, lazy.lua or a pkg.json in the repo root (see our wiki article on packspec).

If you feel strongly about it, I'll happily remove the lazy example from the rocks.nvim readme this evening when I get back home.
It's not my intention to shed a bad light on lazy.nvim. 😀

@OXY2DEV
Copy link
Owner

OXY2DEV commented Jul 29, 2024

@mrcjkb Sorry for the late reply, I didn't have internet connection and then was busy fixing issues.

Does the file need to be maintained? Or do I have to do something(other than just merging this)?

@mrcjkb
Copy link
Author

mrcjkb commented Jul 29, 2024

No problem 😄

Here are the scenarios where the luarocks.yml file might need maintenance:

  • If you add or remove dependencies
  • If there's a breaking change in the luarocks-tag-release workflow (although you can just keep v7 pinned).
  • If there's a breaking change in luarocks or the luarocks-build-treesitter-parser build backend
    that affects the toolchain that needs to be installed.

do I have to do something(other than just merging this)?

To be able to publish to luarocks, you need an API key (see the note in the PR message).

You need releases or SemVer tags for the workflow to be triggered.
I have not added a workflow that creates automated releases, because I don't know how you want to handle this.
I highly recommend it (see this blog post).

If you don't want to do that, I can add this plugin to the NURR.
The downside of that is:

  • Without SemVer versions, it becomes harder to rollback and pin plugins if something breaks after an update,
    or if there's a breaking change.
  • For plugins in the NURR, we might not notice if dependencies are added or removed.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Jul 29, 2024

Looks like my knowledge is lacking to say the least.

Oh well, guess I am reading more docs.

I will merge this after I actually learn to use workflows(or API keys or any of the other stuff) and get a proper versioning of the plugin set up.

@mrcjkb
Copy link
Author

mrcjkb commented Jul 29, 2024

This is all very new stuff in the Neovim plugin ecosystem 😄
I think this guide has everything you need to know.

And of course, if you feel like it's too much of a maintenance burden, we understand.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Jul 29, 2024

This is all very new stuff in the Neovim plugin ecosystem 😄 I think this guide has everything you need to know.

Huh, neat. Guess I will check that out too.

And of course, if you feel like it's too much of a maintenance burden, we understand.

Nah, even if it is too much maintainance I will still try to add maximum support coverage. I mean I only have a single proper plugin so it shouldn't be too much(plus it's a good learning experience, in case I ever need to work in IT).

@OXY2DEV
Copy link
Owner

OXY2DEV commented Jul 31, 2024

Hey there, @mrcjkb! 👋

I am trying to set up release-please and I am facing an issue.

On the workflow config file I have this.

on:
  push:
    branches:
      - dev

So, I thought it would create new release after I push changes to the dev branch.

However, it didn't seem to work?
Screenshot_2024-07-31-10-32-11-773_mark via gp-edit

It shows an old pull request that I merged.

Did I get it up incorrectly? Or does it not create releases when commits are pushed in the specific branch?

@mrcjkb
Copy link
Author

mrcjkb commented Jul 31, 2024

release-please opens a release PR when you push to the specified branch.
If you merge that, it'll run again and publish the release.
There are ways to auto-merge the release PR, but I don't recommend it. You might want to check the changelog, e.g. for typos in the commit messages.
And you can also leave the PR open to wait for other PRs before releasing.

@OXY2DEV
Copy link
Owner

OXY2DEV commented Jul 31, 2024

release-please opens a release PR when you push to the specified branch.

But why does it not have the changes I commited to the dev branch in it?

Does it only works for changes in the main branch?

@OXY2DEV
Copy link
Owner

OXY2DEV commented Jul 31, 2024

Turns out I had to use the target-branch option.

OXY2DEV added a commit that referenced this pull request Aug 5, 2024
@OXY2DEV
Copy link
Owner

OXY2DEV commented Aug 5, 2024

I couldn't merge this pull request to the dev branch for some reason.

So, I just manually copied the file.

@mrcjkb mrcjkb closed this Aug 5, 2024
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