Bug 453083 (Samba4)

Shouldn't this depend on the recently submitted heimdal package?

It should, but at the moment we have not finished the extraction of
Samba-specific hacks from the internal copy of Heimdal (and the incorporation of
build system magic to detect an appropriate system Heimdal etc).  We are very
close, but not quite there yet. 

Then I guess the next question is whether you would like a review of this now or
whether you've opened this to track the development effort.  Not that I can
promise to do a review myself, of course; this is a big package and several
people will probably needed share the review work.

Some initial comments from a quick look at the spec; I did not build the package:

Please use the proper versioning scheme for prerelease packages:
  Release: 0.1.alpha%{alpha_version}%{?dist}
             ^
and increment the '1' with each new release until 4.0.0 is actually released, at
which point you can just go to "Release: 1%{?dist}".  See
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Pre-Release_packages

Please use the correct License: tag; comma-separation is ambiguous and not
valid.  I'm not sure if the code is triple-licensed or if different pieces of
the built package have different licenses, but both situations are covered in
http://fedoraproject.org/wiki/Packaging/LicensingGuidelines.

Any possibility of parallel make?

The ldconfig call in %install is confusing to me.  What's it for?

Please use the proper scriptlets for user/group creation;
http://fedoraproject.org/wiki/Packaging/UsersAndGroups

Shouldn't the condrestart go in %postun, not %post?

You need the proper dependencies for the scriptlets.
  Requires(post): /sbin/chkconfig, /sbin/service
and so on.

This package is submitted for immediate review.  I hope to see it included in
Fedora well before the final release of Samba 4.0.0.

Parallel make is specifically excluded in the samba4 build process.

The ldconfig call in %install is to create the macros that a post-install
ldconfig would provide, as required by rpmlint.

The group add and other scriptlets are copied from the Samba 3.2 package. 
Please also file a bug there. 

Thanks!

Updated packages will continue to be placed at:

http://abartlet.net/samba4-rpm/

The License tag is still not correct (should be "GPLv3+ and LGPLv3+ and BSD", 
not "GPLv3+, LGPLv3+, BSD").

Frankly, like the Samba 3.2 package, the situation is not even as simple as
fixing the syntax.  While I believe all the code to be under GPLv3 compatible
licences, I'll probably have to do a full licence sweep to verify the full list.

I know we have at least some GPLv2+ code (a crypto cypher, as it happens), and
different components in Heimdal (which is bundled here) come under different
wordings of of MIT or BSD style licences. 

To give you an idea of the scope of the work, the python-wmi package is actually
a fork of Samba4 from over a year ago, but has this many licences:
http://changelogs.ubuntu.com/changelogs/pool/universe/w/wmi/wmi_0.1.6-1/python-wmi.copyright

Some of these have changed in the past year, but I use this as evidence I'm
pretty certain the licence tag header will only get longer. 

When I get the full list, I'll fix the tag.  

Well, I'll chip away at this (any external review help welcome).

1.  Should drop Epoch: 0

2.  drop
BR: ldconfig

3.  drop extraneous, hard-coded library deps
Requires: libtdb >= 0:%{tdb_version}
Requires: libtalloc >= 0:%{talloc_version}
should already be satisified by fedora's default versions (verified F9+)

