mate-desktop / mate-terminal

The MATE Terminal Emulator
http://www.mate-desktop.org
GNU General Public License v3.0
133 stars 73 forks source link

build: Isolate smclient #344

Closed rbuj closed 4 years ago

rbuj commented 4 years ago

test: disable smclient

$ ./autogen.sh --prefix=/usr --enable-compile-warnings=maximum --enable-debug --disable-smclient
$ make &> make.log && sudo make install
raveit65 commented 4 years ago

Good question, it doesn't crash here with f30 or f32.

rbuj commented 4 years ago

I fixed the 2 cashes:

  1. gdk should be initialized before to call gdk_display_get_name otherwise display_name is NULL.
  2. gtk should be initialized before to call terminal_app_handle_options.

terminal.c:

    gdk_init (&argc, &argv);
    const char *display_name = gdk_display_get_name (gdk_display_get_default ());
    options->display_name = g_strdup (display_name);

terminal.c

        gtk_init(&argc, &argv);
        terminal_app_handle_options (terminal_app_get (), options, TRUE /* allow resume */, &error);
raveit65 commented 4 years ago

You mean that it crashes when using the new compiler option --disable-smclient without the other changes of this PR?

raveit65 commented 4 years ago

Can we have a check for libICE in configure please? Compiling failed when libICE-devel isn't installed. Normally, libSM-devel pulls in libICE-devel into build environment. But when using --disable-smclient build requires libSM-devel isn't needed.

rbuj commented 4 years ago

Can we have a check for libICE in configure please? Compiling failed when libICE-devel isn't installed. Normally, libSM-devel pulls in libICE-devel into build environment. But when using --disable-smclient build requires libSM-devel isn't needed.

Yes, eggsmclient-xsmp.c requires libICE, WIP.

$ git grep Ice -- ':*.c'
src/eggsmclient-xsmp.c:static gboolean process_ice_messages (IceConn       ice_conn);
src/eggsmclient-xsmp.c:                 process_ice_messages (SmcGetIceConnection (xsmp->connection));
src/eggsmclient-xsmp.c:static void        ice_error_handler    (IceConn        ice_conn,
src/eggsmclient-xsmp.c:        IcePointer     values);
src/eggsmclient-xsmp.c:static void        ice_io_error_handler (IceConn        ice_conn);
src/eggsmclient-xsmp.c:static void        ice_connection_watch (IceConn        ice_conn,
src/eggsmclient-xsmp.c:        IcePointer     client_data,
src/eggsmclient-xsmp.c:        IcePointer    *watch_data);
src/eggsmclient-xsmp.c: IceSetIOErrorHandler (ice_io_error_handler);
src/eggsmclient-xsmp.c: IceSetErrorHandler (ice_error_handler);
src/eggsmclient-xsmp.c: IceAddConnectionWatch (ice_connection_watch, NULL);
src/eggsmclient-xsmp.c:process_ice_messages (IceConn ice_conn)
src/eggsmclient-xsmp.c: IceProcessMessagesStatus status;
src/eggsmclient-xsmp.c: status = IceProcessMessages (ice_conn, NULL, NULL);
src/eggsmclient-xsmp.c: case IceProcessMessagesSuccess:
src/eggsmclient-xsmp.c: case IceProcessMessagesIOError:
src/eggsmclient-xsmp.c:         sm_client_xsmp_disconnect (IceGetConnectionContext (ice_conn));
src/eggsmclient-xsmp.c: case IceProcessMessagesConnectionClosed:
src/eggsmclient-xsmp.c:ice_connection_watch (IceConn     ice_conn,
src/eggsmclient-xsmp.c:                      IcePointer  client_data,
src/eggsmclient-xsmp.c:                      IcePointer *watch_data)
src/eggsmclient-xsmp.c:         int fd = IceConnectionNumber (ice_conn);
src/eggsmclient-xsmp.c:ice_error_handler (IceConn       ice_conn,
src/eggsmclient-xsmp.c:                   IcePointer    values)
src/eggsmclient-xsmp.c:ice_io_error_handler (IceConn ice_conn)
rbuj commented 4 years ago

@raveit65 done.

https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Desktop-generic/LSB-Desktop-generic/libice-ddefs.html

rbuj commented 4 years ago

Because SMlib is dependent on ICE, applications should link against SMlib and ICElib by using -lSM -lICE.

