Open MonkeysAreEvil opened 4 years ago
Hi! Thanks for the work you've done!
Your PR is breaking the installation: https://travis-ci.org/hakavlad/nohang/builds/632553221
Could you fix it, please?
Make PR in dev branch, please.
No worries. I have the issue fixed in my fork. KAMiKAZOW has suggested that the copyright notice be dropped, and they also note that the OpenRC addition and envsubst changes are actually mixed in.
What I can do is cancel the existing 2 pull requests and replace them with a single one with the fixed systemctl/openrc check + drop licence header, and envsubst change. How does this sound?
I would like to get 2 pull requests to test them separately.
make install-openrc
.Could you do the first pull request in dev
branch, please?
Sure, no worries. I will update the pull requests shortly.
Awesome. So, regarding OpenRC we could 1) check for systemctl/openrc within the install
recipe, and branch appropriately; 2) write a config script to generate an appropriate Makefile
; or 3) have a separate install-openrc
recipe.
(1) is potentially fragile, but should be ok. (2) is a bit of work, but only slightly increases the complexity of installing. (3) is really easy to implement. It does make the ebuild slightly more complicated, and means the user has to know which recipe to use.
What are your thoughts?
At least we already have some precedent:
Start earlyoom automatically by registering it as a service:
sudo make install # systemd
sudo make install-initscript # non-systemd
-- https://github.com/rfjakob/earlyoom#download-and-compile
In fact, I do not know which option is better and have no experience with non-systemd. But it seems that the way used in earlyoom simply works. Could you check how earlyoom can be installed in Gentoo, and does it work well with earlyoom? Maybe we can do it this way if it works well.
Yeah it's not obvious to me which approach is best either.
Anyway, so earlyoom just works, but they have a little extra complexity. Their install
recipe calls install: earlyoom.service install-bin install-default install-man
i.e. install is split into logical units. This means on the Gentoo side we can conditionally call each recipe separately i.e. for systemd vs OpenRC installs, and whether or not to install docs. So it comes out quite nicely. This is possible here.
I think the simplest possible change, with minimal duplication is best:
1) Separate out each paragraph in the install recipe into something like:
install: install-bin install-conf install-logs install-doc install-systemd-service
.
2) Add a install-openrc-service
recipe.
3) Add a install-openrc: install-bin ... install-openrc-service
recipe.
So from the user perspective installation is identical, with the addition of and OpenRC recipe. If someone wants to add runit/sysv or whatever, it's just another recipe. This is then less fragile and less branching than checking for openrc
or systemctl
bins.
From a packaging perspective, systemd distros don't change it's still make install. From my perspective, I can check if systemd or not, and just call the correct recipe.
I've updated my dev branch and overlay to reflect these changes. Do you have any comments or suggestions?
@MonkeysAreEvil Seems like Makefile is ready to update.
Could you add openrc
and install-openrc
targets, please, and openrc-specific files?
This may be like follow:
install openrc:
install -d $(DESTDIR)$(SYSCONFDIR)/init.d
-sed "s|:TARGET_SBIN:|$(SBINDIR)|g;s|:TARGET_SYSCONFDIR:|$(SYSCONFDIR)|g" nohang/openrc/nohang.in > nohang/openrc/nohang
-sed "s|:TARGET_SBIN:|$(SBINDIR)|g;s|:TARGET_SYSCONFDIR:|$(SYSCONFDIR)|g" nohang/openrc/nohang-desktop.in > nohang/openrc/nohang-desktop
install -m0775 nohang/openrc/nohang $(DESTDIR)$(SYSCONFDIR)/init.d/nohang
install -m0775 nohang/openrc/nohang-desktop $(DESTDIR)$(SYSCONFDIR)/init.d/nohang-desktop
install-openrc: base openrc
Slightly off topic, but ebuild fails with this:
>>> Install sys-process/nohang-9999 into /var/tmp/portage/sys-process/nohang-9999/image
* ERROR: sys-process/nohang-9999::eph_kit failed (install phase):
* 'einstall' has been banned for EAPI '7'
*
* Call stack:
* ebuild.sh, line 125: Called src_install
* environment, line 1746: Called einstall
* phase-helpers.sh, line 693: Called die
* The specific snippet of code:
* die "'${FUNCNAME}' has been banned for EAPI '$EAPI'"
*
Cool cool. I have update my fork and overlay and everything appears to be working. I'll test it a bit more later and update the pull request if it's all ok.
Let me know if there are any questions/comments.
cheers
@MonkeysAreEvil Makefile updated, please test again. If everything appears to be working I'll waiting for a PR that fixes readme: how to install nohang on Gentoo.
Thanks! Actually, there is a small bug, which I have just fixed, with installing the init files.
@MonkeysAreEvil how to enable and start the service after installing?
# rc-service nohang status
* rc-service: service `nohang' does not exist
I believe that issue is fixed in my fork. If we can confirm that is correct, I can do a new pull request.
Ok, please make a PR.
And please update readme: how to install in Gentoo.
Fix Makefile to install on openrc-based systems https://github.com/hakavlad/nohang/commit/ec9d84806db208f3aa6de2ffe8f6e0e6693895e5
Ah, excellent. Sorry, life is very busy at the moment. Everything appears to be working for me.
I will open a PR for the readme shortly.
To complement my pull requests, I have updated my overlay to include sys-process/nohang. If you're happy with all this, can we get the README updated to note this?
In principle I can support both dev and stable versions, which could be preferable.