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

static and dynamic libraries both built and installed #51

Closed
Be-ing opened this issue Nov 19, 2021 · 20 comments
Closed

static and dynamic libraries both built and installed #51

Be-ing opened this issue Nov 19, 2021 · 20 comments

Comments

@Be-ing
Copy link

Be-ing commented Nov 19, 2021

This is a hassle for packaging and downstream users. Please use Meson's standard mechanism for controlling whether the library is built statically or dynamically. Per Meson documentation:

It is generally preferred to use the library command instead of shared_library and static_library and then configure which libraries (static or shared or both of them) will be built at the build configuration time using the default_library built-in option.

@cannam
Copy link
Member

cannam commented Dec 1, 2021

I did look at this at the time I think, but the static library is used when building the other targets such as the command-line tool and plugins, so we have to build it regardless of whether the dynamic one is requested or not.

Of course it's possible I've missed a better way to do this.

My goals for build scripts in general are

  • The default build command should build (and then install) as many of the standard targets as possible in the current environment. This serves as a kind of documentation of what features are included, facilitates exercising the build system in testing, avoids people reflexively assuming that the target they want is not supported, and is generally a nice thing to do
  • Names of targets should match for compatibility those of any other (or previous) supported build systems
  • Any specific customisations people need should be available through command line arguments if possible, and if they are standard ones, so much the better

It sounds as if my first goal there (which is not really negotiable in my mind) may be in conflict with what you're asking for, but it could be helpful if you can explain why you think the current system is a hassle - I certainly don't want the system to be actively annoying!

@eli-schwartz
Copy link

eli-schwartz commented Dec 19, 2021

In another project of mine, I've built the library like this:

libfoo_a = static_library('foo_objlib', all_src, install: false)
libfoo = library('foo', link_with: libfoo_a, install: true)

All sources are built exactly once. Any targets that must be linked statically get a dependent link_with: libfoo_a. Any targets that don't care how they are linked, but wish to follow the default user configured link model, will link_with: libfoo instead.

If the user configures with -Ddefault_library=static then libfoo_objlib.a and libfoo.a are both created with the same objects, but you only pay the cost of two ar commands, not the cost of recompiling.

The current meson.build is using a method compatible with older versions of meson (< 0.52, but this project requires 0.53) that involves extract_all_objects(recursive: true), which accomplishes something similar for building the shared library from the static library, while introducing a bit of weirdness due to a custom option -Dno_shared=<bool> -- most people building projects using meson are likely expecting -Ddefault_library to work out of the box.

  • The default build command should build (and then install) as many of the standard targets as possible in the current environment.

FWIW, you can override meson's own defaults by adding to project() the kwarg default_options: ['-Ddefault_library=both'].

This would not prevent users from manually setting it back to =shared if they only want the shared library.

Aside: if you pass your installable library to pkgconfig.generate() it will include your pkg-config dependencies and found libraries and suchlike automatically. It will also correctly define the -l flag, which currently seems like it could be intended as -lrubberband-static (but is instead hardcoded to -lrubberband) for unclear reasons.

@cannam
Copy link
Member

cannam commented Dec 20, 2021

This all looks very helpful, thanks!

@Be-ing
Copy link
Author

Be-ing commented Dec 20, 2021

@eli-schwartz that seems like a decent solution, thanks. The important point for me is using the build system's standard mechanism for controlling whether the library is built statically or dynamically, not creating an ad-hoc system idiosyncratic to this particular library.

@cannam
Copy link
Member

cannam commented Jan 4, 2022

I've had a go at this and pushed the results - let me know how this looks!

There were a couple of other complications, mostly from the MSVC build for which I have been trying to retain naming compatibility with prior build systems, but the main thing is that it respects the default_library option and the wacky extra option has gone. Seems to check out OK in my tests so far.

@eli-schwartz
Copy link

From a quick glance, that seems to make sense. I'm surprised that link_with was unhappy on MSVC but tbh I don't know anything about that toolchain anyway.

IIRC the rationale for meson using the libfoo.a name on Windows is because foo.lib is the name for both an import library for the shared dll, and the common name for the static lib, and when building both they would have a name clash. I guess you traditionally added "-static" to the name instead... I guess there is not much you can do there, other than your workaround in the name of compat with previous versions. At least now it is scoped to windows :D

What do you think about adding another declare_dependency for the default library, e.g. rubberband_dep = declare_dependency(...)?

This would allow people to add rubberband as a wrap dependency fallback: https://mesonbuild.com/Wrap-dependency-system-manual.html#provide-section

@cannam
Copy link
Member

cannam commented Jan 5, 2022

IIRC the rationale for meson using the libfoo.a name on Windows is because foo.lib is the name for both an import library for the shared dll, and the common name for the static lib, and when building both they would have a name clash.

I do kind of like this logic, and I would probably go along with it for a new library.

What do you think about adding another declare_dependency for the default library, e.g. rubberband_dep = declare_dependency(...)?

This would allow people to add rubberband as a wrap dependency fallback: https://mesonbuild.com/Wrap-dependency-system-manual.html#provide-section

I've added what I think you mean (i.e. a two-liner after the definition of rubberband_library). Do take a look. I haven't actually looked into Wrap at all myself yet.

Thanks for your help here.

@eli-schwartz
Copy link

Almost. It should also set include_directories: 'rubberband' so that the superproject both:

  • links to subprojects/rubberband-2.0.0/librubberband.{so,a}
  • adds subprojects/rubberband-2.0.0/rubberband/ as an -I directory

when using rubberband_dep as a build dependency.

@cannam
Copy link
Member

cannam commented Jan 5, 2022

OK, I've added that too - though I am flying blind here as I don't know how to test it, but the idea seems benign enough! Let me know if I've bungled this.

@eli-schwartz
Copy link

I made a dumb mistake and suggested the wrong thing. Includes are supposed to be done as

#include <rubberband/RubberBandStretcher.h>

So, the include_directories object should be '.', not 'rubberband'.

Now I am actually at my computer and can test it. :)

You can test it by making an example meson project:

project('rubberband-test', 'cpp')

rubberband_dep = dependency('rubberband')

executable('rubberband-example', 'main.cpp', dependencies: [rubberband_dep])

And then creating this file:

$ cat subprojects/rubberband.wrap 
[wrap-hg]
url = https://hg.sr.ht/~breakfastquay/rubberband
revision = tip

[provide]
rubberband = rubberband_dep

Then configure meson using:

$ meson setup builddir --wrap-mode=forcefallback
[...]
Looking for a fallback subproject for the dependency rubberband because:
Use of fallback dependencies is forced.
requesting all changes
adding changesets
adding manifests
adding file changes
added 494 changesets with 1801 changes to 238 files (+3 heads)
new changesets 9996a45cda46:fa0323bacc9e
updating to branch default
129 files updated, 0 files merged, 0 files removed, 0 files unresolved

Executing subproject rubberband 

rubberband| Project name: Rubber Band Library
rubberband| Project version: 2.0.0
[...]

And verify that everything builds correctly (make sure rubberband isn't installed globally to the system, or /usr/include may leak).

Again, it will not build correctly, because I gave a bad suggestion:

adds subprojects/rubberband-2.0.0/rubberband/ as an -I directory

I lied, you don't want this, the files to include are not subprojects/rubberband-2.0.0/rubberband/rubberband/RubberBandStretcher.h (too many directories there)

So, the following fix makes it actually work:

diff -r fa0323bacc9e meson.build
--- a/meson.build	Wed Jan 05 12:55:01 2022 +0000
+++ b/meson.build	Wed Jan 05 19:56:59 2022 -0500
@@ -488,7 +488,7 @@
 #
 rubberband_dep = declare_dependency(
   link_with: rubberband_library,
-  include_directories: 'rubberband',
+  include_directories: '.',
 )
 
 if get_option('default_library') != 'shared' and rubberband_additional_static_lib != ''

@cannam
Copy link
Member

cannam commented Jan 6, 2022

I probably should have spotted that myself, even without the test run - sorry! The further explanation you've given is great though, really helpful.

When trying it myself, I was also led to discover that meson.source_root(), which I used in a couple of places to pick up the LADSPA and Vamp plugin linker scripts, is a bad idea because it gives the parent project's root instead of the subproject's. So I have fixed that as well as making the change you suggest.

Perhaps I should break out some of the more esoteric build info in the Rubber Band README into a separate file and add this information about subprojects to it, with a link to the Wrap docs. If nothing else it would be a useful reminder for me.

@eli-schwartz
Copy link

Great, happy to help.

Once this is tagged in a release I can add rubberband to the Meson WrapDB, and then people who depend on rubberband can download that wrap file using meson wrap install rubberband.

(It will download the tarball rather than mercurial tip, hence waiting for the tagged release.)

@Be-ing
Copy link
Author

Be-ing commented Jan 6, 2022

I will make a PR to finally get Rubberband in upstream vcpkg when a new release is tagged.

@cannam
Copy link
Member

cannam commented Jan 6, 2022

I will make a PR to finally get Rubberband in upstream vcpkg when a new release is tagged.

I had never previously tried vcpkg, but I just followed the getting-started instructions in this post and pulled down a couple of libraries to take a look. I was curious whether it would build static or dynamic libraries, since from the context here you did not want the Rubber Band build system to default to both. It went for static - is this the norm for vcpkg across platforms or is this a configurable thing?

I am curious how vcpkg handles libraries with differing or incompatible licence requirements. The examples in their intro are all BSD-ish, but I installed an LGPL library (libsndfile) and a GPL one (wildmidi) and the vcpkg command gave me no feedback that indicated there was any licence to consider. Since it does dependency tracking (so may install many libraries from a single command), is intended for vendoring in development (hence pre-built static libraries etc without any developer involvement), and comes from a company that typically supports proprietary software, I can easily imagine a user assuming everything there was redistributable in a proprietary application, which seems problematic. Does it have a standard or conventional way to query licence status?

@Be-ing
Copy link
Author

Be-ing commented Jan 6, 2022

Whether vcpkg builds static or dynamic libraries is controlled by the target triplet.

If a vcpkg user violates the license of a library they are using, that's their responsibility, but I agree that vcpkg should make it easier to identify the licenses of the whole dependency graph. There is a way for vcpkg manifests to specify an SPDX license identifier but it is not widely used. There is a pending PR to check for that on vcpkg's CI.

@cannam
Copy link
Member

cannam commented Jan 20, 2022

v2.0.1 is out now with this fix in it!

@Be-ing
Copy link
Author

Be-ing commented Jan 20, 2022

I made a PR to add it to vcpkg.

@Be-ing
Copy link
Author

Be-ing commented Jan 20, 2022

This bug is fixed now, closing.

@Be-ing Be-ing closed this as completed Jan 20, 2022
@Be-ing
Copy link
Author

Be-ing commented Jan 23, 2022

There is a way for vcpkg manifests to specify an SPDX license identifier but it is not widely used. There is a pending PR to check for that on vcpkg's CI.

microsoft/vcpkg#20790 was merged recently, so this situation of making the license info more easily accessible in vcpkg should improve gradually.

@eli-schwartz
Copy link

Once this is tagged in a release I can add rubberband to the Meson WrapDB, and then people who depend on rubberband can download that wrap file using meson wrap install rubberband.

I forgot to do this, originally, but it is now done: mesonbuild/wrapdb@c029bee

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