Code review comment for ~ahasenack/ubuntu/+source/samba:groovy-samba-4.12.2-update

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Changelog:
- [√] changelog entry correct version and targeted codename
- [√] changelog entries correct
- [√] update-maintainer has been run (well - in the past, since this goes forward on our side)

Actual changes:
- [√] no "further" major upstream changes to consider
      glad they dropped their "own" aesni, reimplementing that just is a source of issues.
      I'm also glad we have 4.12.2 stable already when moving to the new major, that should help
      to be working better.
- [√] no further upstream version to consider
      I was reviewing https://wiki.samba.org/index.php/Samba_4.12_Features_added/changed if
      there is anything else we might need to do, but as always you had all things under control
      already - a few minor things below.
- [n/a] debian changes look safe

New Delta:
- [√] new patches are good or match what was proposed upstream
- [√] new patches correctly included in debian/patches/series?
- [√] new patches have correct DEP3 metadata

Build/Test:
- [√] build is ok
- [√] autopkgtest against the PPA package passes

This is no merge of the latest Debian but the latest upstream. So our usual changelog-templates don't 100% apply, never the less I'd have expected the three dropped patches under a "* Dropped" section to spot them more easily. This is an unimportant style thing, so it is up to you if you want to change it.

Again I was wondering about some build issues related to pidl:
source4/librpc/idl/irpc.idl:28: warning: subcontext() is deprecated. Use represent_as() or transmit_as() instead
source4/librpc/idl/irpc.idl:70: warning: [out] argument `info' not a pointer
source4/librpc/idl/irpc.idl:83: warning: top-level [out] pointer `dcname' is not a [ref] pointer
source4/librpc/idl/irpc.idl:91: warning: [out] argument `num_addrs' not a pointer
source4/librpc/idl/irpc.idl:113: warning: [out] argument `generic_reply' not a pointer
source4/librpc/idl/irpc.idl:159: warning: [out] argument `info' not a pointer
source4/librpc/idl/irpc.idl:70: error: nbtd_information: [out] argument 'info' is not a pointer or array, skip client functions
source4/librpc/idl/irpc.idl:83: error: nbtd_getdcname: [out] argument 'dcname' is a pointer to type 'string', skip client functions
source4/librpc/idl/irpc.idl:91: error: nbtd_proxy_wins_challenge: [out] argument 'num_addrs' is not a pointer or array, skip client functions
source4/librpc/idl/irpc.idl:113: error: kdc_check_generic_kerberos: [out] argument 'generic_reply' is not a pointer or array, skip client functions
source4/librpc/idl/irpc.idl:159: error: smbsrv_information: [out] argument 'info' is not a pointer or array, skip client functions

But these were present in the former builds as well.
@Andreas do you think that is an issues that should be analyzed or is this known and ok?

4.12 says "GnuTLS 3.4.7 required" and in fact later bumps that to 3.6.5 for some extras.
Build dep libgnutls28-dev is unversioned at the moment for the potential that people might backport it to bionic (3.5.18) adding a >=3.6.5 might be useful - what do you think?

Note: the SMB3 speed improvements out of using tls might be worth a release not entry for gorilla what do you think?

I know you think about uring independent to this merge.

Do you want to check samba conf on upgrade an warn for removed options like "write cache size" ?

All findings are style or can be done as later improvements, +1 on the MP as presented for early groovy cycle.

review: Approve

« Back to merge proposal