Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JUCE LV2 won't build in unattended/non-XWindows pipeline builds #4030

Closed
baconpaul opened this issue Mar 5, 2021 · 76 comments
Closed

JUCE LV2 won't build in unattended/non-XWindows pipeline builds #4030

baconpaul opened this issue Mar 5, 2021 · 76 comments
Labels
JUCE Ex Machina Issue that will likely be fixed by porting Surge to JUCE UI/plugin back-end
Milestone

Comments

@baconpaul
Copy link
Collaborator

Moving conversation with @KottV over here from DISTRHO/JUCE#14 (comment)

replace libs/juce-6.0.7/juce with that fork https://github.com/lv2-porting-project/JUCE/tree/lv2
change from line 1493 in https://github.com/surge-synthesizer/surge/blob/main/CMakeLists.txt#L1493
URI can be any different of course:

        FORMATS AU VST3 Standalone LV2
        LV2_URI https://surge-synthesizer.github.io/lv2/surge_xt

configure:

cmake -Bbuildxt -DBUILD_SURGE_JUCE_PLUGINS=True -DLV2_TTL_GENERATOR=/usr/bin/lv2_ttl_generator
make

cmake --build buildxt --config Debug --target surge-xt_LV2 -j8
/usr/bin/lv2_ttl_generator is from https://github.com/lv2-porting-project/lv2-ttl-generator you might replace it with your own

as it fails to generate .ttl the cmake will delete Surge XT.so
so it's worth to comment generation part https://github.com/lv2-porting-project/JUCE/blob/lv2/extras/Build/CMake/JUCEUtils.cmake#L1325 from L1325 to L1339 for testing

@baconpaul baconpaul added LV2 JUCE Ex Machina Issue that will likely be fixed by porting Surge to JUCE UI/plugin back-end labels Mar 5, 2021
@baconpaul baconpaul added this to the Surge XT 1.0 milestone Mar 5, 2021
@baconpaul
Copy link
Collaborator Author

Hello @KottV

We've started our work on XT-Alpha (we release 1.9 tomorrow which will be our last non-JUCE synth). You can find a version of surge which builds and runs AU VST3 Standalone and (if you have the SDK) VST2 on the xt-alpha branch here

https://github.com/surge-synthesizer/surge/tree/xt-alpha

By default it will use the juce in the submodule (which is juce 6.0.7) but the cmake file has a variable SURGE_ALTERNATE_JUCE which lets you point at a different juce.

Would you be willing to work with us to make mergeable changes and write directions on how to build an LV2? xt won't release until summer and it is still alpha, but knowing we have LV2 under control early on and having directions so we can test would be great. I really don't know how to do or test it.

Thanks!

@KottV
Copy link
Contributor

KottV commented Apr 21, 2021

Hi!

Sure I will. And I think it's worth to cast the real programmers: @jatinchowdhury18 @falkTX

@baconpaul
Copy link
Collaborator Author

Yeah @jatinchowdhury18 has a bunch of stuff in 19 and we’ve discussed how he gets lv2 from juce on our discord. Just I saw you offer up a build to the disco guys on that kvr thread today and thought I’d tag you in case you had time to join in the fun here too!

I think it is a pretty easy change

oh and if there’s bugs in the lv2 compare with standalone or vst3. The juce port is still rather alpha

@falkTX
Copy link
Contributor

falkTX commented Apr 21, 2021

oh and if there’s bugs in the lv2 compare with standalone or vst3. The juce port is still rather alpha

huh, says who? afaik all the issues were fixed.

for vitalium release, the lv2 is the version that works the best actually.
linux-vst3 support is still subpar, though that has been improving.

@KottV
Copy link
Contributor

KottV commented Apr 21, 2021

oh and if there’s bugs in the lv2 compare with standalone or vst3. The juce port is still rather alpha

huh, says who? afaik all the issues were fixed.

for vitalium release, the lv2 is the version that works the best actually.
linux-vst3 support is still subpar, though that has been improving.

I think the surge juce port is still alpha )

@falkTX
Copy link
Contributor

falkTX commented Apr 21, 2021

ah lol, the surge juce port. not lv2 juce port.
I should sleep... :P

