Hi Arther, Thanks for taking the time to review the patch. :)
The motive for Upstart scripts instead of a init.d script is simple: with the init.d script, I couldn't find a simple way to guarantee nslcd started _after_ a network connection was available. At the time I logged the feature request (I haven't tested this recently--I probably should), nslcd would simply terminate if no network connections were available, so there was a race condition between nslcd and the networking configuration scripts. The Upstart script is triggered by an event that's emitted when a network interface is fully configured (net-device- up), so that race condition is avoided. I removed the init.d script for a couple reasons. First, running `service nslcd restart` was preferring the init.d script to the Upstart script if the init.d script was present, leading to unexpected behavior. Second, it seemed best to have a single method for starting the daemon. Multiple, independent methods seemed like a good way to confuse users. When I built a test package with debuild in Ubuntu, wrappers for the Upstart scripts were placed in /etc/init.d/. I'd assume the same is true for Debian packages, but I haven't tested it. Point taken about the status 1 exit when sasl_mech or krb5_ccname isn't available--those two checks should definitely exit with status 0. I setup logging to the /tmp directory because AFAIK there's no good way to leverage existing log facilities with Upstart, and I understand that /tmp is usually available earlier in the boot process than /var. The log files contain diagnostic information about the startup process only--no secrets are shared. So, it seemed safe to have a log file, even though it wouldn't necessarily survive a reboot. Can you point out the duplicate checks? There's a lot of annoying redundancy in the variable initialization because Upstart's env directive doesn't support expansion (see https://bugs.launchpad.net/upstart/+bug/328366), but I tried to eliminate duplicate checks wherever possible. The one duplicate check I see is the initialization of the state directory, which happens in both nslcd and nslcd-kerberos scripts. That's there because we require a state directory for both daemons, but we can't assume one has run before the other. Resolving the race condition with networking configuration is the primary motive for moving to the Upstart script. Extracting the Kerberos-related initialization into a separate file doesn't improve the functionality at all; I did it so I could take advantage of Upstart's management features instead of manually killing the daemon. Also, it makes the core nslcd script much more readable, IMO. Hopefully that answers your questions. Just let me know if anything needs clarification. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/806761 Title: Feature Request: Upstart scripts for nslcd To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/nss-pam-ldapd/+bug/806761/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
