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

runtime-install: install rsvg-pixbuf-loader #1334

Closed
wants to merge 1 commit into from

Conversation

AdamWill
Copy link
Contributor

@AdamWill AdamWill commented Jul 21, 2023

This was split out of librsvg2 as a subpackage recently, and is only Recommended by the parent (not Required). For size's sake, we don't pull Recommends into the installer env by default. But we do really need this package, because quite a lot of icons used in the installer are only present as SVGs in the installer environment, so if we don't have the SVG loader we can't show the icons.

Right now anaconda actually crashes if the SVG renderer isn't present, which wasn't an expected outcome, but even if the crash is fixed, the result would be that anaconda would run but not show a lot of icons, which obviously isn't what we want. So we should add this regardless of a fix for the crash.

@coveralls
Copy link

coveralls commented Jul 21, 2023

Pull Request Test Coverage Report for Build 5631471302

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 43.325%

Totals Coverage Status
Change from base Build 5557115468: 0.0%
Covered Lines: 1578
Relevant Lines: 3435

💛 - Coveralls

Comment on lines 108 to 111
## this is only recommended by librsvg2, but anaconda crashes without it
## see https://bodhi.fedoraproject.org/updates/FEDORA-2023-f069ac712b
installpkg rsvg-pixbuf-loader

Choose a reason for hiding this comment

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

You could also just say:

Suggested change
## this is only recommended by librsvg2, but anaconda crashes without it
## see https://bodhi.fedoraproject.org/updates/FEDORA-2023-f069ac712b
installpkg rsvg-pixbuf-loader
## required to display SVG images
installpkg rsvg-pixbuf-loader

i.e. it would be needed even without the crash

Copy link
Contributor Author

@AdamWill AdamWill Jul 22, 2023

Choose a reason for hiding this comment

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

OK, I'll tweak that.

One thing I think we maybe can tell from the backtrace: it's trying to display an icon which only exists (in the installer environment at least) as an SVG. Frame 11 mentions input-keyboard-symbolic , and in the installer env, the only file with that in its name is /usr/share/icons/Adwaita/symbolic/devices/input-keyboard-symbolic.svg. IIRC there is some kinda hidden magic fallback to a non-symbolic version somewhere in the codepath, but even with that, the only input-keyboard icons we have are also SVGs, there is no PNG input-keyboard* anywhere.

So maybe there's a path where GTK just assumes it will definitely be able to render the icon somehow once it's been found, but because we don't have the SVG renderer and we don't have any non-SVG icons, that assumption breaks? Or something along those lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

comment and commit message updated, thanks.

Choose a reason for hiding this comment

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

It's not possible to know without understanding the missing frame in the backtrace. What I can say for sure is that it shouldn't crash inside gtk_widget_show_all().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, that does seem odd. I tried following it out from that end but it's a long way. it gets into glib deciding whether the thing is a widget, which involves trying to instantiate it as one, best as I can tell...

I think the thing to do might be to try and write a simpler reproducer which doesn't involve python and gobject introspection, maybe? just...see what happens if you do stuff with an icon that's present, but only as SVG, without the SVG pixbuf loader being there...

This was split out of librsvg2 as a subpackage recently, and
is only Recommended by the parent (not Required). For size's
sake, we don't pull Recommends into the installer env by
default. But we do really need this package, because quite a
lot of icons used in the installer are only present as SVGs in
the installer environment, so if we don't have the SVG loader we
can't show the icons.

Right now anaconda actually crashes if the SVG renderer isn't
present, which wasn't an expected outcome, but even if the crash
is fixed, the result would be that anaconda would run but not
show a lot of icons, which obviously isn't what we want. So we
should add this regardless of a fix for the crash.

Signed-off-by: Adam Williamson <[email protected]>
@bcl
Copy link
Contributor

bcl commented Jul 24, 2023

This belongs in anaconda-install-env-deps, not in the template. Or maybe even on anaconda-gui.

@bcl bcl closed this Jul 24, 2023
@AdamWill
Copy link
Contributor Author

oh, right, I kinda forgot about anaconda-install-env-deps. you're right, that would be better. so many places...

@AdamWill
Copy link
Contributor Author

for now the dep was changed to a Requires: anyway which makes the whole point moot, so I think I'll leave it for a bit and see whether there's any danger of it going back to Recommends:.

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.

4 participants