Bug 250824

Summary: databases/postgresql13-server: incorrect checking of llvm versions
Product: Ports & Packages Reporter: Dmitry Marakasov <amdmi3>
Component: Individual Port(s)Assignee: pgsql
Status: Open ---    
Severity: Affects Many People CC: brooks, decke, girgen, olivier.freebsd
Priority: --- Flags: bugzilla: maintainer-feedback? (pgsql)
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Fix version detection and dependency none

Description Dmitry Marakasov freebsd_committer 2020-11-03 01:39:47 UTC
The conditions to check LLVM version, e.g.

.if ${COMPILER_VERSION} > ${LLVM_DEFAULT}

and

.  if ${COMPILER_VERSION} <= 11

are not correct, because llvm version is not monotonic (60, 70, 80, 90, 10, 11)

Imo we should rename e.g. llvm10 to llvm100 to fix this properly and not overcomplicate logic here and in other places.
Comment 1 Palle Girgensohn freebsd_committer 2020-11-10 21:18:39 UTC
@brooks, what do you think? It would be very handy if version numbers for llvm where in fact matching what compiler.mk sets for clang.
Comment 2 Olivier Certner 2021-01-21 17:16:44 UTC
Created attachment 221794 [details]
Fix version detection and dependency

I stumbled on this as well. Please see the attached patch.

I'm not completely sure of what was exactly intended in the original code. The test on ${COMPILER_VERSION}, in particular, led me to think that using the base compiler had been considered as a possibility. However, without further patching of the configure part, this cannot work since there is no `llvm-config` in base (and LLVM in base may lack required components anyway, I don't know).

Apart from that, changes are pretty straightforward. They deal with non-monotonic versions. The algorithm is simply to try to use LLVM_DEFAULT, provided it is not greater than MAX_ALLOWED_MAJOR (set to 11, as before), in which case that one is used instead.

I agree that part of it (e.g., determination of major LLVM version) should ideally go into some infrastructure *.mk file.
Comment 3 Bernhard Froehlich freebsd_committer 2021-03-04 10:56:20 UTC
Can we make some progress on this please? On 13.0 we also install llvm11 though we have the same llvm version in base - this looks at least questionable.
Comment 4 Palle Girgensohn freebsd_committer 2021-03-04 14:35:38 UTC
I'm pretty sure that llvm is not installed in full on FreeBSD-12. As Olivier writes, required components are missing AFAIK. Hence, we need to install llvm from ports anyway.

I'm planning to bump a machine to FreeBSD-13 and get poudriere running there, so I can verify alls this properly. Missing llvm-config could be worked around, but that is probably not all that's missing.

Anyway, I guess the patch could be applied right now and would mitigate against installing a newer version of clang from ports, right?

Palle