flathub / com.adobe.Reader

https://flathub.org/apps/details/com.adobe.Reader
3 stars 4 forks source link

Fix #2 #8

Closed konradmb closed 1 year ago

konradmb commented 1 year ago

The main SIGSEGV issue with gtk appears to be the gcc optimization flag. Changing to -O0 fixes this problem.

Depends on #7, closes #2

flathubbot commented 1 year ago

Started test build 38066

flathubbot commented 1 year ago

Build 38066 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/20709/com.adobe.Reader.flatpakref
hmenke commented 1 year ago

Would it be possible to use GTK from shared-modules without copying everything into this repo? The only change is with respect to shared-modules is CFLAGS=-O0, right?

Erick555 commented 1 year ago

-O0 can by passed through cflags option.

konradmb commented 1 year ago

@Erick555 How to pass it to the included module? It only works if I add it globally, which is not a good solution.

@hmenke Right. I can't find a way to pass options to included module. Seems like the only way is to modify included file. Tried submodule which inculdes a submodule, doesn't work.

Erick555 commented 1 year ago

Yeah you can't override shared-module content. I think overriding it globally maybe not necessarily bad since other than gtk2 there is only gdk-pixbuff and you may override it back with -O2 or even leave as is.

konradmb commented 1 year ago

@Erick555 You have to remember about it every time in the future + I want to add cups module for printing (so more than gdk-pixbuf)

Copying everything to this repo is not that bad. Git submodule is frozen to same contents too.

Erick555 commented 1 year ago

What cups module? cups is in the runtime.

BTW: gdk-pixbuf2 is also in runtime, why it's being build here?

hmenke commented 1 year ago

@Erick555 The runtime is 64 bit but Adobe Reader is only available for 32 bit, so you have to cross-compile all the libraries.

@konradmb I'll try to pull in the GTK shared module as a Git subtree. That will make future merging much easier.

Erick555 commented 1 year ago

No, the org.freedesktop.Platform.Compat.i386 extension is 32bit and contains all libraries available in 64bit runtime. This how 32bit apps like steam games or wine can run on flatpak.

gtk2 isn't in extension so you need to build it. Everything else you shouldn't.

konradmb commented 1 year ago

@Erick555

  1. Acrobat needs libgdk_pixbuf_xlib-2.0.so.0 and it isn't in the runtime:
    /app/extra/Adobe/Reader9/Reader/intellinux/bin/acroread: error while loading shared libraries: libgdk_pixbuf_xlib-2.0.so.0: cannot open shared object file: No such file or directory
$ flatpak run --command=bash com.adobe.Reader
$ find -L /lib -iname *pixbuf* 
/lib/i386-linux-gnu/libgdk_pixbuf-2.0.so.0
/lib/i386-linux-gnu/libgdk_pixbuf-2.0.so.0.4200.10
/lib/i386-linux-gnu/gdk-pixbuf-2.0
/lib/i386-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-ani.so
/lib/i386-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-bmp.so
/lib/i386-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-gif.so
/lib/i386-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-icns.so
/lib/i386-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-ico.so
/lib/i386-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-pnm.so
/lib/i386-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-qtif.so
/lib/i386-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-svg.so
/lib/i386-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-tga.so
/lib/i386-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-tiff.so
/lib/i386-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-xbm.so
/lib/i386-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-xpm.so
/lib/i386-linux-gnu/girepository-1.0/GdkPixbuf-2.0.typelib
/lib/i386-linux-gnu/gstreamer-1.0/libgstgdkpixbuf.so
/lib/x86_64-linux-gnu/libgdk_pixbuf-2.0.so.0
/lib/x86_64-linux-gnu/libgdk_pixbuf-2.0.so.0.4200.10
/lib/x86_64-linux-gnu/gdk-pixbuf-2.0
/lib/x86_64-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-ani.so
/lib/x86_64-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-bmp.so
/lib/x86_64-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-gif.so
/lib/x86_64-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-icns.so
/lib/x86_64-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-ico.so
/lib/x86_64-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-pnm.so
/lib/x86_64-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-qtif.so
/lib/x86_64-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-svg.so
/lib/x86_64-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-tga.so
/lib/x86_64-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-tiff.so
/lib/x86_64-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-xbm.so
/lib/x86_64-linux-gnu/gdk-pixbuf-2.0/2.10.0/loaders/libpixbufloader-xpm.so
/lib/x86_64-linux-gnu/girepository-1.0/GdkPixbuf-2.0.typelib
/lib/x86_64-linux-gnu/gstreamer-1.0/libgstgdkpixbuf.so
  1. CUPS from runtime gives SIGSEGV when printing. https://gitlab.com/konradmb/flatpak-for-adobe-reader/-/blob/master/build/com.adobe.Reader.yml#L45
konradmb commented 1 year ago

@hmenke Great idea, I'll update PR

