Bug 253749

Summary: devel/shiboken2: picks up wrong llvm, registers wrong dependency, causes unnecessary rebuilds
Product: Ports & Packages Reporter: Christoph Moench-Tegeder <cmt>
Component: Individual Port(s)Assignee: Christoph Moench-Tegeder <cmt>
Status: Closed FIXED    
Severity: Affects Some People CC: cmt, lbartoletti
Priority: --- Keywords: easy, patch, patch-ready
Version: LatestFlags: lbartoletti: maintainer-feedback+
Hardware: Any   
OS: Any   
See Also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=241490
Attachments:
Description Flags
fix llvm detection
none
fix llvm detection and nail dependency in place none

Description Christoph Moench-Tegeder freebsd_committer freebsd_triage 2021-02-21 21:16:49 UTC
Created attachment 222707 [details]
fix llvm detection

shiboken2 depends (per Makefile) on the "default" llvm port:

: LIB_DEPENDS=    libclang.so:devel/llvm${LLVM_DEFAULT}

but the package shows:

: $ pkg info --dependencies py37-shiboken2 | grep llvm
:        llvm10-10.0.1_5

and every poudriere run rebuilds py37-shiboken and it's downstreams (pyside2, FreeCAD) with the remark  (paraphrasing) "new dependency: devel/llvm90"

The build log shows that shiboken2 recently started picking up llvm10 instead of the desired llvm90:

: -- LLVM_CONFIG:             /usr/local/bin/llvm-config10
: -- CLANG: /usr/local/llvm10, /usr/local/llvm10/lib/libclang.so detected by /usr/local/bin/llvm-config10

and that dependency is registered in the final package

After some investigation, I found that shiboken2 picks up the highest versioned llvm-config it can find - and until recently, the build environment only had llvm90 (unless you're building in a dirty system, but then all bets are off anyways, right?), but ports r566177 made qt5-gui depend on mesa-dri, which in turn pulls in llvm10.
Our Makefile in devel/shiboken2 sets LLVM_CONFIG as CONFIGURE_ENV:

: CONFIGURE_ENV+= LLVM_CONFIG=${LOCALBASE}/bin/llvm-config${LLVM_DEFAULT}

but shiboken2's build system uses the cmake variable LLVM_CONFIG, not the environment variable of the same name - see https://code.qt.io/cgit/pyside/pyside-setup.git/tree/sources/shiboken2/data/shiboken_helpers.cmake?h=5.15.2#n141 for reference (it looks like I made this mistake when creating the port - and until recently, it didn't matter much).

The most obvious fix is to pass LLVM_CONFIG via cmake, not environment - see attached patch ("works for me and my poudriere"). Patch bumps PORTREVISION for changed/fixed dependency.
Comment 1 Christoph Moench-Tegeder freebsd_committer freebsd_triage 2021-02-21 23:33:36 UTC
Created attachment 222709 [details]
fix llvm detection and nail dependency in place

Turns out I was too quick on this one: pkg also does it's best to sabotage our efforts and registers the dependency on the highest versioned libclang.so available, not on the libclang.so we're actually linking to (and there I was, only poking at the binaries...) or let alone the package origin we specified in the Makefile.
To fix that, we also have to do a little dance with LLVM_DEFAULT the get the correct libclang.so. AS LLVM_DEFAULT is "90" for llvm 9.0 and "10" for llvm 10.0 (etc, note single digit vs two digits) I used a regexp which will work for llvm versions between 6 and 59 (I hope that I don't have to care about this anymore when llvm hits 60).
Comment 2 Loïc Bartoletti freebsd_committer freebsd_triage 2021-02-23 18:56:01 UTC
Good catch. LGTM.
Comment 3 commit-hook freebsd_committer freebsd_triage 2021-02-23 19:36:29 UTC
A commit references this bug:

Author: cmt
Date: Tue Feb 23 19:35:32 UTC 2021
New revision: 566424
URL: https://svnweb.freebsd.org/changeset/ports/566424

Log:
  devel/shiboken2: depend on correct llvm port

  between shiboken2's build system picking the highest versioned
  llvm-config available and pkg registering a dependency on the
  highest versioned libclang.so available, we need to
  - pass the correct llvm-config via cmake variable and not via
    environment, as the environment variable is not used by
    shiboken's build system
  - specify the dependency on libclang.so with it's version number; and
    to avoid breaking (or requiring manual intervention) with the next
    LLVM_DEFAULT switch, extract that version number from LLVM_DEFAULT
    in a way that should be working for llvm versions from 6 to 59
    (unless we choose to change our llvm port naming scheme), which
    seems to give us some safety margin.

  PR:		253749
  Approved by:	lbartoletti (kde@)

Changes:
  head/devel/shiboken2/Makefile
Comment 4 Christoph Moench-Tegeder freebsd_committer freebsd_triage 2021-02-23 19:37:07 UTC
committed ports r566424