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: stardust server #3773

Draft
wants to merge 1 commit into
base: frawhide
Choose a base branch
from
Draft

Fix: stardust server #3773

wants to merge 1 commit into from

Conversation

Owen-sz
Copy link
Member

@Owen-sz Owen-sz commented Mar 7, 2025

Add runtime libraries and other packages needed for misc stardust server usages

Add runtime libraries and other packages needed for misc stardust server usages

Signed-off-by: Owen Zimmerman <[email protected]>
@RockGrub
Copy link
Member

RockGrub commented Mar 7, 2025

Are all of these required for the server to function or could some be a Suggests: instead?

@Owen-sz
Copy link
Member Author

Owen-sz commented Mar 7, 2025

They're needed if you wanna load or do anything in the server beyond very basic functionality.

Comment on lines +21 to +23
Requires: libxkbcommon libstdc++ libX11 libXfixes libEGL libgbm fontconfig pcre2
Requires: libgcc glibc libxcb libglvnd libdrm expat freetype libxml2 libXau libbrotli
Requires: zlib-ng-compat bzip2-libs libpng harfbuzz libbrotli xz-libs glib2 graphite2
Copy link
Member

Choose a reason for hiding this comment

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

maybe remove things listed in buildreqs already?

Copy link
Member Author

Choose a reason for hiding this comment

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

The binary links against all of these, so wouldn't they need to all be runtime deps?

Copy link
Member

Choose a reason for hiding this comment

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

RPM detects all the shared dependencies needed, which you can check with dnf rq --requires ./path/to/idk.rpm

the Requires: field is specifically for things that are not linked. For example if your binary runs vim inside bash then both vim and bash should be specified as a dependency since they are not linked.

Remember last time we tested stardust with Nova? Well I suspect everything has already been specified correctly by rpm automatically since it ran "fine"

If you see something like "undefined symbol" or "symbol lookup error" then 1. there's something wrong with the linking process and/or 2. you might need to specify some extra stuff in Requires

You may choose to continue specifying all the runtime dependencies but it probably does nothing and you increase maintenance burden, since if a newer version doesn't need a dep anymore you'll need to manually remove the Requires

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh, I see thank you. I'll do some more testing and talk to Nova.

@RockGrub
Copy link
Member

RockGrub commented Mar 8, 2025

They're needed if you wanna load or do anything in the server beyond very basic functionality.

That is fair, but generally things not needed for the program itself to run should be weak deps so people can exclude the packages they may not want for any reason (not using a specific feature, etc.)

@Owen-sz
Copy link
Member Author

Owen-sz commented Mar 8, 2025

They're needed if you wanna load or do anything in the server beyond very basic functionality.

That is fair, but generally things not needed for the program itself to run should be weak deps so people can exclude the packages they may not want for any reason (not using a specific feature, etc.)

Theoretically they should all be needed for full functionality.

@Owen-sz Owen-sz requested a review from madonuko March 13, 2025 04:17
@Owen-sz Owen-sz marked this pull request as draft March 13, 2025 04:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants