[Bug 1526768] Review Request: clickhouse - a distributed column-oriented DBMS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1526768



--- Comment #5 from Roman Tsisyk <roman@xxxxxxxxxx> ---
Hi Michael,

Thanks for the fast response.

* The BuildRequires are at the top - I'd expect the name block at the top.

BuildRequires have been moved to a proper place.

Fixed.

* This bundles cctz without any comments as to why

According to Github [1], the latest released version of google/cctz is 2.1, but
ClickHouse's scripts needs src/zone_info_source.cc which was added after 2.1
release. I added a comment about that to spec file. This package is very tricky
in terms of dependencies, as you can see in the BuildRequires list.

[1] https://github.com/google/cctz/releases

* Have you tried unbundling boost? Although it looks for 1.60 the bundled
version is 1.65 so I'd guess the version isn't actually that critical - a small
patch to remove the version number would most likely be all that's required

Switched to boost-devel. Initially it didn't compile, but it seems that my
combination of CMake flags has fixed this problem.
Fixed.

* The CMake file seems to support ARM builds - any reason to make this
exclusivearch other than the docs claiming it is?

Upstream claims that "Only x86_64 with SSE 4.2 is supported. Support for
AArch64 is experimental." [2]. I tried to compile aarch64 on my test host and
realized that some code under #ifdef..#else..#endif is completely broken. I
fixed this problem and send a pull request to upstream [2], hope they will
merge it soon. Using this fix I was able to reach 99% completeness in CMake.
Unfortunately, linking of C++ code requires a lot of memory and fails with OOM
on my coin-size aarch64 mini-PC. 

[2] https://clickhouse.yandex/docs/en/development/build.html
[3] https://github.com/yandex/ClickHouse/pull/1674

Plan:

- Wait until pull request is accepted or include this fix as a patch
- Re-try on an aarch64 host with more RAM
- Enable or disable aarch64 in ExclusiveArch depending on result

I don't think that aarch64 supports blocks review. I'll make the best efforts
to fix aarch64 both in upstream and this package. 

* Groups aren't needed or particularly wanted [1]

I removed `Group:` tag.
Fixed.

Full changelog:

    - Re-order BuildRequires
    - Add `sed` to BuildRequires
    - Add a comment about google/cctz
    - Remove `Group`
    - Use `boost-devel` instead of bundled boost
    - Enable experimental aarch64 support


Spec URL:
https://gist.githubusercontent.com/rtsisyk/1a45e69293f3e2c91126fe04a8374af8/raw/f43fc9f556998507bfbbdfbee50410f1b6c8c39b/clickhouse.spec
SRPM URL:
https://gist.github.com/rtsisyk/1a45e69293f3e2c91126fe04a8374af8/raw/f43fc9f556998507bfbbdfbee50410f1b6c8c39b/clickhouse-1.1.54326-1.fc26.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux