Samba initscript does not conform to the LSB Spec.

[Paolo Dina]

/etc/init.d/samba doesn't implement the status operation at all, while it is requested by Linux Standard Base Core Specification.

Details on initscripts' LSB compliance may be found here:
http://www.linuxbase.org/spec/refspecs/LSB_3.0.0/LSB-Core-generic/LSB-Core-generic/iniscrptact.html

[Phil Bull (philbull)]

Thanks for the report.

I can confirm this.

[Phil Bull (philbull)]

Doesn't seem to implement try-restart either.

[Shawn Smith (asorbus)]

I would like to work on this, I tried modifying the init script but status never reaches nmbd because it exits when smbd's status is found.

[Dustin Kirkland  (kirkland)]

On Wed, Mar 5, 2008 at 2:01 PM, shawn <email address hidden> wrote:
> I would like to work on this, I tried modifying the init script but
> status never reaches nmbd because it exits when smbd's status is found.
>
> ** Attachment added: "samba"
> http://launchpadlibrarian.net/12447200/samba

Hi there-

I'm a big fan of 'status' in init scripts--there's a bunch of places
in Ubuntu that could use some improvement there ;-)

A couple of comments and pointers...

1) Minor nit, but your whitespace in the status section is not in line
with the other case stanzas
2) Take a look at an init script that has a working status stanza, for
instance, /etc/init.d/ntp:
3) Instead of exiting 0, do like the ntp init script does, which is
print an appropriate message via log_success_msg or log_failure_msg,
and then exit with the status error code, instead of hard coding the
exit code.

Does that help?

:-Dustin

[Shawn Smith (asorbus)]

I actually don't have /etc/init.d/ntp, and I've grepped for status\) in all of the init scripts that I have and I haven't found a clean looking solution yet. Would you mind posting /etc/init.d/ntp for me to look at, Dustin?

[Dustin Kirkland  (kirkland)]

Better yet, let me show you how to pull the source and find it ;-)

cd /tmp
mkdir ntp
cd ntp
apt-get source ntp
cat ntp-*/debian/ntp.init

Cool?

:-Dustin

[Shawn Smith (asorbus)]

Hey, I get this error when I tried to pull the source for ntp:

apt-get source ntp
Reading package lists... Done
Building dependency tree
Reading state information... Done
NOTICE: 'ntp' packaging is maintained in the 'Svn' version control system at:
svn://svn.debian.org/pkg-ntp/ntp/
Need to get 3023kB of source archives.
Get:1 http://mirrors.ccs.neu.edu gutsy/main ntp 1:4.2.4p0+dfsg-1ubuntu2 (dsc) [1010B]
Get:2 http://mirrors.ccs.neu.edu gutsy/main ntp 1:4.2.4p0+dfsg-1ubuntu2 (tar) [2819kB]
Get:3 http://mirrors.ccs.neu.edu gutsy/main ntp 1:4.2.4p0+dfsg-1ubuntu2 (diff) [203kB]
Fetched 3023kB in 1s (2699kB/s)
sh: dpkg-source: not found
Unpack command 'dpkg-source -x ntp_4.2.4p0+dfsg-1ubuntu2.dsc' failed.
Check if the 'dpkg-dev' package is installed.
E: Child process failed

[Dustin Kirkland  (kirkland)]

On Wed, Mar 5, 2008 at 6:49 PM, Shawn Smith <email address hidden> wrote:
> sh: dpkg-source: not found
> Unpack command 'dpkg-source -x ntp_4.2.4p0+dfsg-1ubuntu2.dsc' failed.
> Check if the 'dpkg-dev' package is installed.

You could solve that by doing an "apt-get install dpkg-dev" and then
redo the "apt-get source ntp".

Otherwise, the Debian source looks something like this:
http://svn.debian.org/viewsvn/pkg-ntp/ntp/trunk/debian/ntp.init?rev=111&view=markup

:-Dustin

[Shawn Smith (asorbus)]

I checked out the NTP init script and modified what I have, I'm still unsure about including nmbd though, is it necessary?

[Shawn Smith (asorbus)]

Uploading another version to see if the status) whitespace matches up this time, as well as deleting unnecessary stuff I had added in earlier.

[Dustin Kirkland  (kirkland)]

On Wed, Mar 5, 2008 at 9:34 PM, Shawn Smith <email address hidden> wrote:
> I checked out the NTP init script and modified what I have, I'm still
> unsure about including nmbd though, is it necessary?
>
> ** Attachment added: "samba"
> http://launchpadlibrarian.net/12455025/samba

Hi Shawn-

First of all, your whitespace is till off. What editor are you using?

Second of all, it's very difficult to see the changes you have made to
that file. What would be much better is a file showing the
*differences*. For that, we have a utility called "diff".

You will need an unmodified samba script (hopefully you saved one),
and the one where you've made your changes. Call the old one
"samba.orig" or something. Then do:

diff -up samba.orig samba > /tmp/samba_status.patch

Have a look at that patch. It will show you the lines that you've
added (+) and the lines that you've removed (-). It will be much
easier for us to read. Also, you or anyone else can "apply" that
patch to their samba script using the "patch" utility to assume your
changes.

Do that, and post the diff here... Thanks.

:-Dustin

[Shawn Smith (asorbus)]

