[Pkg-samba-maint] master branch review

[Michael Tokarev]

02.04.2022 11:39, Mathieu Parent wrote:
> Hi Michael,
> 
> I've reviewed the master branch up to  e5eaefa1775fe841a5e0e81f2c25045d7c95c2bf
> 
> You've done an amazing job! The package is in much better shape now.

Thank you Mathieu :)  Especially thank you for your time
and for the review!

> A few questions:
> 
> - 9ab2a406b2972e18e1568963ecf4fe949d5d4921 "d/rules: specify the
> package name for dh_installpam". The result is that the pam config is
> kept in samba-common?

Yes it stayed in samba-common as it was before.  Maybe some day
we'll move it to samba, - I just need to figure out how to move
a conffile between packages, - it is different from moving a
regular file, since we should preserve changes and transfer
ownership.

> - 9d27f1aa68240921451a0b27d19671c5b13c6f91 "fix long-standing issues
> in shlibdeps handling". Wow! I don't understand completely, but
> thanks!

Yes, this was really shocking for me.

To see what is going on, you can run dh_shlibdeps in verbose
mode (-v), it'll show dpkg-shlibdeps invocation it performs.
For *every* dh_shlibdeps invocation which was in there (which
were supposed to be run against single file in the install
path), there were single invocation of dpkg-shlibdeps for
every *package* listed in d/control. dh_shlibdeps loops over
every package (unless -p is specified) finding every binary
in each, so each

   dh_shlibdeps -- -e foo/bar

were turning into this:

   for pkg in (dh_listpackages); do
     dpkg-shlibdeps ... -e foo/bar $(find debian/$pkg/ -executable)
   done

first dh_shlibdeps call in d/rules were supposed to iterate
over all packages, and subsequent ones (4 or 5 of them)
were *supposed* to act on the given file only, but in fact
these too were iterating over the same files.

It is even logically wrong when you look at subsequent
dh_shlibdeps invocations. Each of them is the same, the
difference is within *additional* arguments being passed
to dpkg-shlibdeps:

   dh_shlibdeps -a -l.. *--* -e foo/bar -pbar
   dh_shlibdeps -a -l.. *--* -e foo/baz -pbaz
   ...

note the "--" which ends dh_shlibdeps options processing.
So dh_shlibdeps does not know it should act on a single file,
it acts on *everything* as it is supposed to do without
restricting it to some packages or files.

No doubt this step produced wrong results and took 5 or 6
times more resources than needed, and produced as much more
warnings too (so it become unrealistic to see what it warns
about).

Yes, this turned out to be a huge mess, I definitely didn't
expect to find anything like this.

With this change, and with replacing waf build with
waf install, the build procedure requires about 1/3
time than before :)

> - Have you tested building sssd with it?

Nope. It *should* work, -- I'm confident here since
the resulting binary package is identical as what were
produced by ldb source package. But this definitely
should be verified. For one, I didn't try building
sssd with ldb-2.5 at all, neither built from its own
source nor from samba.

> Appart from that, some of the patches need to be forwarded, but you
> already know that.

I forwarded them already. Too bad upstream also wants
me to create gitlab account too, - I'm fine to keep
them in debian, it was already *way* too much work :)

> IMO, the package is ready to be uploaded to experimental.

Yes, I think it was ready 2 days ago already. But I didn't
know about the shlibdeps thing at that time. And I fixed the
i386 (32bit) build just yesterday, - had to create an i386
sbuild chroot here to see what's going on.

I'd love to do some more changes in there, - in particular,
the ctdb_etcd_lock thing needs quite some love. We already
have ctdb:Recommends: python3-etcd, so python is already
there actually. But this file should be put into some
arch-neutral directory and all references to it (in docs
and examples) should be fixed.  Also the sssd test build
is a must too.

But we're waiting for the python3 transition to settle
down now. It might be happening for quite some time.

Thank you very much Mathieu for all the work and your review.
It is much appreciated. Nothing is more motivating and
encouraging than a good approval of someone who know
the thing best!

/mjt