@baconpaul
Copy link
Collaborator Author

lol! yeah surge xt is super alpha. its full of bugs even on mac au and standalone. I meant 'if you build the lv2 and it doesn't work, compare with the standalone to make sure that the bug is uniform in surge'

@KottV
Copy link
Contributor

KottV commented Apr 21, 2021

alright...

  1. LV2 wrapper wants updates for some new JUCE functions:
/home/kv/src/audio/surge/libs/JUCE-lv2/modules/juce_audio_plugin_client/LV2/juce_LV2_Wrapper.cpp:901:75: error: ‘virtual int juce::AudioProcessor::getNumParameters()’ is deprecated [-Werror=deprecated-declarations]
  901 |         portControls.insertMultiple (0, nullptr, filter->getNumParameters());
      |                                                                           ^
/home/kv/src/audio/surge/libs/JUCE-lv2/modules/juce_audio_plugin_client/LV2/juce_LV2_Wrapper_Exporter.cpp:322:48: error: ‘virtual int juce::AudioProcessor::getNumParameters()’ is deprecated [-Werror=deprecated-declarations]
  322 |     for (int i=0; i < filter->getNumParameters(); ++i)
      |                                                ^

it's not a big case, passed it by adding -Wno-error=deprecated-declarations for now

  1. LV2 needs surge extensions too, right?
[100%] Linking CXX shared module "surge-xt_artefacts/Release/LV2/Surge XT.lv2/Surge XT.so"
/usr/lib64/gcc/x86_64-suse-linux/10/../../../../x86_64-suse-linux/bin/ld: surge-xt_artefacts/Release/libSurge XT_SharedCode.a(SurgeSynthProcessor.cpp.o): в функции «SurgeSynthProcessor::SurgeSynthProcessor()»:
SurgeSynthProcessor.cpp:(.text+0x2f90): неопределённая ссылка на «SurgeSynthProcessorSpecificExtensions(SurgeSynthProcessor*, SurgeSynthesizer*)»
/usr/lib64/gcc/x86_64-suse-linux/10/../../../../x86_64-suse-linux/bin/ld: surge-xt_artefacts/Release/libSurge XT_SharedCode.a(SurgeSynthEditor.cpp.o): в функции «SurgeSynthEditor::SurgeSynthEditor(SurgeSynthProcessor&)»:
SurgeSynthEditor.cpp:(.text+0xa44): неопределённая ссылка на «SurgeSynthEditorSpecificExtensions(SurgeSynthEditor*, SurgeGUIEditor*)»
collect2: error: ld returned 1 exit status

passed by adding


  target_sources(surge-xt_LV2 PRIVATE
          src/surge_synth_juce/SurgeSynthStandaloneExtensions.cpp
          )

just for compilation, maybe it's the reason of the next error

  1. And I dont know what is happening here:
lv2_ttl_generator ./build/surge-xt_artefacts/Release/LV2/Surge\ XT.lv2/Surge\ XT.so
Generate ttl data for './build/surge-xt_artefacts/Release/LV2/Surge XT.lv2/Surge XT.so', basename: 'Surge XT'
SurgeXT : Version 1.0.xt-alpha.2e3703c8
Segmentation fault 
Starting program: /home/kv/bin/lv2_ttl_generator ./build/surge-xt_artefacts/Release/LV2/Surge\ XT.lv2/Surge\ XT.so
Downloading separate debug info for /media/src/src/audio/surge/system-supplied DSO at 0x7ffff7fca000...
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Generate ttl data for './build/surge-xt_artefacts/Release/LV2/Surge XT.lv2/Surge XT.so', basename: 'Surge XT'
SurgeXT : Version 1.0.xt-alpha.2e3703c8

Program received signal SIGSEGV, Segmentation fault.
__strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:65
65              VPCMPEQ (%rdi), %ymm0, %ymm1
(gdb) bt
#0  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:65
#1  0x00007ffff63de81a in SurgeSynthProcessor::SurgeSynthProcessor() () from ./build/surge-xt_artefacts/Release/LV2/Surge XT.lv2/Surge XT.so
#2  0x00007ffff63ded4c in createPluginFilter() () from ./build/surge-xt_artefacts/Release/LV2/Surge XT.lv2/Surge XT.so
#3  0x00007ffff63d161b in createLv2Files(char const*) () from ./build/surge-xt_artefacts/Release/LV2/Surge XT.lv2/Surge XT.so
#4  0x0000000000401348 in main ()
(gdb)