Ref: 4. Header Files and Library Name, Page 2 http://refspecs.linux-foundation.org/X11/SMlib.pdf

raveit65 commented 4 years ago

On a system without libSM-devel and libICE-devel installed:

[rave@mother mate-terminal-dist]$ ./autogen.sh  --enable-compile-warnings=maximum --enable-debug --disable-smclient

<cut>

    mate-terminal-1.24.0:

    prefix:                 /usr/local
    source code location:   .
    compiler:               gcc
    cflags:                  -g -O0
    warning flags:          -Wall -Wmissing-prototypes -Wbad-function-cast -Wcast-align -Wextra -Wno-unused-parameter -Wformat-nonliteral -Wmissing-declarations -Wmissing-field-initializers -Wnested-externs -Wpointer-arith -Wredundant-decls -Wshadow -Wstrict-prototypes -Wno-sign-compare
    linker flags:           

    s/key support:          yes

[rave@mother mate-terminal-dist]$ make
make  all-recursive

<cut>

  CC       mate_terminal-terminal-marshal.o
  CC       mate_terminal-terminal-resources.o
  CC       mate_terminal-terminal-type-builtins.o
  CCLD     mate-terminal
/usr/bin/ld: cannot find -lICE
collect2: error: ld returned 1 exit status
make[4]: *** [Makefile:651: mate-terminal] Error 1
make[4]: Leaving directory '/media/Data/Programme/Linux/Mate-Desktop/git-version/github-matedesktop/mate-terminal-dist/src'
make[3]: *** [Makefile:1075: all-recursive] Error 1

Configure should stop if libICE isn't installed because compiling gives an error, or not?

rbuj commented 4 years ago

@raveit65 I think it's fixed now.

raveit65 commented 4 years ago

Thanks, compiling without libICE is working now. When compiling without smclient saving open terminal windows doesn't work any more, So this works as expected. Now testing compiling with smclient enable.

raveit65 commented 4 years ago

Hmm, did you you test saving session-state with this PR?

raveit65 commented 4 years ago

Without PR configure gives me a hint about using SMClient.

checking for SMCLIENT... yes

This doesn't work with PR. It seems that SMClient support is broken now.

rbuj commented 4 years ago

@raveit65 I updated configure summary. (smclient support: yes/no) test:

$ ./autogen.sh --prefix=/usr
<cut>

    mate-terminal-1.24.0:

    prefix:                 /usr
    source code location:   .
    compiler:               gcc
    cflags:                 
    warning flags:          -Wall -Wmissing-prototypes
    linker flags:           

    smclient support:       no
    s/key support:          yes

Now type `make' to compile mate-terminal

test:

$ ./autogen.sh --prefix=/usr --enable-smclient
<cut>
    mate-terminal-1.24.0:

    prefix:                 /usr
    source code location:   .
    compiler:               gcc
    cflags:                 
    warning flags:          -Wall -Wmissing-prototypes
    linker flags:           

    smclient support:       yes
    s/key support:          yes

Now type `make' to compile mate-terminal
raveit65 commented 4 years ago

@rbuj Before i restart my session again, does saving session-state work now if compiled with SMClient? And terminal windows will be restored?

[rave@mother ~]$ gsettings get org.mate.session auto-save-session
true
raveit65 commented 4 years ago

Ok, i found the problem. Now, it is needed to enable SMClient support explicitly with --enable-smclient But this is a new behaviour. Can we set the default to --enable-smclient, please?

rbuj commented 4 years ago

@raveit65 done

raveit65 commented 4 years ago

Thanks, now it works like it should and mate-session-manager can save terminal windows again.

yetist commented 4 years ago

There is no problem with this PR. I agree to merge this PR first.

As a final solution, we need to use GDesktopAppinfo instead of eggdesktopfile; and to use smclient's DBUS implementation to replace the XSMP implementation.

Because many components are involved, I recommend using git submodules as a transition until the goal is achieved. Here is a example of git submodule: https://github.com/mate-desktop/mate-terminal/pull/342#issuecomment-619313966

raveit65 commented 4 years ago

@yetist Good, should i fork your git modul https://github.com/yetist/libegg to our repos?

yetist commented 4 years ago

@yetist Good, should i fork your git modul https://github.com/yetist/libegg to our repos?

yes, please do that.

raveit65 commented 4 years ago

@yetist https://github.com/mate-desktop/libegg Done, you're are admin to manage all needed settings ;)