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 crash when using --tree on musl #1550

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

leso-kn
Copy link
Contributor

@leso-kn leso-kn commented Dec 4, 2023

This PR addresses the crash described in #1549 and adds the value passed to the --tree argument to cfg.rocks_trees.
Tested and working on alpine:edge running luarocks-5.1 v3.9.2.

@codecov-commenter
Copy link

codecov-commenter commented Dec 4, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4cfcf9d) 79.10% compared to head (afdf03f) 78.97%.

❗ Current head afdf03f differs from pull request most recent head 7ac3a6f. Consider uploading reports for the commit 7ac3a6f to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1550      +/-   ##
==========================================
- Coverage   79.10%   78.97%   -0.13%     
==========================================
  Files          89       89              
  Lines       11092    11088       -4     
==========================================
- Hits         8774     8757      -17     
- Misses       2318     2331      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

* Fixes a crash in `fulfill_dependency()` on musl
@hishamhm
Copy link
Member

The combination of all possible configurations and flags added over the year to tend for numerous scenarios and use-cases often end up leading to some surprising behaviors... A big cleanup of the tree management code would do it good, but given backward compatibility constraints, I prefer keeping changes to this logic minimal for now.

Since that you reported this fixes a crash and the test suite seems to happy about it, I am going to merge your change, thanks!

@hishamhm hishamhm merged commit f4b1dea into luarocks:master Dec 12, 2023
8 checks passed
algitbot pushed a commit to alpinelinux/aports that referenced this pull request Feb 17, 2024
Fixed in luarocks!1550 (merged) but not included in the latest release yet (luarocks v3.9.2 from 2022-12-08).

See luarocks/luarocks#1550
algitbot pushed a commit to alpinelinux/aports that referenced this pull request Feb 17, 2024
Fixed in luarocks!1550 (merged) but not included in the latest release yet (luarocks v3.9.2 from 2022-12-08).

See luarocks/luarocks#1550
algitbot pushed a commit to alpinelinux/aports that referenced this pull request Feb 17, 2024
Fixed in luarocks!1550 (merged) but not included in the latest release yet (luarocks v3.9.2 from 2022-12-08).

See luarocks/luarocks#1550
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.

None yet

3 participants