Bug 247626

Created attachment 216035 [details]
Update patch to wsdd 0.6.1

Here is a patch to update wsdd to 0.6.1, with a little change to rc.d script to properly stop the daemon.

(In reply to Hiroo Ono from comment #1)
In general, this rc script provided by the vendor is pretty bad.  Your modification fixes a long-standing problem of the daemon not TRULY being stopped properly, which is great!

The vendor version has an unnecesary dependency on Samba (testparm) just for something like "set the workgroup name", and does not let you set your own flags in rc.conf for wsdd (often needed for things like "-i em0").  That's a feature I really, really needed.

What I use is below, named "wsdd_alternate" as to not conflict with the port/pkg one.  It allows me to do this in rc.conf:

wsdd_alternate_enable="yes"
wsdd_alternate_args="-w WORKGROUP -4 -i em0 -v -v -v"

Keep in mind I COULD NOT call the variable wsdd_alternate_flags because that ends up adding the flags to daemon(8) as well (bad).  I don't know how to override that behaviour without writing a bunch of wsdd_alternate_prestart() etc. nonsense.



#!/bin/sh

# PROVIDE: wsdd_alternate
# REQUIRE: DAEMON NETWORKING SERVERS
# BEFORE: LOGIN
# KEYWORD: shutdown

. /etc/rc.subr

name=wsdd_alternate
rcvar=wsdd_alternate_enable

load_rc_config $name

: ${wsdd_alternate_enable:="NO"}
: ${wsdd_alternate_args:=""}

pidfile="/var/run/${name}.pid"
procname="python3"
command="/usr/sbin/daemon"
command_args="-f -S -p ${pidfile} -u daemon -T wsdd /conf/ME/scripts/wsdd.py ${wsdd_alternate_args}"

run_rc_command "$1"

Created attachment 216064 [details]
Update patch to wsdd 0.6.1

patch update.

Created attachment 216065 [details]
proposed rc.d script.

> The vendor version has an unnecesary dependency on Samba (testparm) just for something like "set the workgroup name", and does not let you set your own flags in rc.conf for wsdd (often needed for things like "-i em0").  That's a feature I really, really needed.

How about attached rc.d script. With it, you can just add
    wsdd_suppflags="-i em0"
in rc.conf and the rc.d script detect the workgroup automatically.
There are alternative choices to write workgroup in rc.conf like
    wsdd_group="WORKGROUP"
to overwrite automatically detected one, or
    wsdd_domain="EXAMPLE.COM"
to replace "-w WORKGROUP" with "-d EXAMPLE.COM".

wsdd_flags=... does not work, because it will be automatically added to the arguments of $command, but if there is a better word than 'suppflags', I will change the variable name.

If this is OK, I think I need to check at least NetBSD and OpenBSD's daemon command and rc script if the above rc.d script might work before reporting it to upstream.

Three things:

1. Just some feedback: regarding the variable name (wsdd_suppflags): I had the same conundrum. :)  I'm not sure what to call it either.  I might suggest looking through some other ports to see, although it might be difficult because there's only a limited number which use daemon(8).  So, I think wsdd_suppflags is fine.

The other variables look very reasonable -- zero complaints!

2. I would suggest adding NETWORKING and SERVERS to the REQUIRE line.  This can  affect rcorder(8).  I know only DAEMON works as-is, but there is no guarantee of that long-term depending on if there are revamps in the future.  We also must consider possibility of jail usage, where IIRC behaviours are different.  I suggest looking at /etc/rc.d/* for examples.  I just know that the 3 combined will always do the right thing.

3. Someone (I can do this if it makes your life easier) should submit the wsdd rc script modifications, when finished, back to the author in GitHub, so that there's at least some uniformity.  FreeNAS may eventually pick that up too (their wsdd version right now is old and using an equally old service model, based entirely on what's in GitHub I think and not what's in ports).

Thank you for your attentivity!  It's greatly appreciated.

Created attachment 216297 [details]
revised patch for wsdd 0.6.1.

Updated the patch.

1. I borrowed the _flags variable usage from cups_browsed's rc.d script.
  Thus, the variable wsdd_flags works.

2. As the DAEMON requires NETWORKING and SERVERS, I think it is not needed to add NETWORKING and SERVERS in addition to DAEMON.

Looks good! Can we push this out?

Comment 8 commit-hook 2020-07-26 14:34:20 UTC

A commit references this bug:

Author: tcberner
Date: Sun Jul 26 14:34:05 UTC 2020
New revision: 543482
URL: https://svnweb.freebsd.org/changeset/ports/543482

Log:
  net/py-wsdd: Update to 0.6.1

  0.6.1 fixes a critical bug pertaining to BSD, where the HTTP server in wsdd
  would basically "do nothing" due to listen(2) being called _after_ kevent(2)
  (and pending network socket I/O was therefore never processed).  I was the
  reporter and analyst of this bug: https://github.com/christgau/wsdd/issues/49

  - Additionally: little change to rc.d script to properly stop the daemon.

  PR:		247626
  Submitted by:	Hiroo Ono <hiroo.ono+freebsd@gmail.com> (maintainer)
  Reported by:	Jeremy Chadwick <jdc@koitsu.org>

Changes:
  head/net/py-wsdd/Makefile
  head/net/py-wsdd/distinfo
  head/net/py-wsdd/files/
  head/net/py-wsdd/files/patch-etc_rc.d_wsdd

Comment 9 Tobias C. Berner 2020-07-26 14:36:14 UTC

Committed, and MFH-ed.

mfg Tobias

Comment 10 commit-hook 2020-07-26 14:36:21 UTC

A commit references this bug:

Author: tcberner
Date: Sun Jul 26 14:35:30 UTC 2020
New revision: 543483
URL: https://svnweb.freebsd.org/changeset/ports/543483

Log:
  MFH: r543482

  net/py-wsdd: Update to 0.6.1

  0.6.1 fixes a critical bug pertaining to BSD, where the HTTP server in wsdd
  would basically "do nothing" due to listen(2) being called _after_ kevent(2)
  (and pending network socket I/O was therefore never processed).  I was the
  reporter and analyst of this bug: https://github.com/christgau/wsdd/issues/49

  - Additionally: little change to rc.d script to properly stop the daemon.

  PR:		247626
  Submitted by:	Hiroo Ono <hiroo.ono+freebsd@gmail.com> (maintainer)
  Reported by:	Jeremy Chadwick <jdc@koitsu.org>

  Approved by:	ports-secteam (blanket)

Changes:
_U  branches/2020Q3/
  branches/2020Q3/net/py-wsdd/Makefile
  branches/2020Q3/net/py-wsdd/distinfo
  branches/2020Q3/net/py-wsdd/files/