Here's the diff, I'm using vim to edit the file and in vim the whitespace appears to match up.

[Dustin Kirkland  (kirkland)]

On Fri, Mar 7, 2008 at 11:09 AM, Shawn Smith <email address hidden> wrote:
> Here's the diff,

Okay, cool, Shawn, you're almost there.

> I'm using vim to edit the file and in vim the
> whitespace appears to match up.

Okay, vim is good!

Now the whitespace isn't quite right because there's a difference
between individual spaces (the spacebar) and tabs (the tab key).

In vim, go up to those other lines and move with your cursor. Does it
jump one character at a time, or multiple? If one character, then the
previous code is using lots of single spaces. You should do the same.
 If it jumps blocks at a time, then it's using tabs, and you should do
the same. It might well be a combination of both (not my favorite
spacing format, but hey....). In any case, you should *definitely*
match the standard shown in the rest of the file.

As for the code...you define DESC but never use it. Dead code can be
confusing and is a no-no.

Other than that, this looks good to me. I'd give it a +1. The last
thing to do is to generate a real debdiff. I'll post a followup
tomorrow with more detailed instructions.

:-Dustin

[Shawn Smith (asorbus)]

Here is the final init script, with (hopefully) fixed tabs.

[Dustin Kirkland  (kirkland)]

On Sat, Mar 8, 2008 at 8:14 PM, Shawn Smith <email address hidden> wrote:
> Here is the final init script, with (hopefully) fixed tabs.

Hi Shawn-

You're nearly there... A couple of things left.

* The log_*_msg lines inside of the if/else are not indented properly
* You attached the script again... We always deal in diffs and patches! :-)
* Speaking of diffs and patches... You're proposing that this change
be accepted into Ubuntu. For that to happen (more easily), you should
really attach a debdiff. Here are detailed instructions that I used
to generate a proper debdiff from your code.
* mkdir -p /work/source/samba # Make a working directory. Perhaps
/work/source/samba
* cd /work/source/samba # go to that directory
* apt-get source samba # grab the source of samba
* wget http://launchpadlibrarian.net/12504891/samba_status.patch #
grab your patch
* cd samba-*/debian/ # go to the unpacked directory
* patch < ../../samba_status.patch # apply your patch
* At this point, this patch has some bad white space problems, I'm
going to manually fix those for you
* cd .. # back one directory
* dch -i # Add a changelog entry describing your changes. This
format is VERY VERY VERY particular. Look at the other entries and
try to follow those.
* Notice that the -i will increment the version. Notice the
indentation on the changes, followed by asterisk bullets. Notice that
the file name that was changed, followed by a colon, then a
description of the rationale for the changes. Because I'm creating
the debdiff, my name and email address appear at the bottom of the
changes. Since you did the hard work, I added a line giving you
credit for the initial patch ;-) Finally, notice the (LP: XXXXXX),
That part is very important. This is how bugs get closed by patches.
It must match that format exactly.
* Save and quit
* apt-get install build-essential # get the build tools
* apt-get build-dep samba # get the build dependencies for samba
* debuild -uc -us # test building a new build, ~10 minutes
* debdiff > ../samba_status.diff
* Have a look at samba_status.diff, note that ONLY the desired changes
are present, and give the changelog another look over, for grammar,
spelling, and format.
* You would attach that debdiff here (I've attached this one)

There are many, many service scripts in /etc/init.d that are missing
"status" sections. Perhaps you might want to try your hand at another
one? Open a separate bug of course.

:-Dustin

[Dustin Kirkland  (kirkland)]

Better yet, see the documentation in the wiki on creating a debdiff:

https://wiki.ubuntu.com/PackagingGuide/Recipes/Debdiff

:-Dustin

[Shawn Smith (asorbus)]

Most recent patch to test Dustin's instructions on making a debdiff.

[Launchpad Janitor (janitor)]

This bug was fixed in the package samba - 3.0.28a-0ubuntu1

---------------
samba (3.0.28a-0ubuntu1) hardy; urgency=low

  [Chuck Short]
  * New upstream release. This fixes the following Ubuntu bugs.
    - Prevent nmbd from shutting down when no network interfaces can be
      located. (LP: #180493)
    - Fixes I/O errors on access to SMB shares of OS/2. (LP: #112839)
  * Dropped patches:
    - make-distclean.patch
    - linux-cifs-user-perms.patch
    - cifs-umount-same-user.patch
    - get_global_sam_sid-non-root.patch
    - chgpasswd.patch
    - cups.patch
    - samba-syslog.patch
  * debian/mksambapasswd.awk
    - Don't add user with UID less than 1000 to smbpasswd. (LP: #199412)
  * debian/samba.if-up
    - ifup hook to reload samba once the interfaces comes up. (LP: #180493)
  * Updated control version.

  [Nicolas Valcárcel]
  * debian/patches/fix-documentation.patch
    - Fixed some escape typos in smb.conf(5) manpage. (LP: #182571)

  [Shawn Smith]
  * debian/samba.init
    - Samba init script does not conform to the LSP specification; it needs a
      status section (LP: #39157)

 -- Chuck Short <email address hidden> Tue, 11 Mar 2008 14:21:29 -0400

[Paolo Dina (paolodina)]

Thanks for the fix!