View Issue Details

IDProjectCategoryView StatusLast Update
0000473Cinelerra-GG[All Projects] Bugpublic2020-08-14 02:01
Reporterferdnyc Assigned Togoodguy  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version 
Target VersionFixed in Version2020-07 
Summary0000473: [PATCH] Faulty mutual-exclusion logic for lv2 dependencies in configure.ac
DescriptionThe mutual-exclusion logic that attempts to force building of lv2 dependencies when any of the dependent libs
are missing is, as it turns out, ALWAYS forcing the dependencies to be build, even if they are all present on the system.

(This is when using --disable-static-build, but not --without-thirdparty.)
Steps To ReproduceOn a system that has the following packages installed and available:

$ pkg-config --libs serd-0
-lserd-0
$ pkg-config --libs sord-0
-lsord-0 -lserd-0
$ pkg-config --libs sratom-0
-lsratom-0 -lsord-0 -lserd-0
$ pkg-config --libs lilv-0
-llilv-0 -ldl -lsratom-0 -lsord-0 -lserd-0
$ pkg-config --libs suil-0
-lsuil-0

Configure cinelerra-gg as:

$ ./configure ... --enable-lv2=shared --enable-lilv=shared --enable-serd=shared --enable-sord=shared \
--enable-sratom=shared --enable-suil=shared --with-lv2 --disable-static-build ...

This results in:

  needed lilv
  needed sratom
  needed serd
  needed sord
  needed suil

even though HAVE_lv2 is set to 'yes' and lv2 itself is not built.
Additional InformationThe build can be corrected by removing the following section of configure.ac (patch attached) and regenerating configure:

# have all or none
test "x$HAVE_lv2" = "xyes" && \
test "x$HAVE_lilv" = "xyes" && \
test "x$HAVE_sord" = "xyes" && \
test "x$HAVE_serd" = "xyes" && \
test "x$HAVE_sratom" = "xyes" && \
test "x$HAVE_suil" = "xyes" && \
  HAVE_LV2=yes || \
HAVE_lv2=no && \
HAVE_lilv=no && \
HAVE_sord=no && \
HAVE_serd=no && \
HAVE_sratom=no && \
HAVE_suil=no && \
  HAVE_LV2=no

TBH I'm not sure why that doesn't work. It may simply be that it executes at the wrong point in the configure process.
I only know that after removing it, the configure invocation above now produces:

  shared -llilv-0
  shared -lsratom-0
  shared -lserd-0
  shared -lsord-0
  shared -lsuil-0

And none of the dependent source trees:
  thirdparty/lilv-0.24.6
  thirdparty/serd-0.30.2
  thirdparty/sord-0.16.4
  thirdparty/sratom-0.6.4
  thirdparty-suil-0.10.6
are compiled or linked. Instead the system's shared libs are used, as intended.
TagsNo tags attached.

Activities

PhyllisSmith

PhyllisSmith

2020-08-14 02:01

manager   ~0003918

Applied; will close in a couple of days.
ferdnyc

ferdnyc

2020-07-09 07:45

reporter   ~0003780

Sounds great, thanks! There must be nearly a dozen equally valid, GOOD ways to write the same logic — plus a couple dozen more that would technically work just as well, but also bring shame upon author's descendants lasting three generations. I'm good with any (of the former).

Shell precedence is a pain in the ass. I have to look it up Every. Single. Time. And the difficulties in mixing precedence/associativity with boolean logic are the entire reason Perl has BOTH && / || logic operators _plus_ 'and' and 'or' keywords — the difference being that 'and' and 'or' are at the absolute bottom of the precedence table. Which frees you to write lines of Perl like:

<literally anything> or close(fd) && die("WE GET ERROR!");

and there is no possible code you could put on the left-hand side that would change the meaning of that line. (Well, unless it contained another 'or'.)

...Of course, Perl (and Python, and C/C++) also make OR slightly lower precedence than AND, which is lower precedence than NOT, all of which is generally what you'd expect/intuit. The shell's "Of these list operators, && and || have equal precedence, followed by ; and &, which have equal precedence." is its own special kind of stupid.
PhyllisSmith

PhyllisSmith

2020-07-09 04:08

manager   ~0003776

Thanks for the education and patch. Especially helpful was note #3757 as gg used this as a test and was under the impression that the precedence worked differently. He has locally corrected configure.ac based on your patch suggestion, but a little differently. It has not been checked into GIT yet.
ferdnyc

ferdnyc

2020-07-08 00:19

reporter   ~0003759

Here's an updated patch. It fixes all of the mutual-exclusion sections, by replacing e.g.:

# have all or none
test "x$HAVE_libavc1394" = "xyes" && \
test "x$HAVE_libiec61883" = "xyes" && \
test "x$HAVE_libraw1394" = "xyes" && \
  HAVE_FIREWIRE=yes || \
HAVE_libavc1394=no && \
HAVE_libiec61883=no && \
HAVE_libraw1394=no && \
 HAVE_FIREWIRE=no


with:

# have all or none
test "x$HAVE_libavc1394" = "xyes" && \
test "x$HAVE_libiec61883" = "xyes" && \
test "x$HAVE_libraw1394" = "xyes" && \
  HAVE_FIREWIRE=yes || \
( \
  HAVE_libavc1394=no && \
  HAVE_libiec61883=no && \
  HAVE_libraw1394=no && \
  HAVE_FIREWIRE=no \
)


(It also inserts a missing line-continuation character in the mutual-exclusion code for OpenEXR.)

0001-Fix-mutual-exclusion-logic.patch (2,267 bytes)
From 219896331c151b147b405f27570ee5521709e2d1 Mon Sep 17 00:00:00 2001
From: "FeRD (Frank Dana)" <[email protected]>
Date: Tue, 7 Jul 2020 20:07:30 -0400
Subject: [PATCH] Fix mutual-exclusion logic

---
 cinelerra-5.1/configure.ac | 42 +++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/cinelerra-5.1/configure.ac b/cinelerra-5.1/configure.ac
index daa267f0..7a2dc0db 100644
--- a/cinelerra-5.1/configure.ac
+++ b/cinelerra-5.1/configure.ac
@@ -673,10 +673,12 @@ test "x$HAVE_libavc1394" = "xyes" && \
 test "x$HAVE_libiec61883" = "xyes" && \
 test "x$HAVE_libraw1394" = "xyes" && \
   HAVE_FIREWIRE=yes || \
