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

fix(luarocks): try to install from root manifest #1687

Merged
merged 1 commit into from
Aug 31, 2024
Merged

Conversation

mrcjkb
Copy link
Contributor

@mrcjkb mrcjkb commented Jul 28, 2024

Description

When passing the --dev flag to luarocks, it will prioritise dev versions when resolving dependencies (treating dev or scm as greater than a SemVer version) if the rockspec doesn't specify an upper version constraint (which is often the case).

Dev packages are often unstable and may cause more problems, especially for Windows users (an example I've seen is git for windows trying and failing to checkout submodules).

For now , a good compromise between too many retries and not retrying at all could be to try luarocks install from the root manifest first, but to keep the --dev flag in luarocks make.

If that still causes problems, it might be better to fall back to luarocks make without --dev first, and then to try luarocks ---dev make as a last resort.
In rocks.nvim, we only fall back to adding the --dev flag if the install error message contains the string "No results matching query were found"; assuming that stable non-dev packages shouldn't depend on dev packages.

Copy link
Contributor

This PR is stale because it has been open 30 days with no activity.

@github-actions github-actions bot added the stale label Aug 27, 2024
@mrcjkb
Copy link
Contributor Author

mrcjkb commented Aug 27, 2024

not stale

@github-actions github-actions bot removed the stale label Aug 28, 2024
@folke folke merged commit 591ef40 into folke:main Aug 31, 2024
9 checks passed
@folke
Copy link
Owner

folke commented Aug 31, 2024

ty!

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