@baconpaul
Copy link
Collaborator Author

  1. I'd be fine merging that warning suppression if we have to
  2. Oh that extensions thing - yeah we need one for LV2. Your fix is fine and i would merge it but you can also just copy standalone to a new file called LV2. That's not why it is crashing though.
  3. Do you have a line number in SurgeSynthProcessor or can you run the trace up to 1 so I can see where it is? In theory surge will run fine now without any installed assets but i could have gotten that wrong.

@KottV
Copy link
Contributor

KottV commented Apr 21, 2021

Do you have a line number in SurgeSynthProcessor or can you run the trace up to 1 so I can see where it is?

Is it means the Debug build? How can I do this?

@baconpaul
Copy link
Collaborator Author

Oh sure. For a debug build

cmake -Bbuilddbg -DCMAKE_BUILD_TYPE=Debug
cmake --build builddbg --config Debug --target surge-staged-assets

@KottV
Copy link
Contributor

KottV commented Apr 21, 2021

thanks, will try
I'll provide the steps, need to fix some stuff in cmake with lv2_ttl_generator

@baconpaul
Copy link
Collaborator Author

OK cool And thanks for jumping in - welcome to team surge! :)

@jatinchowdhury18
Copy link
Collaborator

In my fork, instead of filter->getNumParameters(), I use filter->getParameters().size(), which fixes the depracation warning.

@KottV
Copy link
Contributor

KottV commented Apr 21, 2021

bt from Debug build:
lv2_ttl_generator ./builddbg/surge-xt_artefacts/Debug/LV2/Surge\ XT.lv2/Surge\ XT.so

Reading symbols from lv2_ttl_generator...
(gdb) run
Starting program: /home/kv/bin/lv2_ttl_generator ./builddbg/surge-xt_artefacts/Debug/LV2/Surge\ XT.lv2/Surge\ XT.so
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
JUCE v6.0.8
Generate ttl data for './builddbg/surge-xt_artefacts/Debug/LV2/Surge XT.lv2/Surge XT.so', basename: 'Surge XT'
SurgeXT : Version 1.0.xt-alpha.2e3703c8
JUCE Assertion failure in juce_AudioProcessor.cpp:1203

Program received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff7a807ab in kill () at ../sysdeps/unix/syscall-template.S:120
120     T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
(gdb) bt
#0  0x00007ffff7a807ab in kill () at ../sysdeps/unix/syscall-template.S:120
#1  0x00007ffff5a69375 in juce::AudioProcessor::getWrapperTypeDescription (type=juce::AudioProcessor::wrapperType_LV2) at /home/kv/src/audio/surge/libs/JUCE-lv2/modules/juce_audio_processors/processors/juce_AudioProcessor.cpp:1203
#2  0x00007ffff5f597ca in SurgeSynthProcessor::SurgeSynthProcessor (this=0x446fc0) at /home/kv/src/audio/surge/src/surge_synth_juce/SurgeSynthProcessor.cpp:78
#3  0x00007ffff5f5b3a3 in createPluginFilter () at /home/kv/src/audio/surge/src/surge_synth_juce/SurgeSynthProcessor.cpp:345
#4  0x00007ffff5f4f41f in juce::createPluginFilterOfType (type=juce::AudioProcessor::wrapperType_LV2) at /home/kv/src/audio/surge/libs/JUCE-lv2/modules/juce_audio_plugin_client/utility/juce_CreatePluginFilter.h:39
#5  0x00007ffff5f4df71 in createLv2Files (basename=0x7fffffffdd40 "Surge XT") at /home/kv/src/audio/surge/libs/JUCE-lv2/modules/juce_audio_plugin_client/LV2/juce_LV2_Wrapper_Exporter.cpp:465
#6  0x00007ffff5f4e3f9 in lv2_generate_ttl (basename=0x7fffffffdd40 "Surge XT") at /home/kv/src/audio/surge/libs/JUCE-lv2/modules/juce_audio_plugin_client/LV2/juce_LV2_Wrapper_Exporter.cpp:500
#7  0x0000000000401348 in main ()

