lxqt / lxqt-session

The LXQt session manager
https://lxqt.github.io
GNU Lesser General Public License v2.1
57 stars 52 forks source link

Add NetBSD support #403

Closed dhgutteridge closed 2 years ago

dhgutteridge commented 2 years ago

Originally, my goal was to get this to build on NetBSD at all, since it doesn't as-is. Near the end of the file, you universally invoke kill(), which POSIX mandates is in signal.h, but don't include that header unconditionally. Beyond what I've done here for NetBSD, that's a separate issue.

NetBSD doesn't have an equivalent of the "reaper" facility you attach at the beginning, so that's a no-op, but, since you continue anyway if you don't get it on Linux or FreeBSD, I figured we might as well add the balance for NetBSD. (It takes a bit more to get a process list on NetBSD than the others.)

I've tried to match your project's coding style, hopefully I didn't miss anything. When compiling, I do get the following warning, but the extra parentheses aren't your general style (going by the FreeBSD block).

/home/disciple/pkgsrc/lxqt-1.0.0-netbsd/lxqt-session/work/lxqt-session-1.0.0/lxqt-session/src/procreaper.cpp: In member function 'void ProcReaper::stop(const std::set<long int>&)':
/home/disciple/pkgsrc/lxqt-1.0.0-netbsd/lxqt-session/work/lxqt-session-1.0.0/lxqt-session/src/procreaper.cpp:143:12: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
     if (kd = kvm_openfiles(NULL, NULL, NULL, KVM_NO_FILES, buf))

Dave (a.k.a. gutteridge@NetBSD.org)

palinek commented 2 years ago

Near the end of the file, you universally invoke kill(), which POSIX mandates is in signal.h, but don't include that header unconditionally.

Right. Please, add a fixing commit here.

I do get the following warning, but the extra parentheses aren't your general style (going by the FreeBSD block).

Please, add the parentheses to silence the warning.

NetBSD doesn't have an equivalent of the "reaper" facility

Then this feature (stopping children on end) is nearly pointless on NetBSD as most of lxqt components are spawning the children processes by XdgDesktopFileData::startApplicationDetached which is driven by QTXDG_START_DETACH_TRULY env. variable -> default when not defined is 1/true. So lxqt-session won't have any children processes. But as written above you can faciliate the parent->child process hierarchy by setting the use the QTXDG_START_DETACH_TRULY=0 in e.g. startlxqt. In this case each app/process will try to gracefully stop its children. Note, that this behavior is sub-optimal (as your apps can vanish if you stop lxqt-panel, lxqt-runner etc. before end of the session) and was replaced by the 'lxqt-session as reaper' logic.

dhgutteridge commented 2 years ago

Near the end of the file, you universally invoke kill(), which POSIX mandates is in signal.h, but don't include that header unconditionally.

Right. Please, add a fixing commit here.

I've included signal.h universally. Depending on the OS, additional macros may need to be defined to expose kill(), but I'm not going to apply those in this change set. I think that should at least fix the build on OpenBSD as-is as well, but I'm not certain, I don't use or support it. (The downstream packaging project I'm involved with is the primary packaging source for more than one OS, including some Solaris derivates, but right now Qt 5 itself doesn't seem to build for them, so I can't really test fixes here for it.)

I do get the following warning, but the extra parentheses aren't your general style (going by the FreeBSD block).

Please, add the parentheses to silence the warning.

I've added parentheses around that expression, and I also changed NULL to nullptr, as I see I missed that aspect of your coding style before.

NetBSD doesn't have an equivalent of the "reaper" facility

Then this feature (stopping children on end) is nearly pointless on NetBSD as most of lxqt components are spawning the children processes by XdgDesktopFileData::startApplicationDetached which is driven by QTXDG_START_DETACH_TRULY env. variable -> default when not defined is 1/true. So lxqt-session won't have any children processes. But as written above you can faciliate the parent->child process hierarchy by setting the use the QTXDG_START_DETACH_TRULY=0 in e.g. startlxqt. In this case each app/process will try to gracefully stop its children. Note, that this behavior is sub-optimal (as your apps can vanish if you stop lxqt-panel, lxqt-runner etc. before end of the session) and was replaced by the 'lxqt-session as reaper' logic.

Agreed. Without parent PID reassignment of some kind, the only PID normally parented to lxqt-session I see in my testing is the WM (in my case, xfwm4). I'd wondered if it would have some use either for a scenario I haven't thought of with how lxqt-session would spawn some related components, or possibly just as a reference for how get a process list on NetBSD should it be needed for something else.

dhgutteridge commented 2 years ago

As far as building goes, I integrated this patch downstream as part of our update to lxqt-session 1.0.0. An example resultant successful build (against NetBSD's development branch, as part of CI activity) is lxqt-session-1.0.0.tgz

By the way, I should add, this was the only OS-specific compiling issue we found when integrating LXQt 1.0.0, so it was a pretty smooth experience packaging. LXQt has been well received by some of our NetBSD users, thanks!

palinek commented 2 years ago

Any objection for merging this?

dhgutteridge commented 2 years ago

Just checking in on this. :)

tsujan commented 2 years ago

The patch was approved and there was no objection to its merge. Merging...