Skip to content

Make Lua package required#20691

Open
wpferguson wants to merge 2 commits intodarktable-org:masterfrom
wpferguson:make_lua_required
Open

Make Lua package required#20691
wpferguson wants to merge 2 commits intodarktable-org:masterfrom
wpferguson:make_lua_required

Conversation

@wpferguson
Copy link
Copy Markdown
Member

Lua is used to fix issues in master, but is currently an optional feature. In order the issue fixes to be applied Lua must be present, so it needs to be a required package.

Fixes #20683

@wpferguson wpferguson changed the title Make lua required Make Lua package required Mar 27, 2026
@victoryforce
Copy link
Copy Markdown
Collaborator

@wpferguson - Sorry for not commenting on your RFC in time, but I believe we should NOT make building with Lua mandatory. Integrating Lua into darktable code can cause issues and it is vital to be able to build darktable without Lua for debugging purposes.

@TurboGit
Copy link
Copy Markdown
Member

Right, IIRC this could cause issues for GNU/Linux packagers.

@victoryforce
Copy link
Copy Markdown
Collaborator

If a feature is enabled by default, then the inability to build with that feature due to missing prerequisites should result in a build failure, rather than silently disabling the feature and building successfully without it.

@wpferguson
Copy link
Copy Markdown
Member Author

@victoryforce I replied to your PR before reading this.

Right, IIRC this could cause issues for GNU/Linux packagers.

I don't know enough about packaging to understand. The only issue I see would be shipping 2 versions of Lua though I'm not sure why that would be a problem. I have 5.1, 5.2, 5.3, and 5.4 on my system with no issues. The packagers could make 5.5 the default and include 5.4 for compatibility reasons.

Integrating Lua into darktable code can cause issues

Could you give me an example?


@TurboGit I think my solution is the correct one 😄 , but I can live with @victoryforce's solution. So, you choose.

@victoryforce
Copy link
Copy Markdown
Collaborator

Integrating Lua into darktable code can cause issues

Could you give me an example?

I don't really understand why this needs to be proven with examples. Lua is very deeply integrated into many critical parts of the darktable code. If your proposed change is to make it impossible to throw this code out for debugging purposes, then you should prove that Lua integration cannot cause issues.

In any case, doesn't the quote from the comment in your last PR serve as an example?

"If the current view is set to lighttable, but the view hasn't completely initialized, and Lua attempts to install the lib the result will be a hang." (emphasis mine, of course)

@TurboGit I think my solution is the correct one 😄 , but I can live with @victoryforce's solution. So, you choose.

I'm not disputing that your solution is correct, of course it is correct in the sense that it solves the problem of a regression unnoticed by the package maintainer when rebuilding the package. I'm trying to show that it is not optimal considering other factors.

@wpferguson
Copy link
Copy Markdown
Member Author

but the view hasn't completely initialized, and Lua attempts to install the lib the result will be a hang."

That's not a Lua problem, it's a view/views.c problem. Lua just makes it easier to reproduce the issue.

My concern is that if it's a feature, then it's considered optional and I think we're past that point.

@victoryforce
Copy link
Copy Markdown
Collaborator

but the view hasn't completely initialized, and Lua attempts to install the lib the result will be a hang."

That's not a Lua problem, it's a view/views.c problem. Lua just makes it easier to reproduce the issue.

I'm not trying to find the culprit for the problem and say it's Lua. This is just an example of how supporting Lua creates additional execution paths. The bug could be somewhere else, of course, but having Lua present could expose it and lead to bad consequences. Again, this isn't about the culprit. This is about the execution paths.

My concern is that if it's a feature, then it's considered optional and I think we're past that point.

Distributions don't try to package apps with disabled features, it just doesn't make sense. That's not their approach.

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.

RFC: Use internal Lua as fallback if Lua 5.4 not available

3 participants