@KottV
Copy link
Contributor

KottV commented Apr 21, 2021

The steps:

  1. Clone JUCE with LV2 (Jatin's fixes is merged)
    git clone -b lv2 https://github.com/lv2-porting-project/JUCE libs/JUCE-lv2

As we have problems with ttl generator (and it got duplicate target under surge's cmake, do You have any thought?), I commented whole section in libs/JUCE-lv2/extras/Build/CMake/JUCEUtils.cmake from line 1375 to line 1387:

        # generate .ttl files
        if(LV2_TTL_GENERATOR)
#            add_custom_command(TARGET ${target_name} POST_BUILD
#                COMMAND ${LV2_TTL_GENERATOR} "./${product_name}.so"
#                WORKING_DIRECTORY "${products_folder}/${product_name}.lv2/"
#                VERBATIM)
#        else()
#        add_executable(lv2_ttl_generator ${JUCE_SOURCE_DIR}/extras/Build/lv2_ttl_generator/lv2_ttl_generator.c)
#        set_source_files_properties(${JUCE_SOURCE_DIR}/extras/Build/lv2_ttl_generator/lv2_ttl_generator.c PROPERTIES LANGUAGE CXX)
#        target_link_libraries(lv2_ttl_generator ${CMAKE_DL_LIBS})
#
#        add_custom_command(TARGET ${target_name} POST_BUILD
#            COMMAND lv2_ttl_generator "$<TARGET_FILE:${target_name}>"
#            COMMENT "Generating LV2 Turtle manifest files for ${target_name}"
#            WORKING_DIRECTORY "${products_folder}/${product_name}.lv2/"
#            VERBATIM)
        endif()
  1. Add in CMakeLists.txt line 644 (or 641 if You have VST2)
    set(SURGE_JUCE_FORMATS AU VST3 Standalone LV2)

And LV2_URI (I chose this one) in line 849
LV2_URI=https://surge-synthesizer.github.io/lv2

  1. Fix the MPE doesn't work correctly #2. from above, I added in CMakeLists.txt line 897
  target_sources(surge-xt_LV2 PRIVATE
          src/surge_synth_juce/SurgeSynthStandaloneExtensions.cpp
          )
  1. build
cmake -Bbuilddbg -DCMAKE_BUILD_TYPE=Debug -DSURGE_ALTERNATE_JUCE=libs/JUCE-lv2
cmake --build builddbg --config Debug --target surge-staged-assets -j <cpu num>
  1. build ttl generator
    gcc libs/JUCE-lv2/extras/Build/lv2_ttl_generator/lv2_ttl_generator.c -o lv2_ttl_generator -ldl

  2. run it
    gdb -ar lv2_ttl_generator ./builddbg/surge-xt_artefacts/Debug/LV2/Surge\ XT.lv2/Surge\ XT.so

@baconpaul
Copy link
Collaborator Author

so that crash looks like a crash in getWrapperTypeDescription when the type is LV2.

In Juce 6.0.7 it is implemented like this

const char* AudioProcessor::getWrapperTypeDescription (AudioProcessor::WrapperType type) noexcept
{
    switch (type)
    {
        case AudioProcessor::wrapperType_Undefined:     return "Undefined";
        case AudioProcessor::wrapperType_VST:           return "VST";
        case AudioProcessor::wrapperType_VST3:          return "VST3";
        case AudioProcessor::wrapperType_AudioUnit:     return "AU";
        case AudioProcessor::wrapperType_AudioUnitv3:   return "AUv3";
        case AudioProcessor::wrapperType_RTAS:          return "RTAS";
        case AudioProcessor::wrapperType_AAX:           return "AAX";
        case AudioProcessor::wrapperType_Standalone:    return "Standalone";
        case AudioProcessor::wrapperType_Unity:         return "Unity";
        default:                                        jassertfalse; return {};
    }
}

and your branch is the same. You need to add an LV2 case to that. (So before the default add

case AudioProcessor::wrapperType_LV2: return "LV2";

)

and then merge it and the core dump will go away.

@baconpaul
Copy link
Collaborator Author

in ./modules/juce_audio_processors/processors/juce_AudioProcessor.cpp that is

@baconpaul
Copy link
Collaborator Author

Oh also I'll try and merge some of these steps into the xt-alpha branch when i'm next doing dev on it today in a way which has a flag (-DJUCE_HAS_LV2_SUPPORT=True or some such) today. I'll update here when I do.

@KottV
Copy link
Contributor

KottV commented Apr 21, 2021

so that crash looks like a crash in getWrapperTypeDescription when the type is LV2.

In Juce 6.0.7 it is implemented like this

and your branch is the same. You need to add an LV2 case to that. (So before the default add

case AudioProcessor::wrapperType_LV2: return "LV2";

)

and then merge it and the core dump will go away.

excellent!

wow... 2137 presets....

@baconpaul
Copy link
Collaborator Author

The reason you get the dup on lv2_ttl_generator is because surge makes two plugins in the cmake file and you add the target with the wrong scope. I fixed this by prepending the target name to the lv2 generator target. Also surge-xt didn't have C as a language so it didn't work but I have fixed that.

@baconpaul
Copy link
Collaborator Author

Wahey! Got it to all build with internal ttl generator

Will merge the surge diffs in a couple but required some diffs to your branch also. Will attach them here in a second.

@KottV
Copy link
Contributor

KottV commented Apr 21, 2021

my LV_URI code looks broken, i've made a dirty workaround, also it doesn't like the space in "Surge XT" so I named it Surge_XT for now
anyway, it starts and makes a sound!

surgext
изображение

@baconpaul
Copy link
Collaborator Author

baconpaul commented Apr 21, 2021

Here's the patches I had to install to your branch:

diff --git a/extras/Build/CMake/JUCEUtils.cmake b/extras/Build/CMake/JUCEUtils.cmake
index d6cc66f86..dd25d4441 100644
--- a/extras/Build/CMake/JUCEUtils.cmake
+++ b/extras/Build/CMake/JUCEUtils.cmake
@@ -1376,10 +1376,10 @@ function(_juce_set_plugin_target_properties shared_code_target kind)
                 WORKING_DIRECTORY "${products_folder}/${product_name}.lv2/"
                 VERBATIM)
         else()
-            add_executable(lv2_ttl_generator ${JUCE_SOURCE_DIR}/extras/Build/lv2_ttl_generator/lv2_ttl_generator.c)
-            target_link_libraries(lv2_ttl_generator dl)
+            add_executable(${target_name}_lv2_ttl_generator ${JUCE_SOURCE_DIR}/extras/Build/lv2_ttl_generator/lv2_ttl_generator.c)
+            target_link_libraries(${target_name}_lv2_ttl_generator dl)
             add_custom_command(TARGET ${target_name} POST_BUILD
-                COMMAND lv2_ttl_generator "${output_path}/${shared_code_target}.so"
+                COMMAND ${target_name}_lv2_ttl_generator "./${product_name}.so"
                 DEPENDS ${target_name} lv2_ttl_generator
                 WORKING_DIRECTORY "${products_folder}/${product_name}.lv2/"
                 VERBATIM)
diff --git a/modules/juce_audio_processors/processors/juce_AudioProcessor.cpp b/modules/juce_audio_processors/processors/juce_AudioProcessor.cpp
index 040accf2e..b1d45c751 100644
--- a/modules/juce_audio_processors/processors/juce_AudioProcessor.cpp
+++ b/modules/juce_audio_processors/processors/juce_AudioProcessor.cpp
@@ -1200,6 +1200,7 @@ const char* AudioProcessor::getWrapperTypeDescription (AudioProcessor::WrapperTy
         case AudioProcessor::wrapperType_AAX:           return "AAX";
         case AudioProcessor::wrapperType_Standalone:    return "Standalone";
         case AudioProcessor::wrapperType_Unity:         return "Unity";
+        case AudioProcessor::wrapperType_LV2:           return "LV2";
         default:                                        jassertfalse; return {};
     }
 }

