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

[FEATURE]: Implement an install script for rocks.nvim #30

Merged
merged 31 commits into from
Nov 27, 2023
Merged

Conversation

vhyrro
Copy link
Collaborator

@vhyrro vhyrro commented Nov 24, 2023

This PR adds a fully working installation process using a remote :source command invocation.

While there are still a few rough edges I believe it's a good enough MVP to push into main :)

Copy link
Member

@mrcjkb mrcjkb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Do we need to configure luarocks to use the rocks-binaries so that rocks.nvim can update itself?
Otherwise, users would have to have the rust and C++ toolchains installed, wouldn't they?

@vhyrro
Copy link
Collaborator Author

vhyrro commented Nov 25, 2023

Yep, without the binary rocks users would need to have a modern CPP compiler and rust tool chain. If luarocks can't find the binary rock for the target architecture it'll try compiling instead.

Copy link
Member

@NTBBloodbath NTBBloodbath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@NTBBloodbath
Copy link
Member

question: Do we need to configure luarocks to use the rocks-binaries so that rocks.nvim can update itself?
Otherwise, users would have to have the rust and C++ toolchains installed, wouldn't they?

How could we do that? Since it is the dependencies that we want to extract from rocks-binaries and not the complete package 🤔

@mrcjkb
Copy link
Member

mrcjkb commented Nov 26, 2023

question: Do we need to configure luarocks to use the rocks-binaries so that rocks.nvim can update itself?
Otherwise, users would have to have the rust and C++ toolchains installed, wouldn't they?

How could we do that? Since it is the dependencies that we want to extract from rocks-binaries and not the complete package 🤔

Maybe we have to specify the dependencies separately - or we can add rocks.nvim to the rocks-binaries manifest, but not as a binary. And add a server option to the rocks.toml settings or something.

@vhyrro
Copy link
Collaborator Author

vhyrro commented Nov 26, 2023

Not sure if I understand the questions being asked here - the server that we configure is is simply the preferred server. When luarocks tries to pull in rocks.nvim, it notices the dependencies, then first tries to pull from our server before falling back to the regular luarocks manifest. Everything works without interruptions :)

@mrcjkb
Copy link
Member

mrcjkb commented Nov 26, 2023

Not sure if I understand the questions being asked here - the server that we configure is is simply the preferred server. When luarocks tries to pull in rocks.nvim, it notices the dependencies, then first tries to pull from our server before falling back to the regular luarocks manifest. Everything works without interruptions :)

That's the install script. What about updates? I don't think we specify --server for Rocks sync?

@vhyrro
Copy link
Collaborator Author

vhyrro commented Nov 26, 2023

Ah you're right. I was certain I also added that, but apparently I didn't. All rocks operations should also pass the --server flag

@mrcjkb
Copy link
Member

mrcjkb commented Nov 27, 2023

I just tried running the installer (from a local file).

  • There was a graphical glitch when editing the installation path:

installer

  • I was unable to change the setup_luarocks option from true to false (E21: Cannot make changes, 'modifiable' is off) and it had a similar graphical glitch.

@vhyrro
Copy link
Collaborator Author

vhyrro commented Nov 27, 2023

Hm, weird. I guess it has to do with some calculations that the installer is making. Let me see if I can fix that.

@vhyrro
Copy link
Collaborator Author

vhyrro commented Nov 27, 2023

Should be fixed now! :D

@vhyrro vhyrro merged commit 8564817 into master Nov 27, 2023
6 checks passed
@vhyrro vhyrro deleted the installer branch November 27, 2023 18:31
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