Bug 237318 - databases/clickhouse update to 19.5.3.8
Summary: databases/clickhouse update to 19.5.3.8
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Kai Knoblich
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-04-16 14:15 UTC by proler
Modified: 2019-04-29 00:00 UTC (History)
3 users (show)

See Also:


Attachments
patch (60.04 KB, patch)
2019-04-16 14:15 UTC, proler
no flags Details | Diff
patch 19.5.3.8 (60.10 KB, patch)
2019-04-18 21:54 UTC, proler
no flags Details | Diff
clickhouse-19.5.3.8-preliminary.patch (59.93 KB, patch)
2019-04-27 12:59 UTC, Kai Knoblich
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Kai Knoblich freebsd_committer 2019-04-18 19:20:43 UTC
(In reply to proler from comment #0)

Hi, thank you for your patch. With initial testing there were some issues with the pkg-plist:

> ====> Checking for pkg-plist issues (check-plist)
> ===> Parsing plist
> ===> Checking for items in STAGEDIR missing from pkg-plist
> Error: Orphaned: lib/libclickhouse.so.19.5.2.1
> Error: Orphaned: @dir %%DATADIR%%/headers/19.5.2.1
> ===> Checking for items in pkg-plist which are not in STAGEDIR
> Error: Missing: lib/libclickhouse.so.%%SOVERSION%%
> Error: Missing: @dir %%DATADIR%%/headers/%%SOVERSION%%
> ===> Error: Plist issues found.
> *** Error code 1

If I change SOVERSION=${PORTVERSION} to SOVERSION=19.5.2.1 the build was fine for all FreeBSD releases on amd64.

There are also some lines that are commented out. I guess these lines can be removed then?
Comment 2 proler 2019-04-18 21:54:51 UTC
Created attachment 203774 [details]
patch 19.5.3.8
Comment 3 proler 2019-04-18 21:55:58 UTC
Updated version with fixed SOVERSION attached.

Yes, you can remove any commented lines.
Comment 4 ev 2019-04-24 05:00:10 UTC
(In reply to Kai Knoblich from comment #1)

Have any news about this patch?
What are the plans to commit this patch?
Comment 5 Kai Knoblich freebsd_committer 2019-04-25 20:12:26 UTC
(In reply to ev from comment #4)

Thank you for the ping. Your updated patch builds fine with the default options. There are two issues at the moment with non-default options:

If the TEST option is set to off, the build doesn't pass the stage-QA:

> ====> Running Q/A tests (stage-qa)
> Error: '/usr/local/bin/python' is an invalid shebang you need USES=shebangfix for 'bin/clickhouse-test'
> Error: '/usr/local/bin/python' is an invalid shebang you need USES=shebangfix for 'share/clickhouse-test/performance/create_benchmark_page.py'
> *** Error code 1

This is a bit strange as both files are given to the SHEBANG_FILES variable in the port's Makefile. There might be the chance that these are modified again during the build process. I'm currently investigating that issue.

The other one might be a bug with Clang80 on 13.0-CURRENT and occurs only if the CTEST option is set:

> Assertion failed: (getActiveBits() <= 64 && "Too many bits for uint64_t"), function getZExtValue, file /usr/local/poudriere/jails/13Camd64/usr/src/contrib/llvm/include/llvm/ADT/APInt.h, line 1566.
> c++: error: unable to execute command: Abort trap (core dumped)
> c++: error: clang frontend command failed due to signal (use -v to see invocation)
> FreeBSD clang version 8.0.0 (tags/RELEASE_800/final 356365) (based on LLVM 8.0.0)

My plan is:
- Investigate the issue when the TEST option is unset and fix it. If you have a patch for it that would be fine and sped things a bit up.
- Do a bit more tests regarding the Clang80 issue and open an PR for it. After that mark the CTEST option as broken when using on FreeBSD 13.0-CURRENT.

If everything goes well the databases/clickhouse update will be committed during the upcoming weekend.
Comment 6 Kai Knoblich freebsd_committer 2019-04-27 12:59:48 UTC
Created attachment 204063 [details]
clickhouse-19.5.3.8-preliminary.patch

Here's an updated version of your patch:

I was able to identify the issue with the SHEBANG_FILES when the TEST option is unset. Both Python scripts are installed always regardless of the TEST option.

The description of the TEST option is a bit misleading because it only installs two additional files and all the %%DATADIR%%-test/* files are installed unconditionally.

Thus I did following:

- Converted the dependencies of the TEST option to a default installation
- Re-Used the TEST option then a alternative for the CTEST option as it compiles some more files for testing purposes.
- Set SOVERSION accordingly because it slightly differs from PORTVERSION.

The build still crashes on 13.0-CURRENT amd64 with TEST (former: CTEST) set to on. I'll file a PR for this later on the day.

If this sound reasonable and the patch looks ok for you I'll use that patch then.
Comment 7 Kai Knoblich freebsd_committer 2019-04-27 13:01:08 UTC
P.S.: Any special reason why "compiler:c++14-lang" was switched to "compiler:c++17-lang"? It builds fine with both variants.
Comment 8 proler 2019-04-27 21:40:10 UTC
clickhouse uses c++17 features and cant be compiled with c++14 only compiler.

Patch is good. Please commit.

Working server have high priority.

Tests have low priority because many of them fails under freebas and require many work to fix/skip them.
Comment 9 commit-hook freebsd_committer 2019-04-28 23:58:06 UTC
A commit references this bug:

Author: kai
Date: Sun Apr 28 23:57:23 UTC 2019
New revision: 500376
URL: https://svnweb.freebsd.org/changeset/ports/500376

Log:
  databases/clickhouse: Update to 19.5.3.8

  * Clickhouse requires now a compiler that understands C++17.

  While I'm here:

  * Set the TEST option as non-default and convert all runtime dependencies of
    that option to default ones. The former description was somewhat
    misleading because the TEST option only installed two additional Python
    scripts.

    The test query datasets in %%DATADIR%%-test/ and the previously mentioned
    Python scripts were always installed regardless if TEST was set/unset
    because they are part of the normal release installation. This also would
    lead to build failures if the TEST option was disabled. [1]

  * Use the TEST option for compiling and testing of additional tests by
    enabling -DENABLE_TESTS (instead of introducing a new option with another
    name for it). [1]

  * Also mark the TEST option as broken when building on FreeBSD 13.0-CURRENT
    with Clang 8 because it segfaults with an assertion error in function
    "getZExtValue()".

  * Pet portlint a bit by placing some variables to their intended locations.

  Changelog:

  https://github.com/yandex/ClickHouse/blob/v19.5.3.8-stable/CHANGELOG.md

  PR:		237318
  Submitted by:	proler@gmail.com (maintainer)
  Approved by:	maintainer [1], mentors (implicit)

Changes:
  head/databases/clickhouse/Makefile
  head/databases/clickhouse/distinfo
  head/databases/clickhouse/pkg-plist
Comment 10 Kai Knoblich freebsd_committer 2019-04-29 00:00:35 UTC
Committed, thank you for the patches and info!