let me know when you merge those. I'm going to merge some changes to the surge-xt branch now to work with them.

@baconpaul
Copy link
Collaborator Author

OK so once #4346 is merged (in about 20 minutes) there's a new prescription.

  1. Check out the juce lv2 and apply the above patches
  2. Check out surge on the xt-alpha branch
  3. Run cmake with the extra flag JUCE_SUPPORTS_LV2=TRUE and the SURGE_ALTERNATE_JUCE location to your checked out
cmake -Bignore/lv2 -DCMAKE_BUILD_TYPE=Debug -DJUCE_SUPPORTS_LV2=True -DSURGE_ALTERNATE_JUCE=/home/paul/dev/music/JUCE-lv2/
  1. Build it.
 cmake --build ignore/lv2 --config Debug --target surge-xt_LV2

Now, that exports all 2100 presets - we probably don't want to do that. But the prescription above sets all the flags and stuff so it builds, and skips an LV2 in regular juce mode.

So now: what's this about a space in a plugin name?

@baconpaul
Copy link
Collaborator Author

Screen Shot 2021-04-22 at 11 08 08 AM

Oh and here' the FX bank running in jalv.gkt3. That's not alpha software at all - that's the same code we released yesterday as 1.9.

baconpaul added a commit to baconpaul/surge that referenced this issue Apr 22, 2021
1. The XT FX Bank is "Surge XT Effects"
2. The LV2 URIs are correct
3. The LV2_SHARED_LIB thing per surge-synthesizer#4030 is in place although
   requires an upstream merge to be workable
baconpaul added a commit that referenced this issue Apr 22, 2021
1. The XT FX Bank is "Surge XT Effects"
2. The LV2 URIs are correct
3. The LV2_SHARED_LIB thing per #4030 is in place although
   requires an upstream merge to be workable
@KottV
Copy link
Contributor

KottV commented Apr 22, 2021

OK @KottV I have this all working.

Turns out replacing special characters in cmake at build time is a bit tricky so what I did was add another parameter which I called LV2_SHARED_LIBRARY_NAME. In Surge I set that to SurgeXT and it makes the manifest and so on skip the space at TTL generation time.

So that requires a tiny change to Surge (which I'll commit i a minute) and then for you to merge the change below to your branch. I also tested this with plugins that don't set the SHARED_LIBRARY_NAME and the rename falls back there to the harmless no-op.

If you can let me know when you've merged this I can do a fresh checkout and try again and, if that works, can update our developer doc and pipeline.

Good and simple solution. Merged.

@baconpaul
Copy link
Collaborator Author

Awesome thanks! Lemme check a build from head. I'll keep this open until i get it in CI and the README but it really does all seem to be working. Or at least starting to work.

@baconpaul
Copy link
Collaborator Author

Wahey! All works at head of both repos! Pleasure working with you on this.

@KottV
Copy link
Contributor

KottV commented Apr 23, 2021

great, really)

@baconpaul
Copy link
Collaborator Author

So @KottV we are almost done but not quite. The unattended pipeline build I ran crashed because the ttl generator tried to connect to a display. While I could spin up a fake x server on our build host, we should really debug this. So let me re-purpose this issue for that.

The problem, I think, is that juce_LV2_WRapper_Exporter.cpp at the top of createLv2Files creates a const ScopedJuceInitiaiser_GUI which sets up the GUI pipeline. It then goes on to use non of the GUI code, though.

If I comment it out it still works fine in my environment but I need to figure out how to apply that in the pipeline also to test.

@KottV
Copy link
Contributor

KottV commented Apr 23, 2021

May I take a look at the crash's output ?

My builds of LV2 JUCE in Open Build Server doesn't requires running X server. I guess it tries to dlopen such libs, perhaps.

@KottV
Copy link
Contributor

KottV commented Apr 23, 2021

Ok, I reproduced it. Weird...

@baconpaul
Copy link
Collaborator Author

OK cool.
I'm spinning up an Xvfb in our pipeline for now for LV2 but

[100%] Linking CXX shared module "surge-xt_artefacts/Debug/LV2/Surge XT.lv2/Surge XT.so"
JUCE v6.0.8
Generate ttl data for './SurgeXT.so', basename: 'SurgeXT'
SurgeXT : Version 1.0.HEAD.597056a
JUCE Assertion failure in juce_linux_XWindowSystem.cpp:1674
JUCE Assertion failure in juce_linux_XWindowSystem.cpp:1837
JUCE Assertion failure in juce_linux_XWindowSystem.cpp:1662
Segmentation fault (core dumped)

that's the error I see. Azure is running standard ubuntu-20.04 with up to date packages.

@KottV
Copy link
Contributor

KottV commented Apr 23, 2021

Is it possible that Surge XT has calls for X11 outside of JUCE?

@baconpaul
Copy link
Collaborator Author

Not that I know of no. But maybe there's some lingering goobits from the pre-juce port. I will scan.

If it helps, when I start an XVFB I get this

JUCE v6.0.8
Generate ttl data for './SurgeXT.so', basename: 'SurgeXT'
SurgeXT : Version 1.0.HEAD.6f8f260
X Error of failed request:  BadAtom (invalid Atom parameter)
  Major opcode of failed request:  18 (X_ChangeProperty)
  Atom id in failed request:  0x0
  Serial number of failed request:  135
  Current serial number in output stream:  136
*** Leaked objects detected: 1 instance(s) of class LinuxComponentPeer
JUCE Assertion failure in juce_LeakedObjectDetector.h:92

so here's what I'm gonna do

I'm going to disable but keep the code intact for our pipeline builds and merge our README change

then go onto other surge bugs for a bit

Right now users can build and run an LV2. Our pipeline builds can come later in the cycle (I mean, were in day 4 of a 5 month cycle or what not now so we have plenty of time). I'll leave this issue open but change the subject.

Sound good?

@baconpaul baconpaul changed the title Use non-JUCE-mainline JUCE to build LV2 of SFX and SXT JUCE LV2 won't build in unattended/non-XWindows pipeline builds Apr 23, 2021
@KottV
Copy link
Contributor

KottV commented Apr 23, 2021

Of course.

But let me notice once more that other JUCE plugins builds fine in such way. I'll try to figure out where is the case.

@baconpaul
Copy link
Collaborator Author

Yeah i still have some of the old pre-juce code in the codebase so it entirely could be that that is getting triggered and screwing us up. I'm ripping it out now.

100% confident we can get this fixed. But want to update that README now.

Also would be good if some folks can start testing this. I'll hit up @unfa and @selenologist to see what their experience with the XT plugin in Ardour.

@KottV
Copy link
Contributor

KottV commented Apr 23, 2021

I got it, the error from generating ttl for SurgeXT_FX.so led me out.

generator initializes the loading of presets , surge tries to find them in /usr/share/surge and other dirs, not found then throws the error window (I guess)
so I put the copying of resources before the build and passed further
but it's preferable to search presets in sourcetree/resources folder during build, providing CMAKE_SOURCE_DIR/resources in data search path, I think

@KottV
Copy link
Contributor

KottV commented Apr 23, 2021

