Marcus Glocker <[email protected]> writes:

> On Fri, Mar 31, 2017 at 12:33:03AM +0200, Jeremie Courreges-Anglas wrote:
>
>> Marcus Glocker <[email protected]> writes:
>> 
>> > On Thu, Mar 30, 2017 at 09:45:34PM +0200, Jeremie Courreges-Anglas wrote:
>> >
>> >> Marcus Glocker <[email protected]> writes:
>> >> 
>> >> > KiwiIRC is a web-based IRC client built using Node.js.  It runs as
>> >> > standalone which means it doesn't require an underlying HTTP server.
>> >> >
>> >> > Comments, OKs?
>> >> 
>> >> You can't use npm install like this, the build process should not fetch
>> >> from the network.
>> >
>> > Hmm, ok.  Any good idea how to solve this?  Unfortunately it's required
>> > to fetch some modules so you can do the build afterwards.
>> 
>> No precise idea, I don't know mich about node/npm.
>> 
>> Creating ports for all deps seems to be what fedora does:
>> 
>>   https://fedoraproject.org/wiki/Packaging:Node.js
>> 
>> An alternative would be to make npm use a local cache populated with
>> distfiles explicitely registered in Makefile/distinfo.  But I don't know
>> whether this would actually work.
>
> Hmm, ok, I will check the URL and have a think.
>
>> [...]
>> 
>> >> -$(command -v nodejs || command -v node) 
>> >> $basedir/server/helpers/launcher.js "$@"
>> >> +$(command -v nodejs || command -v node) 
>> >> $basedir/server/helpers/launcher.js "$@" -p /var/log/kiwiirc/kiwiirc.pid
>> >> 
>> >> /var/log seems like a weird choice for a pid file, you can install
>> >> /var/run/kiwiirc in the rc script.  Is this pid file actually needed?
>> >
>> > No, the pid file doesn't seem to be required.  There seems no easy
>> > way to disable it.  Therefore I just send it to /dev/null instead.
>> 
>> Ah, I see.  Doesn't this result in (failed) attempts to unlink
>> /dev/null?  Maybe it would be cleaner to bite the bullet and install
>> /var/run/kiwiirc.  The README still lists the pidfile btw.
>
> I didn't noticed an error during stop/restart but I agree it's cleaner
> to just create the pid file in /var/run/kiwiirc.

You can't use the PLIST to create the pid file directory, /var/run is
pruned at boot time.  Let's do it in rc_pre().  I'm not completely happy
with the "automatic" handling of pid_dir, it is trying to be a bit too
smart...  On the other hand, if someone wants to run multiple instances,
better find a way to specify alternate pid files.

> README fixed, also for
> the installation path location.

I trimmed down the README, IMO no need to describe where the pid file is
stored, no need to say "please".

> So there is the node/npm thing left now I guess ...

Good luck with that one. :)

Please find an updated tarball below, with the following changes:
- no need to patch to pass the path to the pidfile, we can use
  daemon_flags.  We can also use daemon_flags for the path to the config
  file (no need for a symlink any more).
- support rc_reload(), kiwiirc uses SIGUSR1 instead of SIGHUP
- drop DISTNAME/WRKSRC, the defaults are fine.  I kept "kiwiirc" for
  PKGNAME, but using "KiwiIRC" would be reasonable too IMHO.
  No need for MASTER_SITES either, the GH_* variables are enough.
- no need to rename the config file, no need for a symlink
- merge do-install and post-install, and reorder: first the main copy
  step, then tweaks for the manpage and config file.  We don't modify
  WRKSRC any more.
- tweak DESCR: less verbose mention of the standalone property, speak
  more about features

Attachment: kiwiirc.tgz
Description: Binary data

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to