flathubbot commented 1 year ago

Started test build 38632

flathubbot commented 1 year ago

Build 38632 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/21274/com.adobe.Reader.flatpakref
Erick555 commented 1 year ago

Acrobat needs libgdk_pixbuf_xlib-2.0.so.0 and it isn't in the runtime:

gdk-pixbuf-xlib is slightly different project than gdk-pixbuf. You should use this as source: https://download.gnome.org/sources/gdk-pixbuf-xlib/2.40/gdk-pixbuf-xlib-2.40.2.tar.xz

CUPS from runtime gives SIGSEGV when printing. https://gitlab.com/konradmb/flatpak-for-adobe-reader/-/blob/master/build/com.adobe.Reader.yml#L45

Interesting. Is it still true after 4 years? Also what is the reason if there are no changes in build options??

Erick555 commented 1 year ago

You have to remember about it every time in the future

As I said you can keep building all those helper modules with global -O0 and nobody could tell the difference. This isn't performance oriented stuff. Please don't over complicate this PR for no reason.

konradmb commented 1 year ago

@hmenke Decision what should I do is up to you

Erick555 commented 1 year ago

@konradmb Are you sure that its -O0 that specifically fixes the crash?

The CFLAGS runtime set contains: -O2 -g -pipe -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer

and you are replacing all of above with just -O0 which may fix it coincidentally.

konradmb commented 1 year ago

@Erick555 I'm pretty sure that when I've checked env, CFLAGS env var looked exactly the same as you say, but with -O0 appended at the end. Also I think there's additional flatpak manifest option like cflags-override which makes it behave like you describe, but it's set to false by default.

Erick555 commented 1 year ago

@konradmb well, please take a look at the build log from https://github.com/flathub/com.adobe.Reader/pull/8#issuecomment-1530383428

i686-unknown-linux-gnu-gcc -DHAVE_CONFIG_H -I. -I../.. -DG_LOG_DOMAIN=\"Gdk\" -DGDK_COMPILATION -I../.. -I../../gdk -I../../gdk -DG_DISABLE_CAST_CHECKS -I/usr/include/pango-1.0 -I/usr/include/gio-unix-2.0 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/cairo -I/usr/include/harfbuzz -I/usr/include/freetype2 -I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include -I/usr/include/blkid -I/usr/include/pixman-1 -I/usr/include/libmount -I/usr/include/fribidi -pthread -I/usr/include/libpng16 -DG_DISABLE_SINGLE_INCLUDES -DATK_DISABLE_SINGLE_INCLUDES -DGDK_PIXBUF_DISABLE_SINGLE_INCLUDES -DGTK_DISABLE_SINGLE_INCLUDES -O0 -Wall -c -o checksettings.o checksettings.c

Clearly all CFLAGS are gone except -O0. The thing is if you pass CFLAGS as argument in configure then the CFLAGS from env are being replaced by it. This wouldn't happen if you use cflags manifest option instead.

konradmb commented 1 year ago

@Erick555 I'll take a closer look at this, but I'm 100% sure the bug appeared again, when the only thing I've manipulated was s/-O0/-O1/ as is on configure flag

flathubbot commented 1 year ago

Started test build 38785

flathubbot commented 1 year ago

Build 38785 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/21427/com.adobe.Reader.flatpakref
flathubbot commented 1 year ago

Started test build 38789

flathubbot commented 1 year ago

Build 38789 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/21431/com.adobe.Reader.flatpakref
flathubbot commented 1 year ago

Started test build 39306

flathubbot commented 1 year ago

Build 39306 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/21950/com.adobe.Reader.flatpakref
flathubbot commented 1 year ago

Started test build 39316

konradmb commented 1 year ago

@hmenke Rebased @Erick555 Moved to cflags build-opt. The only difference between no-SIGSEGV and SIGSEGV is to change -O0 to -O1, respectively.

flathubbot commented 1 year ago

Build 39316 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/21960/com.adobe.Reader.flatpakref
Erick555 commented 1 year ago

Great, this way the rest of build flags (-g -pipe -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer) are kept.

I still think moving cflags to global section and use ordinary submodule is worth it as it avoids committing over 120 files to this repo. At least for transparency (it will be clear from where the module come from and they are untouched). -O0 for gdk-pixbuff-xlib isn't big deal.

konradmb commented 1 year ago

Ehhhh, if you want it so much... :smiley:

flathubbot commented 1 year ago

Started test build 39349

flathubbot commented 1 year ago

Build 39349 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/21992/com.adobe.Reader.flatpakref
flathubbot commented 1 year ago

Started test build 39358

flathubbot commented 1 year ago

Build 39358 successful To test this build, install it from the testing repository:

flatpak install --user https://dl.flathub.org/build-repo/22001/com.adobe.Reader.flatpakref
Erick555 commented 1 year ago

Slim & clean, thx.