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: lockfile support #244

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

feat: lockfile support #244

wants to merge 1 commit into from

Conversation

mrcjkb
Copy link
Member

@mrcjkb mrcjkb commented Apr 5, 2024

Closes #34.

The documentation says luarocks will use a lockfile if there is one in the cwd.
But I haven't confirmed this yet.

This needs some vigorous testing.

How it works

Adding/updating/removing rocks:

  • We add a --pin flag to the install command
  • luarocks creates a luarocks.lock file in the rock's install tree (for all transitive dependencies of the installed rock)
  • We add the path to the rtp
  • We load the lockfile and use it to update our rocks-lock.toml

Syncing rocks:

  • We get the locked dependencies from our rocks-lock.toml
  • We use those to create a luarocks.lock in a temp directory
  • We set the cwd to the location of the lock file for any install commands

@mrcjkb mrcjkb marked this pull request as draft April 5, 2024 01:25
Copy link
Contributor

github-actions bot commented Apr 5, 2024

Review Checklist

Does this PR follow the Contribution Guidelines? Following is a partial checklist:

Proper conventional commit scoping:

  • For example, fix(installer): some installer bugfix

  • Pull request title has the appropriate conventional commit prefix.

If applicable:

  • Tested
    • Tests have been added.
    • Tested manually (steps in PR description).
  • Updated documentation.

@mrcjkb
Copy link
Member Author

mrcjkb commented Apr 5, 2024

🤔 maybe we should just call the file rocks.lock instead of rocks-lock.toml.

lua/rocks/operations/lock.lua Outdated Show resolved Hide resolved
@mrcjkb
Copy link
Member Author

mrcjkb commented Apr 5, 2024

I just can't get the test cases to succeed (╯°□°)╯︵ ┻━┻

It looks like it could be something to do with the neovim scheduler? Not sure how to get busted working with that.

@mrcjkb mrcjkb force-pushed the rocks-lock branch 3 times, most recently from 167af51 to 0132598 Compare April 7, 2024 00:04
@mrcjkb
Copy link
Member Author

mrcjkb commented Apr 7, 2024

This PR is pretty much ready. But I would like to merge #245 first.

@mrcjkb mrcjkb force-pushed the rocks-lock branch 3 times, most recently from 55b05d6 to 403b485 Compare April 7, 2024 12:04
@mrcjkb mrcjkb marked this pull request as ready for review April 7, 2024 12:04
@mrcjkb
Copy link
Member Author

mrcjkb commented Apr 7, 2024

Todo:

  • Update the new sync_spec.lua to install a package that has a locked dependency, and confirm that the locked dependency is indeed installed.

@mrcjkb
Copy link
Member Author

mrcjkb commented Apr 7, 2024

Looks like luarocks is ignoring the luarocks.lock file in the install cwd.

@mrcjkb
Copy link
Member Author

mrcjkb commented Apr 8, 2024

Blocked by luarocks/luarocks#1662

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.

[Feature] support pinning dependencies
1 participant