-HAVE_libavc1394=no && \
-HAVE_libiec61883=no && \
-HAVE_libraw1394=no && \
- HAVE_FIREWIRE=no
+( \
+  HAVE_libavc1394=no && \
+  HAVE_libiec61883=no && \
+  HAVE_libraw1394=no && \
+  HAVE_FIREWIRE=no \
+)
 
 CHECK_WANT([DV], [auto], [use dv], [
  CHECK_LIB([libdv], [dv], [dv_init])
@@ -700,9 +702,11 @@ CHECK_WANT([ESOUND], [no], [use esd], [
 test "x$HAVE_esound" = "xyes" && \
 test "x$HAVE_audiofile" = "xyes" && \
   HAVE_ESOUND=yes || \
-HAVE_esound=no && \
-HAVE_audiofile=no && \
-  HAVE_ESOUND=no
+( \
+  HAVE_esound=no && \
+  HAVE_audiofile=no && \
+  HAVE_ESOUND=no \
+)
 
 CHECK_WANT([PULSE], [auto], [use pulseaudio], [
  CHECK_LIB([pulse_simple], [pulse-simple], [pa_simple_new])
@@ -749,13 +753,15 @@ test "x$HAVE_serd" = "xyes" && \
 test "x$HAVE_sratom" = "xyes" && \
 test "x$HAVE_suil" = "xyes" && \
   HAVE_LV2=yes || \
-HAVE_lv2=no && \
-HAVE_lilv=no && \
-HAVE_sord=no && \
-HAVE_serd=no && \
-HAVE_sratom=no && \
-HAVE_suil=no && \
-  HAVE_LV2=no
+( \
+  HAVE_lv2=no && \
+  HAVE_lilv=no && \
+  HAVE_sord=no && \
+  HAVE_serd=no && \
+  HAVE_sratom=no && \
+  HAVE_suil=no && \
+  HAVE_LV2=no \
+)
 
 CHECK_WANT([CUDA], [auto], [build cuda plugins], [
   CHECK_HEADERS([CUDA], [cuda sdk], [${CUDA_PATH:-/usr/local/cuda}/include/cuda.h])])
@@ -813,9 +819,11 @@ AC_SUBST([HAVE_OPENEXR])
 test "x$HAVE_openexr" = "xyes" && \
 test "x$HAVE_ilmbase" = "xyes" && \
   HAVE_OPENEXR=yes || \
-HAVE_openexr=no && \
-HAVE_ilmbase=no &&
-  HAVE_OPENEXR=no
+( \
+  HAVE_openexr=no && \
+  HAVE_ilmbase=no && \
+  HAVE_OPENEXR=no \
+)
 
 # build global_config
 OBJDIR=`uname -m`
-- 
2.26.2

PhyllisSmith

PhyllisSmith

2020-07-08 00:17

manager   ~0003758

OK, downloaded the patch and got the update. GG will look at this yet today.
ferdnyc

ferdnyc

2020-07-08 00:00

reporter   ~0003757

That's a lie, I do know why it doesn't work: Precedence issues.

$ test 1 && test 1 && test 1 && test 1 && test 1 && echo -n "yes " || echo -n "no1 " && echo -n "no2 " && echo -n "no3"; echo
yes no2 no3

The intention of the code is that nothing after the || should execute if all of the preceding statements test true. But since && and || have equal precedence in bash, that's not how it actually happens.

Parentheses would fix it:
$ test 1 && test 1 && test 1 && test 1 && test 1 && echo -n "yes " || (echo -n "no1 " && echo -n "no2 " && echo -n "no3"); echo
yes
ferdnyc

ferdnyc

2020-07-07 23:22

reporter  

0001-Fix-lv2-dep-logic.patch (1,173 bytes)
From 2eaebaa4151529fef14c72b2e724e54b8f74ae23 Mon Sep 17 00:00:00 2001
From: "FeRD (Frank Dana)" <[email protected]>
Date: Tue, 7 Jul 2020 18:39:32 -0400
Subject: [PATCH] Fix lv2 dep logic

---
 cinelerra-5.1/configure.ac | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/cinelerra-5.1/configure.ac b/cinelerra-5.1/configure.ac
index daa267f..4350127 100644
--- a/cinelerra-5.1/configure.ac
+++ b/cinelerra-5.1/configure.ac
@@ -741,21 +741,6 @@ CHECK_WANT([LV2], [auto], [use lv2], [
  CFLAGS="-I/usr/include/suil-0 -I/usr/local/include/suil-0"
  CHECK_HEADERS([lv2], [suil headers], [suil/suil.h])
  CFLAGS="$saved_CFLAGS"])
-# have all or none
-test "x$HAVE_lv2" = "xyes" && \
-test "x$HAVE_lilv" = "xyes" && \
-test "x$HAVE_sord" = "xyes" && \
-test "x$HAVE_serd" = "xyes" && \
-test "x$HAVE_sratom" = "xyes" && \
-test "x$HAVE_suil" = "xyes" && \
-  HAVE_LV2=yes || \
-HAVE_lv2=no && \
-HAVE_lilv=no && \
-HAVE_sord=no && \
-HAVE_serd=no && \
-HAVE_sratom=no && \
-HAVE_suil=no && \
-  HAVE_LV2=no
 
 CHECK_WANT([CUDA], [auto], [build cuda plugins], [
   CHECK_HEADERS([CUDA], [cuda sdk], [${CUDA_PATH:-/usr/local/cuda}/include/cuda.h])])
-- 
2.26.2

Issue History

Date Modified Username Field Change
2020-07-07 23:22 ferdnyc New Issue
2020-07-07 23:22 ferdnyc File Added: 0001-Fix-lv2-dep-logic.patch
2020-07-08 00:00 ferdnyc Note Added: 0003757
2020-07-08 00:17 PhyllisSmith Note Added: 0003758
2020-07-08 00:17 PhyllisSmith Assigned To => PhyllisSmith
2020-07-08 00:17 PhyllisSmith Status new => assigned
2020-07-08 00:17 PhyllisSmith Assigned To PhyllisSmith => goodguy
2020-07-08 00:19 ferdnyc File Added: 0001-Fix-mutual-exclusion-logic.patch
2020-07-08 00:19 ferdnyc Note Added: 0003759
2020-07-09 04:08 PhyllisSmith Note Added: 0003776
2020-07-09 07:45 ferdnyc Note Added: 0003780
2020-08-14 02:01 PhyllisSmith Status assigned => resolved
2020-08-14 02:01 PhyllisSmith Resolution open => fixed
2020-08-14 02:01 PhyllisSmith Fixed in Version => 2020-07
2020-08-14 02:01 PhyllisSmith Note Added: 0003918