Bug 247626 - net/py-wsdd: Update to 0.6.1
Summary: net/py-wsdd: Update to 0.6.1
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: freebsd-ports-bugs (Nobody)
URL: https://github.com/christgau/wsdd/rel...
Keywords: needs-patch
Depends on:
Blocks:
 
Reported: 2020-06-29 01:03 UTC by Jeremy Chadwick
Modified: 2020-07-07 15:52 UTC (History)
2 users (show)

See Also:
hiroo.ono+freebsd: maintainer-feedback+
fernape: merge-quarterly?


Attachments
Update patch to wsdd 0.6.1 (1.69 KB, patch)
2020-06-29 12:58 UTC, Hiroo Ono
hiroo.ono+freebsd: maintainer-approval+
Details | Diff
Update patch to wsdd 0.6.1 (3.42 KB, patch)
2020-06-30 09:09 UTC, Hiroo Ono
hiroo.ono+freebsd: maintainer-approval+
Details | Diff
proposed rc.d script. (1.53 KB, text/plain)
2020-06-30 09:19 UTC, Hiroo Ono
no flags Details
revised patch for wsdd 0.6.1. (3.61 KB, patch)
2020-07-07 15:52 UTC, Hiroo Ono
hiroo.ono+freebsd: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Chadwick 2020-06-29 01:03:11 UTC
Would like to a request an update of this port to version 0.6.1:

https://github.com/christgau/wsdd/releases/tag/v0.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

This bug was introduced in late August 2019 during a code revamp.  FreeNAS does not see this bug because they are using an older version of wsdd (prior to the bug being introduced).

Thank you.
Comment 1 Hiroo Ono 2020-06-29 12:58:59 UTC
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.
Comment 2 Jeremy Chadwick 2020-06-29 18:49:01 UTC
(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"
Comment 3 Hiroo Ono 2020-06-30 09:09:05 UTC
Created attachment 216064 [details]
Update patch to wsdd 0.6.1

patch update.
Comment 4 Hiroo Ono 2020-06-30 09:19:49 UTC
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.
Comment 5 Jeremy Chadwick 2020-06-30 10:44:34 UTC
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.
Comment 6 Hiroo Ono 2020-07-07 15:52:57 UTC
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.