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

Please make a release #306

Open
yurivict opened this issue Dec 12, 2021 · 25 comments
Open

Please make a release #306

yurivict opened this issue Dec 12, 2021 · 25 comments
Milestone

Comments

@yurivict
Copy link

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
Copy link
Collaborator

falkTX commented Dec 12, 2021

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

@yurivict
Copy link
Author

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
Copy link
Collaborator

falkTX commented Dec 12, 2021

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
Copy link
Author

where did you hear or seen such things?

Here.

@JohannesLorenz JohannesLorenz added this to the 0.90.4 milestone Oct 19, 2023
@JohannesLorenz JohannesLorenz pinned this issue Jan 22, 2024
@JohannesLorenz
Copy link
Collaborator

JohannesLorenz commented Jan 22, 2024

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
Copy link
Author

@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
Copy link
Author

yurivict commented Jan 22, 2024

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
Copy link
Author

yurivict commented Jan 22, 2024

@JohannesLorenz

The latest master revision 04c6ad9 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
Copy link
Collaborator

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
Copy link
Collaborator

@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
Copy link
Author

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

Which do you mean?

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

@JohannesLorenz
Copy link
Collaborator

@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
Copy link
Author

yurivict commented Feb 8, 2024

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
Copy link
Author

I tried the latest revision 1df3a2a 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
Copy link
Collaborator

@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
Copy link
Collaborator

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

@JohannesLorenz
Copy link
Collaborator

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

@yurivict
Copy link
Author

yurivict commented May 13, 2024

@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
Copy link
Collaborator

@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 0f567ee .

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
Copy link
Collaborator

@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
Copy link
Collaborator

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants