-
Notifications
You must be signed in to change notification settings - Fork 231
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
examples are hard-coded to use the shared form of the library #106
Comments
FYI, yes-it-does.zip Edit: It is a relatively new feature (2015). |
Okay, I stand corrected :-] (Must have been thinking of GitLab, then... I recalled hearing that one of the two has that limitation) |
This seems correct. Let me play around with it to make sure I understand and then we'll schedule it for next API breakage. |
cc @Naftoreiclag @IceDragon200 @ligfx @hasufell @diogocp Everyone I just cc'd, you made a pull request that had to do with the cmake build script. Everyone OK with this change? |
TL;DR: LOOKS GOOD Seems legit, that's how I've seen other CMake projects set up. While we're on the topic, there's plenty of other possible improvements to the |
Looks fine to me, reduces some of the code churn from before |
👎 this is removing useful functionality. It doesn't really matter what is "conventional" in cmake. Being able to build both dynamic and shared libraries in one go is conventional in linux. Just because it requires a bit more code doesn't mean it's not useful. Also, the breakage you encountered is simply a bug that can be fixed and unrelated to the decision you are proposing. |
@hasufell I agree it's a feature removal, but how useful is it really? Seems like almost everything on Linux packages/builds against shared libraries anyways, and the building-both-in-one-go feature is more a quirk of auto tools. Let's step back a sec. It comes down to, is this feature useful enough to merit more effort on the project's part, vs. pushing that effort to the users trying to do something out-of-the-ordinary? |
Most end users would only want one form (shared or static) of the library. The time when you would want both is generally when you're building the library for others, e.g. as a distro maintainer or packager. In that scenario, you're typically dealing with a large number of packages, and your job is made easier when all those packages follow their respective build-system conventions. It's not much of a favor for us to build both libraries in one go if packagers already have standard scripts for CMake that do the two separate builds. I've been looking a bit more closely at the listfile, and am preparing a revised patch. The |
@iskunk Probably better to fix those issues in a different patch, to keep this one on-focus. Also, if you haven't gotten Git set up yet, GitHub lets you make edits in your browser: |
Okay, so the JACK problem just turned out to be issue #11. My system has that older version of JACK without I fixed up the One small thing that I didn't address here: @ligfx: I could open a new issue for these... they are relatively minor. Would the house prefer lots of little issues over consolidating them by area? Editing in the browser... hmm... that's a new one on me. I'll have to play with that.... |
The house is @andrewrk, but my personal preference is to try to keep changes small enough that new failures can be properly root-caused. Changing from shared+static libraries could introduce one type of build failure (dunno why it would, but just in case...), and changing how JACK is searched for could introduce a different one. re: the latest revision:
|
@ligfx: Yes, being able to point to the root cause is desirable... if the changes are relatively minor, wouldn't a single issue be specific enough? I concede the three points you made. (Someone needs to check the success case, as I can only test the fail case.) |
That really depends on the distro. Gentoo likes to give the choice to enable the user to build certain parts of the system statically. Alpine Linux also has special needs wrt static libs. Do we really want to make assumptions here that may or may not be correct? It's not really a whole lot of complicated code, is it? The report looks like an exaggeration to me. |
@hasufell, nothing that has been discussed here will prevent distributions from building this library in the form they want. All it takes to build libsoundio statically after these changes is
This is consistent with other CMake-based projects, many of which are already handled by the specialty distributions you mentioned. Whatever limitations CMake and its conventions may impose on a project have already been worked around eons ago. That said, it may be worth revisiting the topic of which form of the library (shared or static) should be the default. My patch has the former, as that was the preferred one for the examples and tests. However, I think that end users building this library will generally want a static build (to avoid the specter of |
You didn't get what I was saying: a lot of distros want both at the same time, not one or the other.
That's irrelevant. It's relevant what people/distributors expect of a build system. What you are proposing is a step backwards. And the fact that other cmake projects do annoying stuff too isn't a particularly appealing argument. In fact, I know quite a few cmake based projects that had to work around this cmake oddity. |
|
oh dear. So you...
Great step forward. |
Albeit off topic; curious; lurker; etc... what is the use-case for the shared + static? I do static packaging for a fairly large project and occasionally we have Debian packagers that chime in with very packaging-specific commits. We always discuss them but in the end tend to honor them because the static packagers are already written; no harm done. The distro packagers however tend to have quite a few packages they manage, so reducing their work as a trade-off to a slight increase in build logic is usually favored. In regards to this "non-PR" PR, although the original diff simplified the Note, I too haven't yet seen the shared + static in a single |
Hi @tresf, The use case for shared+static is straightforward: Build both forms of the library so that The idea of doing shared+static builds in a single pass mainly comes from Libtool, which achieves this using a compiler wrapper. Every source file is compiled into an CMake, however, does not use a compiler wrapper; it drives the compiler directly with the appropriate flags. So if you want to build both shared and static libraries, you have to have separate targets for them (as was done here). Then you have to worry about which of the two forms are used in later parts of the build; Libtool defaults to the shared library, but if you configure with All this logic would have to be re-created for CMake, because this is not how CMake normally works. To give an analogy, this would be akin to setting up a GNU Autotools-based project to build both libfoo_debug and libfoo_release in the same tree, approximating the behavior of a Visual Studio solution. Which is to say, it can be done, and some folks may find it useful/familiar, but most are going to be annoyed that this is not handled as a configure-time option [like every other Autotools-based project out there]. And the project is saddled with maintaining the additional complexity of that build approach in perpetuity. Making a $FRAMEWORK-based build system operate like other $FRAMEWORK-based build systems is ultimately the path of least resistance. It's easier to maintain, it's easier on packagers (who have scripts to address the most common cases), and it's easier for casual users who have never seen the project before.
I'm addressing multiple things in my patch. Less armor-piercing round, more birdshot :-) |
@iskunk thanks for the detailed explanation.
Wouldn't |
Yes, but CMake uses |
Depends on the distro. It's better to not make too many assumptions there.
People have been doing that with all sorts of build system for years, including hand-written Makefiles. It's pretty much the de-facto standard. Libtool is only special with regards to libtool files (.la), but that's a completely different topic. It would save a lot of time if you do less assumptions and accept the fact that the feature is useful. |
New switch name perhaps? 🍻 |
Fedora explicitly recommends not building them. With my debian downstream maintainer hat on, I wouldn't mind losing the static library (I normally only build shared ones), and if required add the couple of commands needed to build the static one too |
Talk to the CMake folks ^_^ |
I don't have an opinion either way, but if you don't want to recompile the code twice and want a shared and static lib you could do something like this:
|
Neither of them being source distros, where the impact is quite different. Both in terms of leaving the choice of building static libraries out and in terms of doubling compile time, because someone thought it's "simpler". |
@hasufell Petulantly repeating yourself and referring to other arguments in scare quotes is not contributing to the conversation! You've offered no concrete data to support your position, instead relying on generalities such as "lots of distros" and "source distros." We've heard from people involved in the actual packaging process who've offered their thoughts and experiences, and who actually respect the idea that making code simpler and following convention might be a reasonable project goal. If you have any specific data to add or point to (e.g. "Distro X requires both static and shared in the same package" or "CMake person Y thinks shared and static should be built at the same time"), contribute that, otherwise please hold your peace. |
Alright everyone. Easy on the name calling. We're all on the same team here, trying to make software work better for the open source community. I'm going to step in and make a judgement call here.
I think this is the key point here - convention. cmake makes some unfortunate decisions and doesn't completely solve the problem of building software optimally, but at the end of the day it's useful because of its widespread use. Follow the cmake conventions, and it becomes easy for package managers of various systems to build the software without having to individually tweak it. Breaking away from cmake convention would cause package maintainers to have to create patch the build system. I appreciate the counterpoints in this discussion, and you're not wrong. But especially given the fact that libsoundio builds in sub ten seconds, I think it makes more sense to optimize for using cmake in the simple, expected, standard way than to try to do something clever to save time, for some use cases. Just wait for the zig build system to come out and everyone can be happy :-) |
I've been a gentoo developer for 5 years and am still working on several source distros as a collaborator now and then (gentoo, exherbo...). Thanks for the reminder though. I've been trying to fix broken cmake build systems for quite some time to make things work properly on the distributions I've been packaging for years, including libraries like mbedtls. Now, seeing some people walking through the ecosystem and reverting things isn't something that I find particularly appealing. I also have no time or interest in educating you about source distros, what they do, why they do it and so on.
Since you seem unable to do your own research, here's an example that this "simplifcation" will break: https://gitweb.gentoo.org/repo/gentoo.git/tree/media-libs/libsoundio/libsoundio-1.1.0.ebuild |
hmm. Now I'm less sure about the best approach here. Status quo does seem reasonable. The proposal this issue makes is one of simplification, but does not fix behavior for any specific use cases. This makes me lean in favor of status quo. Side note: you could enable JACK support in that package by patching 2 lines in libsoundio to match the function prototype from the version of JACK on the system. (This prototype is incorrect according to the official jack.h header which is why this problem exists in the first place). Either way, problem should go away after next jack2 release. |
CMake also has the convention to not provide uninstall rules, so LFS users (or anyone else, temporarily in need to install to I guess we'll just have to go with any convention a random cmake dev imposes on us, so the rest of the users can go screw themselves... |
Are these things really as mutually exclusive as people are making it out to be? Could both camps be satisfied with an extra options flag? |
I agree that not providing an uninstall target is a missing feature in cmake. However, in favor of the convention argument - cmake does provide install_manifest.txt, which a distribution could use to provide an uninstall target, solving the problem system-wide. cmake users would need to do nothing for this to work. Versus if cmake users tried to solve the problem themselves we would end up with competing conventions and overall more complexity.
At the end of the day I'm going to go with the thing that is best for the users. This means people building libsoundio from source directly and it also includes folks such as yourself who are maintaining packages. I'm on your side. Let's figure out what is best for users together. |
@mdsitton there's no need for that, the real problem here was:
which means there is a bug. It can be fixed... |
I agree. I'll fix the problem of the examples requiring the shared form. Then I will close this issue, since it is not clear what the actual issue is. Anyone is free to open new issues with specific problems and use cases. |
My original report was basically "It doesn't have to be that complicated" :-] I would point out that if shared+static libraries in a single CMake build are desirable, then this should be implemented as a CMake module and pulled in (e.g. Re uninstall rule: This is addressed in the CMake FAQ. Keep in mind that Unix makefiles are only one of the many build mechanisms supported by CMake (in addition to Visual Studio solutions, Xcode projects, etc.). There's plenty more critique that can be leveled at CMake-generated makefiles, not least that they are baroquely complex and don't respect subdirectory scoping (a side effect of having the same semantics as IDE projects). At the end of the day, if you want a build system that acts like Autotools, then you're much better off going with Autotools proper rather than trying to beat something else into a similar shape. (That would actually be feasible, as that handily addresses all systems except for [non-MinGW] Windows. And I could submit a straightforward NMAKE makefile for that.) |
@andrewrk I too would appreciate the ability to build examples statically. I'm having problems trying to use libsoundio.a and the .o files with it from another programming language, so the examples are the only way I can think of to determine whether the problem is with the language's compiler options or my libsoundio build. |
I'm not seeing any combination of cmake flags that prevents make from building libsoundio.so* and having the example programs depend on it. Is -DBUILD_SHARED_LIBS=0 -DBUILD_STATIC_LIBS=1 not the way to invoke this fix? |
|
Still not working for me. Another git pull of the v2 branch shows my download is up to date. I was using rsoundio's flags as the example. The Arch User Repository flags use |
sorry, the option is called
so you want |
Thanks a bunch! Do you think the available/used flags should be added to the documentation? For people like me, or package managers. |
They're all kinda at the top of CMakeLists.txt which is where I would expect package managers to look:
where would you put this documentation? |
Changelog? You're right though that they might check CMakeLists again at a major release. |
A file called |
Yeah we can fix that. I'll take a look soon. Feel free to make a PR if you On Oct 20, 2016 10:37 PM, "Joshua Olson" notifications@github.com wrote:
|
I see that the
CMakeLists.txt
file for libsoundio has two separate options for building dynamic vs. static libraries.I take it this was inspired by Libtool, which in its usual way of working compiles both static and shared libraries in one pass.
However, in CMake, it is conventional to build either one or the other, controlled by the special variable
BUILD_SHARED_LIBS
. I've put together a patch that simplifies the listfile accordingly.(I don't currently have a proper setup to prepare a Git pull request, and I see that GitHub still doesn't allow issue attachments, so I've uploaded my patch here.)
The immediate issue that spurred this was breakage when I disabled the dynamic lib and attempted to build the examples. The examples are hard-coded to use the shared form of the library. It would be possible to code the listfile so the examples/tests link with one or the other as appropriate, but because this approach is foreign to CMake anyway, I believe reducing the complexity is preferable.
Oh, and as an extra, I added a bit to set
CMAKE_INSTALL_RPATH
. This allows the executables linked to a shared libsoundio to work in the install tree (without needing to setLD_LIBRARY_PATH
) as well as the build tree.The text was updated successfully, but these errors were encountered: