0.2.2: lv2lint fails on globally visible symbols

[dvzrv]

Hi! When building 0.2.2 as a package for Arch Linux I also ran lv2lint against the plugin.
Unfortunately it fails on globally visible plugin symbols:

lv2lint 0.16.2
Copyright (c) 2016-2021 Hanspeter Portner (dev@open-music-kontrollers.ch)
Released under Artistic License 2.0 by Open Music Kontrollers
Profile Size <1105>
<https://github.com/lucianodato/noise-repellent#new>
    [FAIL]  Plugin Symbols
              binary exports superfluous globally visible symbols:
                * noise_profile_get_elements
                * noise_profile_state_free
                * signal_crossfade_initialize
                * noise_profile_state_initialize
                * signal_crossfade_free
                * signal_crossfade_run
                * noise_profile_get_size
              seeAlso: <http://lv2plug.in/ns/lv2core#binary>

This can be circumvented with using -fvisibility=hidden in c_args.
I can provide a PR for that.

[lucianodato]

Hi @dvzrv! Thanks for this remark. I've changed that in favor of using CFLAGS env variable like I configured into the build pipeline

My question would be, from the standpoint of distros packaging, is it preferable to have them directly in the build script? I'm open to change it back to what it was but not sure what should be the right way to do it considering all the use cases.

Basically this change was just to avoid hardcoding stuff in the build script that could affect the building in other OSes than linux or other architectures, I'm not sure but I think meson overrides whatever you have in the script in favor of the env CFLAGS so it could be that it's completely fine.

Let me know what you think!

[dvzrv]

My question would be, from the standpoint of distros packaging, is it preferable to have them directly in the build script? I'm open to change it back to what it was but not sure what should be the right way to do it considering all the use cases.

I believe it is safe to assume a minimum set of flags and -fvisibility=hidden should definitely be part of those, as it guarantees that there are no symbol clashes.
Optimizations can be a bit more tricky, but it is possible to detect the OS, CPU architecture and probe for capabilities with the help of meson to dynamically add them, so this should not be a problem. Otherwise downstreams all need to do that manually, leading to a plethora of builds using different flags (and therefore potential differences in performance).

[dvzrv]

FTR, this might be helpful for implementing further details in meson.build: https://mesonbuild.com/Reference-tables.html#reference-tables

[lucianodato]

Thanks for your input. I used to have this minimal config in meson. Will revert it back to what it was this afternoon and do another release.

[lucianodato]

@dvzrv I'll leave just the flag that you introduced because it's necessary but the optimizations will be optional so I don't have to pollute the build script with ifs. Fair enough?

[dvzrv]

@dvzrv I'll leave just the flag that you introduced because it's necessary but the optimizations will be optional so I don't have to pollute the build script with ifs. Fair enough?

This is your project and you need to figure out how to configure its build system. However, please note that I neither have the time nor the interest to introduce optimization flags, if this is not done by upstream. I maintain over 700 packages and simply do not have the time to spend that amount of work on each of them to tune them for best performance, etc. (it is also against Arch Linux's principle of "following upstream").

Leaving out the very basic -ffast-math and -msse/ -msse2 is contra-productive in my eyes as it seems you deem them necessary for your application to function properly.

To compare this with another existing LV2 plugin: https://github.com/x42/convoLV2/blob/662d837824d5b7d2c338cb5f928b23e68fe4b974/Makefile#L10

Here you can see, that basic optimization flags are provided by default (although overridable). With meson you have the luxury of detection and fine-grained application of flags based on CPU architecture for all users of your project by default. Why would one not use/ want that? Using conditional statements to allow for various scenarios is one of the big strength of meson's syntax after all.

[lucianodato]

Ok thanks for the clarification. I'm not an arch user so I don't really have a clue what is preferred. Will include the optimizations back.