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

[librsvg] svg GDK pixbuf loader changed names and became 100x bigger #22880

Closed
2 of 6 tasks
StefRe opened this issue Dec 23, 2024 · 18 comments
Closed
2 of 6 tasks

[librsvg] svg GDK pixbuf loader changed names and became 100x bigger #22880

StefRe opened this issue Dec 23, 2024 · 18 comments
Labels

Comments

@StefRe
Copy link

StefRe commented Dec 23, 2024

Description / Steps to reproduce the issue

instead of libpixbufloader-svg.dll there is only a pixbufloader_svg.dll now that is 100 times bigger (and the total package size doubled from 8 to 16 MB):

Content of mingw64\lib\gdk-pixbuf-2.0\2.10.0\loaders:

mingw-w64-x86_64-librsvg-2.58.0-1-any.pkg.tar.zst (8.3 MB):
04.04.2024  07:58             4.176 libpixbufloader-svg.a
04.04.2024  07:58            17.383 libpixbufloader-svg.dll
04.04.2024  07:58             2.298 libpixbufloader-svg.dll.a

mingw-w64-x86_64-librsvg-2.59.2-1-any.pkg.tar.zst (16.1 MB):
01.11.2024  22:33         1.741.243 pixbufloader_svg.dll

I've read the related discussion in #22847, which got rather heated up. Therefore I opened a separate issue with the simple question if this name change and size increase in intended or just a mistake. I must admit that I am completely uninformed about the inner workings of the libraries here, so please bear with me.
See pdfarranger/pdfarranger#1174 for the background of this question.

Expected behavior

No name change and only moderate size increase.

Actual behavior

see description above

Verification

Windows Version

MINGW64_NT-10.0-19045

MINGW environments affected

  • MINGW64
  • MINGW32
  • UCRT64
  • CLANG64
  • CLANGARM64

Are you willing to submit a PR?

No response

@StefRe StefRe added the bug label Dec 23, 2024
@techee
Copy link

techee commented Dec 23, 2024

@StefRe Thanks a lot for creating a separate issue - I really encourage anyone not to waste their time on #22847 which I'm going to close now.

I'd just summarize some observations:

  1. For Geany, which uses the debian cross-compiler, we stopped getting working SVG somewhere between October 15 and October 17
  2. To me it seems it might be caused by a2aaa55 which contains quite many build-related changes.
  3. When running gdk-pixbuf-query-loaders.exe from the cross-compiler environment, we started getting these messages
002b:err:module:import_dll Library bcryptprimitives.dll (which is needed by L"Z:\\build\\gtk-bundle\\bin\\librsvg-2-2.dll") not found

and lib/gdk-pixbuf-2.0/loaders.cache stopped getting updated with the SVG loader. This library isn't part of msys but Windows itself and is not available from the cross-compiler environment. There didn't use to be this issue before October 15.
4. When updating lib/gdk-pixbuf-2.0/loaders.cache manually, SVG started working on the target Windows machine.

A possible workaround for us is to update the loaders.cache manually if there's not a better solution, but I think it would be good to know what is going on and if there's not some problem with the compilation of the loader.

@techee
Copy link

techee commented Dec 23, 2024

Just an idea - isn't it possible that the loader got linked to the actual librsvg.dll statically instead of dynamically? That would explain the library size and also the direct dependency of the loader library on bcryptprimitives.dll that would otherwise be only present in the main librsvg library and (probably) not detected by the cache-updating binary.

@Biswa96
Copy link
Member

Biswa96 commented Dec 24, 2024

2. To me it seems it might be caused by a2aaa55 which contains quite many build-related changes.

Thanks for the hint. The DLL name was changed according to this log https://github.com/msys2/MINGW-packages/actions/runs/11340312284/job/31536524759?pr=21684#step:11:1982

@Biswa96
Copy link
Member

Biswa96 commented Dec 24, 2024

In Windows, the svg pixbuf loader is found without any issue.

$ gdk-pixbuf-query-loaders.exe | grep -A 999 _svg
"lib\\gdk-pixbuf-2.0\\2.10.0\\loaders\\pixbufloader_svg.dll"
"svg" 6 "gdk-pixbuf" "Scalable Vector Graphics" "LGPL"
"image/svg+xml" "image/svg" "image/svg-xml" "image/vnd.adobe.svg+xml" "text/xml-svg" "image/svg+xml-compressed" ""
"svg" "svgz" "svg.gz" ""
" <svg" "*    " 100
" <!DOCTYPE svg" "*             " 100

@MehdiChinoune
Copy link
Collaborator

I don't understand what a debian cross-compiler to do with MSYS2 environment?
Could you explain whether you are using Debian or MSYS2.

@MehdiChinoune
Copy link
Collaborator

The change of name and size of the library is not coming from MSYS2, please report the issue to librsvg developers.
By looking at #22847, the issue is not coming from MSYS2. MSYS2 doesn't fix issues related to Debian cross-compiler.
Closing as invalid.

@MehdiChinoune MehdiChinoune closed this as not planned Won't fix, can't repro, duplicate, stale Dec 24, 2024
@MehdiChinoune MehdiChinoune added invalid and removed bug labels Dec 24, 2024
@techee
Copy link

techee commented Dec 24, 2024

The change of name and size of the library is not coming from MSYS2, please report the issue to librsvg developers.

