Open plorenzo19 opened 2 weeks ago
thanks a lot. I'm trying to reorganize the patches to match with debian's as closely as possible, base on following order:
I'm using both clang & gcc with CFLAGS (-Wall -Wextra) for this purpose. I'm focusing on 1-4 until the patches are merged into Gentoo's official portage.
Actually I recommend to merge just 1-3 (or even 2-3) first, and then prepare second PR with 4. The reasons are:
I've force pushed the ftbfs branch, please test & review the patch before "fix compilation warning". Once it's ok, I'll prepare patch for Gentoo base on these changes. Let's leave the remaining warning fix etc. to later, Mos likely I'll create a new branch.
I've added debian's patch 0002 (which is applied by author) add the documentation update patch after the fix of compilation errors.
Not all warnings may worth to be fixed.
For C, it's best to fix some warnings. Such as: 0015-fix-34-cases-of-Wmisleading-indentation-warnings.patch, which must be handled carefully when change or refactor code. 0016-fix-warning-for-undefined-operation-on-sequence-poin.patch, the behaviour is undefined, i.e., it may depend on compiler's implementation or future change. -Wimplicit-fallthrough in my patch, the order of the case in switch is important because no break for every case.
The reason might be C is an old (50+years) language with many history burden while Go is rather new, :-)
compilation errors and most of the warnings (reported by -Wall -Wextra) had been fixed by using clang 18 and gcc 15, patches had been applied by gentoo.
remaing patch to be considered:
there are some warnings haven't been fixed in debian, such as -Wunterminated-string-initialization, -Wdeprecated-non-prototype, I suggest at least fix warnings reported by using -Wall
remaing patch to be considered:
I don't like the idea to "consider a patch". Let's consider issues solved by these patches, is they actual, is there are any demand from Gentoo users on fixing them. At a glance, to me it looks this way:
communication files
I'm not sure is Gentoo supports read-only /etc
at all. If yes, then it makes sense to continue this discussion, if not - there is no point in spending more time on this.
SIGPWD
To me it feels normal to setup docker containers taking in account running application's behaviour. All applications with unusual shutdown requirements needs this, docker provides a way to setup this, so why do we need to improve something what already works?
SIGALARM (force rescan) handle
If I understood correctly it's related to starting single runit service faster using some other service manager (e.g. systemd or openrc script). I doubt any Gentoo & runit users works this way and unlikely anyone will until Gentoo itself will start using runit for some openrc services etc. which is unlikely to happen.
possible resource leak (open file unclosed)
This one should be investigated in depth. If runit leaks a couple of file descriptors for it's whole lifetime then it may be easier to just ignore it, because it won't create any real issues. If it leak, say, 1 fd every hour - it probably worth to be fixed.
Hi, I'm maintaining runit on Debian and I replied on the supervision mailing list (FTBFS with gcc-14); I see that there is some discussion about the Debian patchset, so maybe it can save you some time if I provide more info on the patchset (deciding which patch to include or not is of course a matter for Gentoo devs/users)
The patchset is at https://salsa.debian.org/debian/runit/-/tree/next/debian/patches?ref_type=heads and is maintained on Debian (Ubuntu fetches the package from Debian) many patches were added by previous maintainers, but I know the rationale
patch with new features are:
0002 (runit.nosync for vserver), merged in Debian by Gerrit Pape, for the rationale see https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=695281
0007 move comunication files: I'm not sure this is actually needed, maybe symlinks from /etc/runit/runit.stopit --> /run/ will do without the need for this patch. I still have to test it, if it works with symlinks I'm likely to drop it.
0013 shutdown on SIGPWR: it makes easy to shutdown runit in container: otherwise you need to tell the container that it has to use a custom signal for shutdown with command line flag. I don't like this patch, it completely ignore the signal mode for runit, which is (signal + flag-file-mode); I'm unlikely to drop it but if I find extra time I may change it to something similar to the ctrlaltdel file mechanism
from 0019 to 0024 + 0026: rescan on SIGALRM runsvdir scan its directory every 5+1 seconds; this is not an issue until you want enable a service and signal it immediately. In Debian service packages usually enable and start/restart the service immediately; without this patch we had to poll for up to 5+1 sec before signal the service, so the patch was added. SIGALRM to pid1 and the main runsvdir does an out of schedule rescan; a file is created inside the runsvdir directory and can be used as a proxy for readiness. Not sure if Gentoo has this need.
Also
Remaining patches are not Debian specific and actually fix some bug (it may be in the code or in manpages).
Hope this helps, Lorenzo