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.

Sign up for GitHub

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

Jump to bottom

fix(sync): prevent luarocks install tree race conditions #63

Merged
merged 7 commits into from
Dec 11, 2023
Merged

fix(sync): prevent luarocks install tree race conditions #63

merged 7 commits into from
Dec 11, 2023

Conversation

mrcjkb
Copy link
Member

@mrcjkb mrcjkb commented Dec 10, 2023 •
edited
Loading

Fixes #61

@mrcjkb mrcjkb added the bug Something isn't working label Dec 10, 2023
@mrcjkb mrcjkb self-assigned this Dec 10, 2023
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 10, 2023 •
edited by mrcjkb
Loading

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 Dec 10, 2023

Something seems to be completely broken with the user_rocks in this branch.
vim.print prints out { "__path" }.

@mrcjkb mrcjkb marked this pull request as draft December 10, 2023 00:56
@nvim-neorocks nvim-neorocks deleted a comment from github-actions bot Dec 10, 2023
@mrcjkb mrcjkb force-pushed the fix-sync-prunes branch 2 times, most recently from 7b76c67 to 4a5efec Compare December 10, 2023 01:17
@mrcjkb
Copy link
Member Author

mrcjkb commented Dec 10, 2023

Something seems to be completely broken with the user_rocks in this branch. vim.print prints out { "__path" }.

Fixed it in #60.

I accidentally replaced toml with toml-edit as the parser.

@mrcjkb
Copy link
Member Author

mrcjkb commented Dec 10, 2023 •
edited
Loading

@mrcjkb mrcjkb force-pushed the fix-sync-prunes branch 2 times, most recently from 05afaca to adc9c54 Compare December 10, 2023 02:18
@mrcjkb
Copy link
Member Author

mrcjkb commented Dec 10, 2023 •
edited
Loading

  • It looks like install/upgrade/downgrade has to be sequential, too.
  • And we should probably implement a locking mechanism to prevent users from spamming sync/install, etc.

@mrcjkb mrcjkb changed the title fix(sync): prune rocks sequentially to prevent partial uninstalls fix(sync): prevent luarocks install tree race conditions Dec 11, 2023
@mrcjkb mrcjkb marked this pull request as ready for review December 11, 2023 00:38
@mrcjkb mrcjkb mentioned this pull request Dec 11, 2023
Merged
@vhyrro vhyrro closed this Dec 11, 2023
@vhyrro vhyrro reopened this Dec 11, 2023
@vhyrro
Copy link
Collaborator

vhyrro commented Dec 11, 2023

Woopsie...

Base automatically changed from fix-prune to master December 11, 2023 17:11
@mrcjkb mrcjkb merged commit 52f1ae6 into master Dec 11, 2023
4 of 6 checks passed
@mrcjkb mrcjkb deleted the fix-sync-prunes branch December 11, 2023 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NTBBloodbath NTBBloodbath Awaiting requested review from NTBBloodbath NTBBloodbath is a code owner

@vhyrro vhyrro Awaiting requested review from vhyrro vhyrro is a code owner

Assignees

@mrcjkb mrcjkb

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] Rocks sync should not invoke luarocks [install|uninstall|update] concurrently
2 participants
@mrcjkb @vhyrro