-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Add a script to install Linux/*BSD dependencies for building #43377
base: master
Are you sure you want to change the base?
Add a script to install Linux/*BSD dependencies for building #43377
Conversation
c68d5c5
to
62468c6
Compare
1b050b4
to
35be826
Compare
What is missing to get this approved and merged? |
4d7f2be
to
9971578
Compare
9971578
to
aa8782c
Compare
This should be ready to merge, but testing on the various BSDs would be appreciated. I haven't had much luck installing any of them in VirtualBox. If someone knows their way around Gentoo, they could also test this PR. |
aa8782c
to
20a6e82
Compare
OpenBSD build is broken. I've tried to build it on freshly installed OpenBSD in QEMU (in VirtualBox I couldn't install it) using the command in the description, and the build is broken. I've found 2 main problems:
Maybe it would make sense to create a separate ticket (GitHib issue) for fixing the build in OpenBSD? |
Yes, I suggest creating a separate issue. Ideally, we should have some kind of CI for BSD (Cirrus CI can do it for instance), so that the build doesn't break again in the future. The issue is that we'd have to use another CI service than GitHub Actions specifically for this, as GitHub Actions doesn't have BSD instances. |
I've created #92130 to fix the build on OpenBSD By the way, the script itself, to install the dependencies, seems working well in OpenBSD 🙂 I tried it in OpenBSD 7.5 amd64 |
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.
Seems like there's a lot of people successfully testing this dependency installer.
There's one review comment that needs to be resolved regarding the sudo message but looks good to me.
I found a cursed github action for freebsd https://github.com/marketplace/actions/freebsd-vm |
20a6e82
to
c311c58
Compare
In general, adding such a script seems fine to me. But the list of dependencies is actually outdated, most of the libs listed are now dlopen'd and we don't need their development packages installed to compile Godot. From the Fedora list:
The actual dependencies I have installed locally on Fedora 40 to compile Godot:
And if I want to use
So I think we should review the list and remove what's not actually needed anymore to compile Godot. |
The command inlined above, if executed without root in Gentoo I have this output: % Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
100 39.4M 0 39.4M 0 0 3286k 0 --:--:-- 0:00:12 --:--:-- 3047k
Detected Linux/*BSD distribution: Gentoo
Installing dependencies...
This action requires portage group access...
*** WARNING *** For security reasons, only system administrators should be
*** WARNING *** allowed in the portage group. Untrusted users or processes
*** WARNING *** can potentially exploit the portage group for attacks such as
*** WARNING *** local privilege escalation.
Would you like to add --pretend to options? [Yes/No] y
emerge: The 'sync' action does not support '--pretend'. You normally shouldn't put a sync in gentoo because sync command tends to take a bit, more than other distros, so I would recommend not syncinc and just outright going for the install, just make sure that the --ask parameter is included in the emerge command. |
I would prefer the script to be able to work in an unattended manner regardless of the distribution currently being used. This is important for CI use cases and the like. This could be done by adding a Ideally, package managers would stop asking for confirmation for safe operations (those that don't remove or downgrade any packages), but I'm not holding my breath for it 🙂 |
Oh boy, CI with gentoo? I think they would just make a ebuild based off the script instead. And a couple hundred cached things... But I can only shiver at the complexity. I think what you could do instead, which I think would be better then, is having an argument like But again, to be fair now that I can think about it, you can alias very easily in portage for the command to always have |
In general, adding such a script is fine. @akien-mga wrote support for this, and I also think it will help new Godot Engine contributors be able to start quickly. However, it is out of date. @Calinou, Are you interested in updating the pull request? |
This closes godotengine/godot-proposals#1614.
The list of commands is taken from Compiling for Linux/*BSD, but with Wayland packages added and unattended CLI arguments (such as
-y
) added.For Alpine,
libexecinfo-dev
has been removed from the package list due to DataDog/dd-trace-php#1824. See also #91817.godotengine/godot-docs#9357 updates the commands in the documentation to be a perfect match for the ones in this script.
Command to test on a fresh system:
All systems listed below are fresh Docker containers.
Successfully tested on
oldstable
), 12 (stable
), 13 (testing
)Not tested on
Please test on these platforms if you can!