@MehdiChinoune It's the mingw PKGBUILD file that got changed in a2aaa55 and there's nothing to report to the upstream project. This file is your responsibility. Notice the line

a2aaa55#diff-0db33fcf01a542992efe526a1612edefac816309632a06fc2b7348ce2ae5b66cR85

and various "static" lines below - as I said, I believe (I may be wrong of course) that by this change you started linking the whole librsvg library to the loader statically. This is definitely not a standard thing to do - on all other platforms, loaders are just small libraries that don't contain the actual image decoding code. Also, you don't do such a thing for neither of the other image loaders - all of them are linked dynamically, except SVG. This leads to the huge binary size, and, in addition, it probably introduces the bcryptprimitives.dll dependency to the loader which causes problems with the cross-compiler.

So your analysis and the reason for closing this issue is not valid IMO.

@lazka Any hint on the changes you made to the build process of librsvg? Maybe you had a good reason to link the actual librsvg library to the loader statically in which case we will have to live with it. But since it's a very non-standard thing to do, to me it just looks like something done by accident which is why I reported this issue but I didn't get a valid answer yet.

@techee
Copy link

techee commented Dec 24, 2024

OK, I'm probably wrong about the "static linking" thing. The actual librsvg-2-2.dll library is much larger and the loader itself doesn't work without it. Still, I believe something weird happened in the commit I mentioned.

@Biswa96
Copy link
Member

Biswa96 commented Dec 24, 2024

it probably introduces the bcryptprimitives.dll dependency to the loader

bcryptprimitives.dll is a valid system DLL in Windows.

@techee
Copy link

techee commented Dec 24, 2024

Alright, I checked a2aaa55 in and the subsequent commits in more detail and it's primarily about switching from autotools to meson for the build. And I'm afraid we won't be able to figure out what exactly is different when meson gets used for the build because it can be lots of things. There doesn't seem to be anything obviously wrong.

So probably nothing to do here and you can keep this issue closed. We'll have to handle the cache update in some other way in Geany.

@techee
Copy link

techee commented Dec 24, 2024

bcryptprimitives.dll is a valid system DLL in Windows.

Yes, that's clear. I was just hoping to get to the bottom of the issue, and, if possible, figure out what exactly triggered its dependency and undo the change so gdk-pixbuf-query-loaders.exe could still be run from the wine environment.

But with the autotools->meson transition, this just isn't an easy task and probably not worth spending time on it.

@MehdiChinoune
Copy link
Collaborator

@techee you mentioned earlier that you are using debian, why report the issue here?
MSYS2 just package and It has no responsibility on Why the upstream uses a certain buildsystem. librsvg moved to meson. So ask them or tell them about your issue.

@techee
Copy link

techee commented Dec 24, 2024

@techee you mentioned earlier that you are using debian, why report the issue here?
MSYS2 just package and It has no responsibility on Why the upstream uses a certain buildsystem. librsvg moved to meson. So ask them or tell them about your issue.

@MehdiChinoune Because I thought it was caused by the change you made to your PKGBUILD file in a2aaa55 in which case msys2 would be the right place to report the issue.

But it seems (that said, I'm not 100% sure) it has something to do with the meson build itself and, yes, in that case it should be reported to the upstream project. But for that I'd probably have to investigate more what exactly is going on and I probably don't have time for that now.

Anyway, thanks for the assistance and sorry for the noise.

@MehdiChinoune
Copy link
Collaborator

MehdiChinoune commented Dec 24, 2024

I still don't understand what msys2 to do with debian, debian uses its own formulas to build packages!
Are you mixing Debian+MSYS2?

@lazka
Copy link
Member

lazka commented Dec 24, 2024

google gives me https://groups.google.com/g/golang-nuts/c/Msg1USaNaqM which suggests that newer wine (I assume you use wine for the cross compile) provides bcryptprimitives.dll and rust-lang/rust#128066 suggests that this new requirements comes from newer rust.

sorry for not providing better commit messages there. the PKGBUILD contain links to context for the patches though

@techee
Copy link

techee commented Dec 24, 2024

@lazka Big thanks! That's exactly the information I was looking for. So the rust problem is clear then, not much to do. And good to know that once we get a newer version of wine, it gets fixed - we can patch the cache file by ourselves in the meantime.

To address the question of the OP about the strange loader name - it got introduced in librsvg itself as part of

https://gitlab.gnome.org/GNOME/librsvg/-/commit/06ec80a370aaeffb3f86f37a967b3c6061f60a8d

I'm not quite sure why it was done, the actual commit seems to be about something different.

@techee
Copy link

techee commented Dec 24, 2024

I still don't understand what msys2 to do with debian, debian uses its own formulas to build packages!
Are you mixing Debian+MSYS2?

@MehdiChinoune FYI, Geany is using the mingw64 cross compiler

https://salsa.debian.org/mingw-w64-team/gcc-mingw-w64/-/blob/master/debian/gcc-mingw-w64-base.README.Debian

and fetches msys2 packages for the resulting installer which is provided as an artifact of the build so Windows users can test the CI changes easily. The update of the pixbuf cache is made as part of this process:

https://github.com/geany/geany/blob/master/scripts/gtk-bundle-from-msys2.sh#L203

@MehdiChinoune
Copy link
Collaborator

That's not supported by MSYS2.
MSYS2 is for building native packages on Windows nothing more.
If you mix between packages cross-built from Linux then you should figure out yourself the issue.
That's it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants