calf-studio-gear / calf

Developers repository of Calf Studio Gear. Expect some issues when using it for production.
http://calf-studio-gear.org
GNU Lesser General Public License v2.1
675 stars 95 forks source link

Please make a release #306

Closed yurivict closed 1 month ago

yurivict commented 2 years ago

The last release was in July 2019, which is 2.5 years ago.

Please also make sure that the release builds with clang, because currently it does not.

falkTX commented 2 years ago

Issue #294 should be fixed before that too, otherwise lv2 guis are just broken

yurivict commented 2 years ago

To be clear: Calf is currently completely broken (unbuildable) on systems with clang as a major compiler because of the above issue. clang and gcc generally don't mix, so if Jack and other dependencies are built with clang, Calf can't just be built with gcc to work around clang incompatibilities like this.

falkTX commented 2 years ago

clang and gcc generally don't mix, so if Jack and other dependencies are built with clang, Calf can't just be built with gcc to work around clang incompatibilities like this.

where did you hear or seen such things?

I have been using clang for testing and mix gcc-built with clang-built libraries without any issues.. Some libraries are even now in rust, so there is a 3rd compiler mixed in. It is the first time I hear of such issues, would love to read more if you have some relevant links.

yurivict commented 2 years ago

where did you hear or seen such things?

Here.

JohannesLorenz commented 7 months ago

For your info, I plan a bugfix release (0.90.4) in the next weeks. The mentioned #156 and #294 have been solved, aswell as some other bugs, and CI is working (except of MinGW, where I could need some help, but it is not critical).

If there are any bugs that you think must make it into the next release, let me know.

yurivict commented 7 months ago

@JohannesLorenz

In the FreeBSD port audio/calf-lv2 we have this post-install script that makes the absolute symlink relative:

post-install: # fix absolute symbolic link to be relative
        @${RM} ${STAGEDIR}${PREFIX}/lib/lv2/calf.lv2/calf.so
        @${RLN} ${STAGEDIR}${PREFIX}/lib/calf/libcalf.so ${STAGEDIR}${PREFIX}/lib/lv2/calf.lv2/calf.so
        @${RLN} ${STAGEDIR}${PREFIX}/lib/calf/libcalflv2gui.so ${STAGEDIR}${PREFIX}/lib/lv2/calf.lv2/calflv2gui.so

If this is still happening in the latest revision - this is better to be fixed.

yurivict commented 7 months ago

Then we have this code:

.if ${CHOSEN_COMPILER_TYPE} == gcc
CXXFLAGS+=      -finline-limit=80 -finline-functions -finline-functions-called-once
.endif

which means that the build fails (or used to fail at some point) when gcc is used without these flags. I am not sure if this is still the case, but this also better be fixed.

These were added in 2015, so maybe they aren't relevant any more.

yurivict commented 7 months ago

@JohannesLorenz

The latest master revision 04c6ad97 still has this failure not fixed:

*** Warning: Linking the executable calfbenchmark against the loadable module
*** libcalf.so is not portable!

We have this patch as a workaround for it in the FreeBSD port:

--- src/Makefile.am.orig        2021-06-25 19:25:29 UTC
+++ src/Makefile.am
@@ -40,9 +40,9 @@ calfbenchmark_LDADD = libcalf.la
 libcalf_la_SOURCES = audio_fx.cpp analyzer.cpp lv2wrap.cpp metadata.cpp modules_tools.cpp modules_delay.cpp modules_comp.cpp modules_limit.cpp modules_dist.cpp modules_filter.cpp modules_mod.cpp modules_pitch.cpp fluidsynth.cpp giface.cpp monosynth.cpp organ.cpp osctl.cpp plugin.cpp preset.cpp synth.cpp utils.cpp wavetable.cpp modmatrix.cpp
 libcalf_la_LIBADD = $(FLUIDSYNTH_DEPS_LIBS) $(GLIB_DEPS_LIBS)
 if USE_DEBUG
-libcalf_la_LDFLAGS = -rpath $(pkglibdir) -avoid-version -module -lexpat -disable-static
+libcalf_la_LDFLAGS = -rpath $(pkglibdir) -avoid-version -lexpat -disable-static
 else
-libcalf_la_LDFLAGS = -rpath $(pkglibdir) -avoid-version -module -lexpat -disable-static -export-symbols-regex "lv2_descriptor"
+libcalf_la_LDFLAGS = -rpath $(pkglibdir) -avoid-version -lexpat -disable-static
 endif

 if USE_LV2_GUI
@@ -55,9 +55,9 @@ pkglib_LTLIBRARIES += libcalflv2gui.la
 libcalflv2gui_la_SOURCES = gui.cpp gui_config.cpp gui_controls.cpp ctl_curve.cpp ctl_keyboard.cpp ctl_knob.cpp ctl_led.cpp ctl_tube.cpp ctl_vumeter.cpp ctl_frame.cpp ctl_fader.cpp ctl_buttons.cpp ctl_notebook.cpp ctl_meterscale.cpp ctl_combobox.cpp ctl_tuner.cpp ctl_phasegraph.cpp ctl_pattern.cpp metadata.cpp giface.cpp plugin_gui_window.cpp preset.cpp preset_gui.cpp lv2gui.cpp osctl.cpp utils.cpp ctl_linegraph.cpp drawingutils.cpp

 if USE_DEBUG
-libcalflv2gui_la_LDFLAGS = -rpath $(lv2dir) -avoid-version -module -lexpat $(GUI_DEPS_LIBS) -disable-static  -Wl,-z,nodelete
+libcalflv2gui_la_LDFLAGS = -rpath $(lv2dir) -avoid-version -lexpat $(GUI_DEPS_LIBS) -disable-static  -Wl,-z,nodelete
 else
-libcalflv2gui_la_LDFLAGS = -rpath $(lv2dir) -avoid-version -module -lexpat -export-symbols-regex "lv2ui_descriptor" $(GUI_DEPS_LIBS) -disable-static  -Wl,-z,nodelete
+libcalflv2gui_la_LDFLAGS = -rpath $(lv2dir) -avoid-version -lexpat -export-symbols-regex "lv2ui_descriptor" $(GUI_DEPS_LIBS) -disable-static  -Wl,-z,nodelete
 endif
 endif

@@ -79,7 +79,7 @@ if USE_GUI
 endif
 if USE_LV2
        install -d -m 755 $(DESTDIR)$(lv2dir)
-       ln -sf $(pkglibdir)/calf.so $(DESTDIR)$(lv2dir)/calf.so
+       ln -sf $(pkglibdir)/libcalf.so $(DESTDIR)$(lv2dir)/calf.so
        rm -f $(DESTDIR)$(lv2dir)/*.ttl
        $(top_builddir)/src/calfmakerdf -m ttl -p $(DESTDIR)$(lv2dir)/ -d $(DESTDIR)$(pkgdatadir)/
 if USE_SORDI
JohannesLorenz commented 7 months ago

In the FreeBSD port audio/calf-lv2 we have this post-install script that makes the absolute symlink relative:

This looks very much like #290 , which has been merged to master. Can you please confirm?

which means that the build fails (or used to fail at some point) when gcc is used without these flags.

These are all optimization flags. Are you sure that the build really fails without them? Can you make some tests?

JohannesLorenz commented 7 months ago

@yurivict About your large diff, the second change (-export-symbols-regex "lv2_descriptor") has been fixed on master. I do not know if we can just remove -module, since I do not know what it does. As this is automake, in the CMake port, this is not present and thus the issue might have been fixed. About the symlink, again, #290 ?

yurivict commented 7 months ago

About GCC flags: w/out these flags supplied by the port they are still present in the build - either calf itself adds them, or some dependency adds them. It is irrelevant from the port standpoint. I'll just remove these lines.

yurivict commented 7 months ago

The above patch fixes the "Warning: Linking the executable calfbenchmark against the loadable module". The build fails w/out this warning fixed. We had a long-standing failure in this port until this patch was added.

It's better that this is fixed in the master. I know for a fact that other distros also experienced such failure, I just don't remember who exactly did.

JohannesLorenz commented 7 months ago

The above patch

Which do you mean? The one that you provided, or the PR that I linked you? Have you tested whether the linked PR fixes the issue?

yurivict commented 7 months ago

Which do you mean?

The patch that I've pasted above that comes from the FreeBSD port patch.

JohannesLorenz commented 7 months ago

@yurivict I made a few tests to remove the -module parameter, as suggested. I cannot see a difference. I guess the people who wrote the patch know what they are doing, but it would really be good to know why we remove it. Right now I would not know how to explain this in a commit message.

The libtool docs just says that theses modules are "libtool modules": "These are libtool libraries meant to be dlopened. They are indicated to libtool by passing -module at link-time.". That does not sound wrong at first?

yurivict commented 7 months ago

Please keep in mind that libtool and GNU tools in general is an ailing, legacy technology. New projects should use cmake or meson as they are a lot more modern and stable.

I'll re-test the latest code in the FreeBSD ports framework today.

yurivict commented 7 months ago

I tried the latest revision 1df3a2a7 and there are still these warnings:

*** Warning: Linking the executable calfbenchmark against the loadable module
*** libcalf.so is not portable!

The build also fails:

ld: error: undefined symbol: dsp::reverb::update_times()
>>> referenced by benchmark.cpp
>>>               benchmark.o:(reverbir_calc())
>>> referenced by benchmark.cpp
>>>               benchmark.o:(reverbir_calc())
>>> referenced by benchmark.cpp
>>>               benchmark.o:(dsp::reverb::setup(int))
>>> referenced 1 more times

ld: error: undefined symbol: dsp::reverb::reset()
>>> referenced by benchmark.cpp
>>>               benchmark.o:(reverbir_calc())
JohannesLorenz commented 6 months ago

@yurivict I removed the -module on branch github-actions, as a first step. For the .if ${CHOSEN_COMPILER_TYPE} == gcc thing, can you please provide a patch, because I do not know where exactly you want them inserted?

About the relative symlinks, can you please explain what is the reason for this? My guess is that this is only relevant if you move the installation from the ./configure --prefix XXX to another directory than XXX? Though I wonder in what case that is useful.

JohannesLorenz commented 5 months ago

@yurivict Friendly Reminder: I could still need your feedback here. It will be valuable to create the next calf release.

JohannesLorenz commented 3 months ago

@yurivict Are you still planning to answer here? I think it would be very important.

yurivict commented 3 months ago

@JohannesLorenz

Sorry for the delay.

This patch has 2 parts.

  1. Removal of -module
  2. Adjustment of the symbolic link

I believe that the "-module" argument caused the original problem, either through the buggy implementation somewhere in GNU Tools, or through some other reason. The item 2. is present because removal of "-module" changes the name of the produced file. It is still linked to the same destination, so there should be no change in the resulting installation.

About the relative symlinks, can you please explain what is the reason for this? My guess is that this is only relevant if you move the installation from the ./configure --prefix XXX to another directory than XXX? Though I wonder in what case that is useful.

Most packaging systems install files into a stage dir, not into $PREFIX. The reason is that they don't want to actually install the package, they just need to get all files that would be installed. The destination is set as ./configure --prefix $(DEST)$(PREFIX).

This is well supported by GNU Tools, cmake, etc.


We have a very rigorous port/build infrastructure in FreeBSD, and it regularly rebuilds the package calf-lv2-0.90.3.20210427_3.

I have high confidence that the linked patch should be good for everybody, because the same ports infrastructure builds thousands GNU Tools based packages without problems.

JohannesLorenz commented 3 months ago

@JohannesLorenz

In the FreeBSD port audio/calf-lv2 we have this post-install script that makes the absolute symlink relative:

post-install: # fix absolute symbolic link to be relative
        @${RM} ${STAGEDIR}${PREFIX}/lib/lv2/calf.lv2/calf.so
        @${RLN} ${STAGEDIR}${PREFIX}/lib/calf/libcalf.so ${STAGEDIR}${PREFIX}/lib/lv2/calf.lv2/calf.so
        @${RLN} ${STAGEDIR}${PREFIX}/lib/calf/libcalflv2gui.so ${STAGEDIR}${PREFIX}/lib/lv2/calf.lv2/calflv2gui.so

If this is still happening in the latest revision - this is better to be fixed.

I think the comment from the patch is wrong. The symlinks have never been absolute - at least until 2011 (I did not check further). What the patch does is 2 things:

  1. Fix a symlink .../calf.lv2/calf.so to lib/calf/libcalf.so (because it earlier linked to "lib/calf/calf.so", which did not exist)
  2. Create a new symlink for libcalflv2gui.so (because this symlink was missing completely)

From how I read it, both must have been fixed in 0f567ee87203ef0b2472b0a688b65e34af3972b7 .

Additionally, I removed -module in my branch github-actions, which I yesterday rebased to the master (which fixes the symlink by the commit I just mentioned).

In summary, branch github-actions

  1. fixes the symlinks like your patch
  2. removes -module like your patch
  3. does not change the compiler switches, but you said you would remove it

So, can you please confirm if branch github-actions now fulfills all your requests?

JohannesLorenz commented 3 months ago

@yurivict Sorry, I just thought about it again. The links are still absolute. I will change them to relative and let you know then.

JohannesLorenz commented 3 months ago

I made some tests. If I run ./configure --prefix=$PWD/install (absolute prefix), then the links are also absolute:

calf.so -> /home/.../calf/install/lib/calf/libcalf.so
calflv2gui.so -> /home/.../calf/install/lib/calf/libcalflv2gui.so

If I run ./configure --prefix=install, I get an immediate error:

$ ./configure --prefix=install
configure: error: expected an absolute directory name for --prefix: install

However, this behavior has never been changed - it is like that since the first commit of CALF in git.

Summary:

  1. I cannot reproduce how to get relative symlinks - can you reproduce it @yurivict ? If yes, how?
  2. It would be nice if you could test branch github-actions now.
JohannesLorenz commented 3 months ago

Summary:

1. I cannot reproduce how to get relative symlinks - can you reproduce it @yurivict ? If yes, how?

2. It would be nice if you could test branch `github-actions` now.

@yurivict Friendly reminder that I could still need your input here. This is one of the both last issues blocking us from making the next release.

JohannesLorenz commented 1 month ago

@yurivict Last reminder: If you don't answer, I will just bring my commit to master without your feedback and make a release next week.

JohannesLorenz commented 1 month ago

Closing as missing feedback. Feel free to reopen.