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

Add support for elogind #253

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

oz123
Copy link

@oz123 oz123 commented Aug 8, 2023

Uses similar patch to the one already included in mate-screensaver.

Uses similar patch to the one already included in mate-screensaver.
Copy link
Member

@lukefromdc lukefromdc left a comment

Choose a reason for hiding this comment

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

I have systemd not elogind on Debian Unstable (Sid) but this configured fine and built fine. Config messages worked, showing

Configure summary:

        mate-system-monitor 1.27.0
        ==========================

        Source code location:   .
        C Compiler:             gcc
        CFLAGS:                 
        WARN_CFLAGS:            -Wall -Wmissing-prototypes
        C++ Compiler:           g++
        CXXFLAGS:               
        Elogind support:        no
        WARN_CXXFLAGS:          -Wall -Woverloaded-virtual
        Maintainer mode:        no
        Systemd support:        yes
        wnck support:           yes

Note that we also support building on meson and this only changes the options in autotools.

@lukefromdc lukefromdc requested a review from a team August 8, 2023 18:35
@oz123
Copy link
Author

oz123 commented Aug 9, 2023

@lukefromdc I am not so familiar with meson. Happy to get some guidance here. I myself don't know enough about it to teach it either

Copy link
Member

@cwendling cwendling left a comment

Choose a reason for hiding this comment

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

I cannot test this because I'm not gonna install libelogind which has serious consequences on other packages here and I don't wanna deal with it. I could try in a VM, but call me lazy for now.

Anyway, I don't think this PR works as-is, even with libelogind installed I doubt the linking phase would work, as the library wouldn't be used on the link command. If it does work, please explain how comes.

configure.ac Outdated Show resolved Hide resolved
configure.ac Outdated Show resolved Hide resolved
configure.ac Outdated Show resolved Hide resolved
configure.ac Show resolved Hide resolved
configure.ac Show resolved Hide resolved
configure.ac Outdated Show resolved Hide resolved
@lukefromdc
Copy link
Member

I myself don't know enough about meson to help with that, I can build with it and test packages but have not yet written or substantially modified meson configuration files.

@oz123
Copy link
Author

oz123 commented Aug 10, 2023

@cwendling I refactored my patch according to your suggestions.
I also tested it on ubuntu vagrant machine which has no libelogind and it seems to behave as desired:

vagrant@m1:~/mate-system-monitor$ ./configure --enable-elogind 
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
....
checking how to hardcode library paths into programs... immediate
checking for
    glib-2.0 >= 2.56.0
    libgtop-2.0 >= 2.37.2
    gtk+-3.0 >= 3.22.0
    gtkmm-3.0 >= 3.8.1
    libxml-2.0 >= 2.0
    librsvg-2.0 >= 2.35
    glibmm-2.4 >= 2.22
    giomm-2.4 >= 2.26.0
    gmodule-2.0
... yes
checking for glib-2.0 >= 2.56.0... yes
checking for libelogind... no
configure: error: *** elogind support requested but libraries not found
vagrant@m1:~/mate-system-monitor$ 

@lukefromdc
Copy link
Member

With no landline I cannot download distro installers to test. If I did they'd go on extra drives as I also have no VM experience. Thus I have no way to test any of the non-systemd stuff

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.

3 participants