Hello, Thanks for having a look at it. I uploaded a new version of the package on mentors
https://mentors.debian.net/package/clickhouse The respective dsc file can be found at: https://mentors.debian.net/debian/pool/main/c/clickhouse/clickhouse_1.1.54284-1.dsc My answer inline On 28/07/2017 12:35, Andrey Rahmatullin wrote: > Control: tags -1 + moreinfo > > Some preliminary remarks: > > Why B-D specifically on gcc-6, g++-6? Because clickhouse didn't built with gcc-5. Recent versions now build with gcc-7 so I removed the B-D > Version in Depends: lsb-base (>= 3.2-14) can be dropped. Done > Current Standards-Version is 4.0.0. > Current debian/compat is 10. Done > Are you sure this Pre-Depends usage is correct? Have you discussed it as > required by the Policy? I suppose it's because clickhouse-client.postinst > uses the user created in clickhouse-common.postinst but I don't think it > is needed in this case. Not discussed it. I guess I took it from the package I used as an example (or even the upstream Debian work). Will see that > Besides, I think you shouldn't use chown to change ownership, see > https://www.debian.org/doc/debian-policy/ch-files.html#s-permissions-owners Will see that > Besides, I don't think it's advisable to have a dir in /etc owned by a > service user (while the group will remain root? why?). For now, I just mimic the upstream behaviour. Since clickhouse generate a preprocessed version of the xml config file, the user must have write access to /etc Is there any good practices for such cases ? > The current consensus is to not delete users on purge. Done > AUTHORS and README.md are included in several subpackages, this seems > wrong. And in debian/clickhouse-utils.docs they are listed twice. Done > There seems to be a typo in the PID file name in > debian/clickhouse-server.service Done > Some of the descriptions miss the trailing period. Done > Is debian/copy_clang_binaries.sh unused? Upstream inheritage. Removed > debian/libclickhouse1.symbols is empty. Yeah, I still have issues to manage it properly. Removed for now. > debian/copyright is very wrong with a single copyright and license for the > entire project: it lacks licenses and copyrights for all bundled third-party > code, there are several MIT-licensed files in the main code and docs, and > that's without doing a full analysis. Fixed. I did not include embedded libs which are removed during build: in the future, I intend to remove them before merging new upstream version using repack > Now to debian/rules. > Why --max-parallel=4? > override_dh_auto_clean runs dh_clean, not dh_auto_clean, and it also does > a bunch of rm's while dh_clean can do that with debian/clean. > I think removing embedded copies should be done in clean, not in build. > dh_auto_configure should run cmake, not override_dh_auto_build. > Manual make -j`grep -c ^processor /proc/cpuinfo` is wrong. > "will wait for the ability to use system libs instead of contrib ones" > comment is worrying, so did that already happen or not? > copy_headers.sh seems wrong. > dh_install --sourcedir=debian/tmp is the default, isn't it? In any case it > shouldn't be run in override_dh_auto_install. > Creating dirs should be done by dh_installdirs. Made some updates to d/rules Not sure it's perfect, but it still better than it was > And the build fails in sbuild: > > ./debian/tests_wrapper.sh > /<<PKGBUILDDIR>> > grep: ../../debian/clickhouse-server-config-tests-preprocessed.xml: No such > file or directory > grep: /etc/clickhouse-server/config-preprocessed.xml: No such file or > directory > > Running stateless tests. > > 00001_select_1: [ FAIL > ] - return code 232 > /usr/share/zoneinfo/Asia/Yekaterinburg: No such file or directory > Poco::Exception: Exception: Cannot load time zone Asia/Yekaterinburg > [...] > 00039_inserts_through_http: [ FAIL > ] - return code 7 > curl: (7) Failed to connect to localhost port 8123: Connection refused > [...] > Having 58 errors! > ./debian/tests_wrapper.sh: line 30: 31404 Aborted (core > dumped) PATH=$PATH:./build/dbms/src/Server > ./build/dbms/src/Server/clickhouse-server > --config-file=./debian/clickhouse-server-config-tests.xml 2> /dev/null (wd: > /<<PKGBUILDDIR>>) > ./debian/tests_wrapper.sh: line 18: kill: (31404) - No such process > debian/rules:31: recipe for target 'override_dh_auto_test' failed Done
signature.asc
Description: OpenPGP digital signature