also we need no cairo, xcb etc and all dependencies check in CMakeLists.txt, JUCE will take care of this:

  pkg_check_modules(CAIRO REQUIRED cairo)
  pkg_check_modules(FONTCONFIG REQUIRED fontconfig)
  pkg_check_modules(X11 REQUIRED x11)
  pkg_check_modules(XCB REQUIRED xcb)
  pkg_check_modules(XCBCURSOR REQUIRED xcb-cursor)
  pkg_check_modules(XCBKEYSYMS REQUIRED xcb-keysyms)
  pkg_check_modules(XCBUTIL REQUIRED xcb-util)
  pkg_check_modules(XCBXKB REQUIRED xcb-xkb)
  pkg_check_modules(XKBCOMMON REQUIRED xkbcommon)
  pkg_check_modules(XKBX11COMMON REQUIRED xkbcommon-x11)
  pkg_check_modules(CAIRO REQUIRED cairo)
  pkg_check_modules(FONTCONFIG REQUIRED fontconfig)
  pkg_check_modules(X11 REQUIRED x11)
  pkg_check_modules(XCB REQUIRED xcb)
  pkg_check_modules(XCBCURSOR REQUIRED xcb-cursor)
  pkg_check_modules(XCBKEYSYMS REQUIRED xcb-keysyms)
  pkg_check_modules(XCBUTIL REQUIRED xcb-util)
  pkg_check_modules(XCBXKB REQUIRED xcb-xkb)
  pkg_check_modules(XKBCOMMON REQUIRED xkbcommon)
  pkg_check_modules(XKBX11COMMON REQUIRED xkbcommon-x11)
  pkg_check_modules(CAIRO REQUIRED cairo)
  pkg_check_modules(FONTCONFIG REQUIRED fontconfig)
  pkg_check_modules(X11 REQUIRED x11)
  pkg_check_modules(XCB REQUIRED xcb)
  pkg_check_modules(XCBCURSOR REQUIRED xcb-cursor)
  pkg_check_modules(XCBKEYSYMS REQUIRED xcb-keysyms)
  pkg_check_modules(XCBUTIL REQUIRED xcb-util)
  pkg_check_modules(XCBXKB REQUIRED xcb-xkb)
  pkg_check_modules(XKBCOMMON REQUIRED xkbcommon)
  pkg_check_modules(XKBX11COMMON REQUIRED xkbcommon-x11)

@baconpaul
Copy link
Collaborator Author

Oh thanks
Yeah so much of this is leftover.
I can clean that up, but if you want (and zero pressure) feel free to make changes and send in PRs also. We gladly accept them. But again only if you want :)

@baconpaul
Copy link
Collaborator Author

I got it, the error from generating ttl for SurgeXT_FX.so led me out.

generator initializes the loading of presets , surge tries to find them in /usr/share/surge and other dirs, not found then throws the error window (I guess)
so I put the copying of resources before the build and passed further
but it's preferable to search presets in sourcetree/resources folder during build, providing CMAKE_SOURCE_DIR/resources in data search path, I think

Oh wow right. Great sleuthing.

So I am going to re-write all of that stuff shortly. My plan is to make it so it prints to stderr if no UI has been created. Let me get to that next in my stack and then try and re-set-up the LV2

Super cool.

@KottV
Copy link
Contributor

KottV commented Apr 23, 2021

Do you ever sleep, Paul?
i'm going...

@baconpaul
Copy link
Collaborator Author

it's the middle fo the day here! I also have 2 days where I have a bit of time

but anyway I just pushed a PR with a fix. Lets see if it works. I've made it so i you set the variable PIPLINE_OVERRIDE_DATA_HOME on linux it looks there.

The popup problem still needs solving of course and have a plan for that but it is a bit more involved.

but i bet this will get us to a pipeline build.

@baconpaul
Copy link
Collaborator Author

OK that hack didn't do it.
lemme ponder
i know what to fix but it will take a bit
i'll test it when i have the fix in place
great detective work!

@baconpaul
Copy link
Collaborator Author

Oh ha no got it now. This will all work.
Should get a pipeline build shortly
and a repro case on that xwindows popup also

@baconpaul
Copy link
Collaborator Author

OK awesome out CI pipeline just built the LV2 by setting that environment variable. I'm going to close this.

It's a separate issue to not make the un-edited surge pop up that dialog but I'll link that to this.

Wahey. We have Surge-XT LV2 building and have a successful build of it as a gate-to-merge

So now the question is: does it work? But those are gonna be separate issues.

Thanks @KottV for all your help

@KottV
Copy link
Contributor

KottV commented Apr 23, 2021

great detective work!

thanx)

@baconpaul
Copy link
Collaborator Author

Did a lot today so basically so you know, with the code at head

  1. If you DONT have any assets it no longer pops up a window but will print to stdout. Only pops a window if you start the GUI
  2. If you set that variable you can force scans for assets wherever you want
  3. I set that variable in our pipeline
  4. Also we have about 2500 lines less code than we did when you went to bed :)

So really everything is great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JUCE Ex Machina Issue that will likely be fixed by porting Surge to JUCE UI/plugin back-end
Projects
None yet
Development

No branches or pull requests

6 participants