-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
WIP: downloading lua scripts without git installed #11624
WIP: downloading lua scripts without git installed #11624
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll let @wpferguson comment on the final design, but to me I think this should not be restricted to Windows (here you're using powershell). I think having the option to download via wget directly in Lua without having to install Git would be nice on all platforms.
Now the question should this be the solution or an option and get the Git support?
imnsho, it would be better to only have a non-git download process |
@TurboGit thanks for pinging me. I meant to respond to this when it was an issue, but I've been crazy busy (have to move me and the family to a new house that's currently under renovation in 10 days, while at work I'm doing a major systems upgrade for one client, keeping the rest of the clients happy and bringing on board a new client). TL;DR the repository is versioned and git is used to install the version matching the lua API version of the darktable executable. Let's go back 6 years (actually 6 years and 15 days) to when I submitted my first lua PR.
6 months later....
2 years after that...
6 months after that...
14 months after that...
9 months ago...
Reasons we use git
Alternatives @darix came up with an idea to package the lua scripts so that they could be installed by the system (https://discuss.pixls.us/t/lua-scripts-in-a-system-wide-path-so-packagers-could-ship-lua-scripts/26133/3 and darktable-org/lua-scripts#352). If we do this, then this would solve the versioning issue (i.e. each darktable release would have a corresponding scripts release) and the ease of use (just use the system package installer). Add an option to the installer to install git if it's not present, then script the install? Danger If we decide to change data/luarc I recommend that we do it early in the 4.2 release cycle so that it can get as much testing as possible. We've already learned the hard way about users doing something we didn't expect or plan for and having to scramble to fix it. With the weekly windows builds on pixls and the nightly builds hopefully we can drive out any bugs before release. Sorry for the novel |
Now that I've had a good night's sleep, one last thought... When we had a low rate of API change most of the problems we had were the users needing to update their scripts. The last 3 years we've had a lot of API changes, but we haven't had many issues with scripts needing updated which in turn has lowered the maintenance burden. Almost all of the issues now are scripts that have a bug or feature requests. |
@wpferguson thank you very much for this detailed insights. My approach to use the zip/tar download from github could solve both issues (of course, if it works what isn't sure so far).
I was a bit surprised that the feedback was to use a non-git download as first choice. I would prefer (at least in the first step) to use it as a fallback. If git is not installed on the system, we could try to download the scripts via zip. In the worst case (except for implementation errors of course) this won't work too and we still could tell the user to install git. Of course, there are still a couple of things to solve:
A specific question: Does the lua directory contain any data the is changed during runtime? What, if I delete the directory completely? Do I loose anything except the scripts that I can easily download again? (ok, I can patch my scripts locally but that's not the common end user use case) A lot of error checking must be implemented to get the status of download success etc of course (I think about the API version branches, the choice between master/main branch etc.). Other alternatives, like a completely standalone installer for scripts (each script repo) like it is done in other applications would also be possible. But this would be a lot more work and would break the benefits of simply maintaining the scripts in the repo and have them updateable. (ok, maybe some git action could pack the installer... I don't know...) So what is the preferred direction to take? Give it a try or cancel that idea an let users install git to use the scripts? |
What about linking to libgit instead of requiring user to install git? I haven’t looked at what’s possible with libgit compared to git tool or how hard is it to use it though. I agree that maintaining 2 ways to download scripts isn’t desired, we should choose one. |
I didn't thought about this, really a good idea worth investigating. |
At first sight it seems that libgit supports everything you need when working with git repos. At least way more than we need to download the scripts. Only drawback I see: libgit would have to be integrated into dt core application and interface to lua scripts must be written. |
There are multiple threads all over the internet about the command windows opening when running a command from an application. Unfortunately it seems to be a built in "feature" of windows with no work around. |
Hi, I was doing some tests with libgit, as suggested by @parafin to download the scripts. Only thing I was not able to achieve at all: Integrating libgit as a submodule (or something else) in a way so it gets built from the build process and darktable is linked against that lib. I managed to get the library built and I managed the link process. But not both with one cmake script. I don't know enough about cmake to know how it has to be integrated but I'm sure it is possible. Since my commit breaks the CI builds, I won't PR it here. Maybe someone can help me with the build integration. If that succeeds, I'd continue to modify the script manager to use the library instead of the installed git.exe. Of course, a bit of error handling has to be done too. Thanks in advance for any help 😁 |
do not do intree libraries ever. That is a last resort and should not be the default. |
Ah, thanks a lot for that tip. Well, I have no idea how to do it right in this case. |
just find the system libgit2 and link via https://cmake.org/cmake/help/latest/module/FindPkgConfig.html |
I just wanted you to know about the (non-) progress of this work. In theory, libgit2 is just perfect for doing what we want. I managed to add the prebuilt lib to dt and also added some lua commands to do the clone etc. The current head of the lib, built from scratch, behaves better (or at least different...🙄) but then it would have to be added as submodule... I already opened an issue at libgit2 and tried to contact the devs on their slack channel but got no answer so far 😥 As soon as I find a solution for this problem, I'll continue the work as I'm still convinced that loading/updating the scripts without having git installed is a good thing... |
This pull request did not get any activity in the past 60 days and will be closed in 365 days if no update occurs. Please verify it has no conflicts with the master branch and rebase if needed. Mention it now if you need help or give permission to other people to finish your work. |
This pull reguest was closed because it has been inactive for 300 days since being marked as stale. |
This is more or less a proof of concept. The idea was (as described in darktable-org/lua-scripts#391) that on end user systems git is usually not installed and therefore they are unable to install the lua script extensions.
Currently it's only implemented for Windows (PowerShell needed, i.e. Win 10) but on Linux the tools (wget, unzip/untar) should be available out of the box on most systems I think...
Tested on a complete fresh Win10 sandbox and seems to work fine.
If git is installed, it will be used (so no change is expected on systems where git is available)
Of course, there is a lot more to do:
Feedback wanted: Do you think it is worth continuing the work on that? 🤔