4.  the -common deps
Requires(post): /sbin/chkconfig, /sbin/service, coreutils
Requires(preun): /sbin/chkconfig, /sbin/service
are misplaced.  these need to be in main pkg.  (and what's coreutils used for?)

5.  SHOULD use a recommended buildroot (cosmetic only).

Going to try some builds...

I'll chance the tdb and talloc to BuildRequires. 

We need the BR: ldconfig because we use it in the build (see %install). 

ldconfig is an exception, that you can assume is available on any (sane)
buildystem or runtime.  (I can find the packaging guideline about that if you want).

Fixes for a few of these are not in Samba.org's git tree and on my website as above

BTW, the reason I took and used the tdb_version and talloc_version stuff from
the Samba3 spec is that I do expect we might update this requirement in future,
as both are fairly core Samba packages. 

And I need to be sponsored

Updated packages are now on my web page, which should address most of these issues, and others that actual use by kdepim raised.

Taking over this effort for the time being to get evolution-mapi approved.

I've updated the Samba4 packages to the GIT revision that OpenChange (rev 909) is currently requiring.

SPEC:
http://mbarnes.fedorapeople.org/mapi/SPECS/samba4.spec

SRPM:
http://mbarnes.fedorapeople.org/mapi/SRPMS/samba4-4.0.0-0.6.alpha6.GIT.3508a66.fc10.src.rpm

Changes from Andrew's latest revision:
http://mbarnes.fedorapeople.org/mapi/SPECS/samba4.spec.diff

[root@walnut ~]# rpm -Uvh --test ~jerry/rpmbuild/RPMS/i386/samba4-common-4.0.0-0.6.alpha6.GIT.3508a66.fc11.i386.rpm ~jerry/rpmbuild/RPMS/i386/samba4-client-4.0.0-0.6.alpha6.GIT.3508a66.fc11.i386.rpm ~jerry/rpmbuild/RPMS/i386/samba4-winbind-4.0.0-0.6.alpha6.GIT.3508a66.fc11.i386.rpm ~jerry/rpmbuild/RPMS/i386/samba4-libs-4.0.0-0.6.alpha6.GIT.3508a66.fc11.i386.rpm ~jerry/rpmbuild/RPMS/i386/samba4-4.0.0-0.6.alpha6.GIT.3508a66.fc11.i386.rpm
Preparing...                ########################################### [100%]
        file /lib/libnss_winbind.so.2 from install of samba4-winbind-4.0.0-0.6.alpha6.GIT.3508a66.fc11.i386 conflicts with file from package samba-winbind-0:3.2.5-0.23.fc11.i386
        file /usr/bin/ntlm_auth from install of samba4-winbind-4.0.0-0.6.alpha6.GIT.3508a66.fc11.i386 conflicts with file from package samba-winbind-0:3.2.5-0.23.fc11.i386
        file /usr/bin/wbinfo from install of samba4-winbind-4.0.0-0.6.alpha6.GIT.3508a66.fc11.i386 conflicts with file from package samba-winbind-0:3.2.5-0.23.fc11.i386
        file /usr/lib/libnss_winbind.so from install of samba4-winbind-4.0.0-0.6.alpha6.GIT.3508a66.fc11.i386 conflicts with file from package samba-winbind-0:3.2.5-0.23.fc11.i386

Yeah, not sure what to do about the library.

File conflicts in other samba4 subpackages were all binaries, which I resolved by simply appending a '4' to the name.

Ideas?

Note, the samba4-winbind subpackage isn't needed by OpenChange.

Doesn't this need a "BuildRequires: python-devel"?

I will note that while it might not help with packaging, it is the intention of the Samba Team to only have one wbinfo and one nss_winbind, which will talk to either Samba3 or Samba4's winbindd (ie, share a protocol).

What about libnss_winbind.so, is that also compatible with Samba3?

If so, perhaps we should just disable the samba4-winbind subpackage until we move to Samba4 officially?

So, I should clarify:

Samba4 and Samba 3.4 are being developed in the samba git repository - under a branch of 'master'.  

In that branch, there will be soon (I hope) a common libnss_winbind and a common wbinfo, along with many other shared things.  

But because this is not a stable protocol, the protocol used will be different to what Samba3 in current Fedora uses.

(In reply to comment #23)
> So, I should clarify:
> 
> Samba4 and Samba 3.4 are being developed in the samba git repository - under a
> branch of 'master'.  
> 
> In that branch, there will be soon (I hope) a common libnss_winbind and a
> common wbinfo, along with many other shared things.  
> 
> But because this is not a stable protocol, the protocol used will be different
> to what Samba3 in current Fedora uses.

Then, logically, libnss_winbind and wbinfo, et. al, will not be "common", ergo they need to be packaged separately, or not at all, right?

Put another way, do the Samba developers consider the 4.x and 3.4 as environments that can exist in parallel, or not? Either it is or it isn't.

Otherwise, wouldn't the direction of this package be faulty? If one or more "shared things" are in the forecast then, it would seem, disabling it until v. 4 is "official" is the way to go.

(In reply to comment #24)
> Otherwise, wouldn't the direction of this package be faulty? If one or more
> "shared things" are in the forecast then, it would seem, disabling it until v.
> 4 is "official" is the way to go.

Bearing in mind the primary reason for packaging Samba4 at this time is to support OpenChange, and OpenChange doesn't need the winbind stuff, I'm inclined to just omit the subpackage for now.

Will try to post another round of updates later today.

Andrew, something annoying I've been struggling with:

If I already have the samba4 packages installed and I go to rebuild them, the configure script wants to link against the libldb.so from samba4-libs instead of building its own.  So I have to uninstall all my samba4 packages (along with openchange and evolution-mapi) before building new ones.

Any way to force samba to build its own ldb whether it finds one already installed or not?

Update.  Disabled the winbind subpackage so there shouldn't be any more file conflicts.

http://mbarnes.fedorapeople.org/mapi/SPECS/samba4.spec
http://mbarnes.fedorapeople.org/mapi/SRPMS/samba4-4.0.0-0.7.alpha6.GIT.3508a66.fc10.src.rpm

Update.  Found another file conflict with Samba3 (/usr/bin/smbstatus).

http://mbarnes.fedorapeople.org/mapi/SPECS/samba4.spec
http://mbarnes.fedorapeople.org/mapi/SRPMS/samba4-4.0.0-0.8.alpha6.GIT.3508a66.fc10.src.rpm

(In reply to comment #28)
> Update.  Found another file conflict with Samba3 (/usr/bin/smbstatus).
> 
> http://mbarnes.fedorapeople.org/mapi/SPECS/samba4.spec
> http://mbarnes.fedorapeople.org/mapi/SRPMS/samba4-4.0.0-0.8.alpha6.GIT.3508a66.fc10.src.rpm

Processing files: samba4-devel-4.0.0-0.8.alpha6.GIT.3508a66.fc11
error: File not found: /home/jerry/rpmbuild/BUILDROOT/samba4-4.0.0-0.8.alpha6.GIT.3508a66.fc11.i386/usr/lib/libldb.so


RPM build errors:
    File not found: /home/jerry/rpmbuild/BUILDROOT/samba4-4.0.0-0.8.alpha6.GIT.3508a66.fc11.i386/usr/lib/libldb.so

Just me? I haven't had a chance to research why yet.

See comment #26.

You have to uninstall samba4 before rebuilding it, otherwise it just links to the already-installed libldb.so and doesn't build it itself.

/me smacks hand on forehead, I knew libldb.so sounded familiar!

my apologies, my time/interest has waned, so I'll withdraw to let someone else jump in.

Update for official Alpha6 release.  No changes to packaging.

http://mbarnes.fedorapeople.org/mapi/SPECS/samba4.spec
http://mbarnes.fedorapeople.org/mapi/SRPMS/samba4-4.0.0-1.alpha6.fc10.src.rpm

(In reply to comment #26)
> Andrew, something annoying I've been struggling with:
> 
> If I already have the samba4 packages installed and I go to rebuild them, the
> configure script wants to link against the libldb.so from samba4-libs instead
> of building its own.  So I have to uninstall all my samba4 packages (along with
> openchange and evolution-mapi) before building new ones.
> 
> Any way to force samba to build its own ldb whether it finds one already
> installed or not?

Has progress been made here? Annoying, yes, and I'm guessing would be a blocker at some point...

Only solution I can think of that doesn't involve hacking on Samba's build scripts is to split ldb into it's own standalone package (not a subpackage) and list it as a BuildRequires in the samba4.spec.

Red Hat Legal is still deliberating over this one. Once I hear back from them, I should be able to move forward (in one way or another).

Lifting FE-Legal, Red Hat has determined this can proceed.

Will we see this in F11-Rawhide soon?

Is it possible to add winbind for those of us who are playing with it looking to the day it can replace Samba3?

(In reply to comment #38)
> Will we see this in F11-Rawhide soon?

Not until someone reviews the package.


> Is it possible to add winbind for those of us who are playing with it looking
> to the day it can replace Samba3?

For now, Samba4 is being added primarily to support OpenChange, and OpenChange does not require the winbind subpackage.  It also conflicts with Samba3.

The srpm from comment 33 builds fine in mock.

Here is the rpmlint output:

samba4.i386: W: no-documentation
samba4.i386: E: non-standard-dir-perm /var/log/samba4/old 0700
samba4.i386: E: non-standard-dir-perm /var/log/samba4 0700
samba4.src: W: mixed-use-of-spaces-and-tabs (spaces: line 115, tab: line 145)
samba4-client.i386: W: no-documentation
samba4-common.i386: E: non-standard-dir-perm /var/lib/samba4/private 0700
samba4-debuginfo.i386: W: spurious-executable-perm /usr/src/debug/samba-4.0.0alpha6/source4/torture/raw/pingpong.c
samba4-devel.i386: W: no-documentation
samba4-python.i386: W: no-documentation
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/ndr.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/samdb.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/__init__.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/tests/dcerpc/unix.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/tests/samdb.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/samba3.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/getopt.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/tests/dcerpc/rpcecho.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/tests/dcerpc/sam.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/hostconfig.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/tests/dcerpc/registry.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/tests/__init__.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/idmap.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/tests/samba3.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/tests/upgrade.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/upgrade.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/tests/dcerpc/bare.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/dcerpc/__init__.py 0644
samba4-python.i386: E: non-executable-script /usr/lib/python2.6/site-packages/samba/tests/provision.py 0644
9 packages and 0 specfiles checked; 22 errors, 6 warnings.

Looks like the description of the -python subpackage should end in a period, not a slash.

And I think python wants to be capitalized when used as a name (compare descriptions of other python packages).

There is some mixed use of $RPM_BUILD_ROOT and %{buildroot} that should probably be cleaned up always use the one or the other.

I think the disabling of the winbind package would better be done with

%if %enable_winbind

or somesuch

the main samba4 package include something called /usr/bin/tdbtorture, which from the looks of it, is a test program for the tdb code. As such, it should probably go in the -devel package, if at all.

Similar for smbtorture and subunitrun in the -common subpackage

And I'm not sure we want a program called /usr/bin/nsstest shipped as part of samba4-devel. 


The -python subpackage has a bunch of autogenerated provides that I don't think it should have:
atsvc.so  
auth.so  
base.so  
com.so  
credentials.so  
dfs.so  
drsuapi.so  
echo.so  
epmapper.so  
glue.so  
initshutdown.so  
irpc.so  
ldb.so  
lsa.so  
messaging.so  
mgmt.so  
misc.so  
nbt.so  
net.so  
netbios.so  
param.so  
registry.so  
samr.so  
security.so  
svcctl.so  
tdb.so  
tevent.so  
unixinfo.so  
uuid.so  
winreg.so  
wkssvc.so

The -devel package shouldn't own /usr/lib/pkgconfig

If you clean up these things, I'll start a formal review.

Simo and I were talking about splitting several Samba libraries (talloc, tevent, tdb and ldb) into a separate, standalone "samba-base" package to be consumed by Samba3, Samba4, OpenChange and SSSD [1].  We also think we want to disable, for the time being, any parts of Samba4 not needed by OpenChange (by means of some simple toggle settings in the spec file).

Expect more churn here before a formal review can start.  In the meantime I'll clean up the details Matthias pointed out.  Thanks again, Matthias.

[1] http://fedoraproject.org/wiki/Features/SSSD

Any update here ?

It's looking like Samba4 is again not going to make the cut.  I'm waiting on Simo Sorce to deliver a interim subset package containing the Samba4 libraries needed for OpenChange.

Release of separate libraries is also slipping :-(

Matt I think it would be safe for now to just grab your own copy of samba4 frozen in a status the openchange people is ok with, and just build the few libraries you need yourself and store them into a private path as part of the openchange package.

While not ideal, I think this will make it easier to release openchange in F11, once there, we will try to gradually split out dependencies as pieces stabilize upstream and become available as official releases.

Too much indecision here... 
Matt, lets just go with the initial packaging approach.

Agreed.  I'll continue addressing your review comments.

I don't want to be supporting a quick and dirty hack in RHEL 6.

Is there anything more I can do from the Samba4 development standpoint to help this along?   Do you need another alpha snapshot, or is it OK to cut a GIT revision at the appropriate time?

Summary of some offline conversation with Simo:

The approach we're going to try is to stick with the existing Samba4 package already under review here, but for Fedora 11 disable the bits not needed by OpenChange.  Basically lobotomize Samba4 until we're ready to support it.

Simo needs a more recent revision of talloc and tdb, so we'll patch Samba3 to provide that (assuming those libraries haven't broken backward compatibility).

I expect OpenChange to be frozen at 0.8 for the duration of Fedora 11, so alpha6 is sufficient for me.  If Simo needs something newer I'd prefer a new upstream release over rolling my own GIT revision, to avoid distro-specific quirks.

I fully support this plan of action.

Yep I definitely need the latest tevent and ldb code from samba4.

New packages up for review.  This update disables most of Samba4, leaving only what OpenChange needs.  It also addresses most of Matthias' review comments.  Not sure how to deal with the Python provides issue in comment #47, but the subpackage is disabled so it's probably not worth worrying about for now.

Successfully built and installed OpenChange and Evolution-MAPI against this package set.

http://mbarnes.fedorapeople.org/mapi/SPECS/samba4.spec
http://mbarnes.fedorapeople.org/mapi/SRPMS/samba4-4.0.0-2.alpha6.fc10.src.rpm

Indeed, all my earlier comments have been taken care of, thanks.

Package builds fine in mock

Formal review: 

rpmlint output: 

samba4.x86_64: E: no-binary
samba4-devel.x86_64: W: no-documentation
6 packages and 0 specfiles checked; 1 errors, 1 warnings.

The warning is ignorable, the error is caused by the main package being an empty shell for now. I understand this is just temporary, until samba4 gets
released, so I don't think this is an issue.

package name: ok
spec file name: ok
packaging guidelines: ok; I guess you could be proactive and adapt to the coming recommendation of %global over %define, but thats not ratified yet, afaik
license: ok
license field: ok, but it would be nice to specify more exactly what parts are LGPL
license file: ok
spec language: ok
spec legible: ok
upstream source: ok
buildable: ok
buildrequires: ok
excludearch: ok
locale handling: ok
ldconfig: ok
relocatable: ok
directory ownership: ok
duplicate files: ok
permissions: ok, I notice that pidl uses %defattr(-,root,root,-) whereas the others use %defattr(-,root,root). Accident ? The former is preferred, I think
%clean: ok
macro use: ok
permissible content: ok
large docs: ok
%doc content: ok
headers: ok
pkgconfig: ok
shared libs: ok
-devel requires: ok, it requires -libs
la files: ok
gui apps: ok
overlap with other packages: NOT ok. -pidl includes things that are owned by other packages, notably perl-Parse-Yapp
%install: ok
utf8 filenames: ok


summary: 
- consider using %global
- add license comment
- consider cleaning up %defattr variation
- fix -pidl conflicts

Latest (hopefully final) update.

http://mbarnes.fedorapeople.org/mapi/SPECS/samba4.spec
http://mbarnes.fedorapeople.org/mapi/SRPMS/samba4-4.0.0-3.alpha6.fc10.src.rpm

> summary: 
> - consider using %global

I think I'll hold off on this until it's ratified and just convert all my packages en masse.

> - add license comment

Done.  Simo said the library licenses are still subject to GPL/LGPL fluctuation, but everything else is GPL.

> - consider cleaning up %defattr variation

Done.

> - fix -pidl conflicts

Done, I think.  Only conflicts I found were:

   /usr/lib/perl5/vendor_perl/5.10.0/Parse/Yapp
   /usr/lib/perl5/vendor_perl/5.10.0/Parse/Yapp/Driver.pm

I removed Samba's copy and added perl-Parse-Yapp as a build requirement.
Were there any others?

Were there any others?

I think /usr/lib/perl5/vendor_perl/5.10.0/Parse

is multiply owned. Not sure if thats intentional, or if there is some official owner. Easiest way out might be to Require perl(Parse::Yapp) - or do you pick that Requires up anyway ?


The rest looks ok, so approved under the assumption that you have a Requires for an owner of that directory.

(In reply to comment #64)
> I think /usr/lib/perl5/vendor_perl/5.10.0/Parse
> 
> is multiply owned. Not sure if thats intentional, or if there is some official
> owner. Easiest way out might be to Require perl(Parse::Yapp) - or do you pick
> that Requires up anyway ?

Not sure if build requirements get propagated as requirements with Perl modules.  I'll just make it an explicit requirement, to be sure.

Thanks for the review.

New Package CVS Request
=======================
Package Name: samba4
Short Description: Samba version 4
Owners: mbarnes
Branches:
InitialCC: simo

cvs done.

Package built.  Closing review.  Finally.

http://kojipkgs.fedoraproject.org/packages/samba4/

gnome-vfs2-2.24.4-9.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/gnome-vfs2-2.24.4-9.fc18