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

Bump the Lua minimum version to 5.3. #1738

Merged
merged 2 commits into from Aug 6, 2021

Conversation

ppentchev
Copy link
Contributor

Hi,

First of all, thanks a lot for working on rpm and the related projects!

The 986be66 commit introduced a use of
lua_rotate(), which is not available in Lua 5.2. Hence, bump the dependency in
the configure script's pkg-config check.

Thanks in advance for your time, and keep up the great work!

G'luck,
Peter

The 986be66 commit introduced a use of
lua_rotate(), which is not available in Lua 5.2.
@ffesti ffesti requested a review from pmatilai June 30, 2021 09:02
@ffesti
Copy link
Contributor

ffesti commented Jun 30, 2021

Yes, something needs to be done here. I am not merging this right away, as we are pretty late in 4.17 and I am not that much into this code. Bumping required versions for something like lua requires a bit of consideration (although lua 5.3 is probably fine). As it is vacation time this may take a while, but we will definitely do something about this before the 4.17 release.

Thanks for the report (and patch)!

Copy link
Member

@Conan-Kudo Conan-Kudo left a comment

Choose a reason for hiding this comment

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

I don't think it's a problem to merge this, we clearly intended to raise the Lua version accordingly when we gutted all the backwards compat code and reworked it for Lua 5.4 support.

Copy link
Member

@pmatilai pmatilai left a comment

Choose a reason for hiding this comment

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

Introducing a dependency on 5.3 in that commit was unintentional, but then again... 5.3 is almost seven years old by now, it doesn't seem like an entirely unreasonable requirement 😆

However INSTALL needs to be updated accordingly.

@pmatilai pmatilai merged commit 24b6c45 into rpm-software-management:master Aug 6, 2021
@pmatilai
Copy link
Member

pmatilai commented Aug 6, 2021

Thanks for spotting this and the patch!

@ppentchev ppentchev deleted the roam-lua5.3 branch August 6, 2021 08:04
pmatilai pushed a commit to pmatilai/rpm that referenced this pull request Aug 19, 2021
Bump the Lua minimum version to 5.3.

The 986be66 commit introduced a use of
lua_rotate(), which is not available in Lua 5.2, unintentionally causing a dependency on 5.3. Rather than work around it, just bump the requirement to 5.3, it's almost seven years old by now...

(cherry picked from commit 24b6c45)
pmatilai pushed a commit that referenced this pull request Aug 20, 2021
Bump the Lua minimum version to 5.3.

The 986be66 commit introduced a use of
lua_rotate(), which is not available in Lua 5.2, unintentionally causing a dependency on 5.3. Rather than work around it, just bump the requirement to 5.3, it's almost seven years old by now...

(cherry picked from commit 24b6c45)
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

4 participants