Now CMake will build both static and shared libs #960
Conversation
| target_link_libraries(avrdude PUBLIC libavrdude) | ||
| if (sharedlib IN_LIST BUILT_LIBS) | ||
| target_link_libraries(avrdude PUBLIC sharedlib) | ||
| else() | ||
| target_link_libraries(avrdude PUBLIC staticlib) | ||
| endif() |
There was a problem hiding this comment.
Should we retain the old behaviour of linking the executable with the static library? Or should we make the executable dynamically-linked when possible (like I've done here) ?
There was a problem hiding this comment.
May I ask, why would one not have binaries with statically linked libraries? It makes it way easier to bundle the official Avrdude release binaries without having to think about having the right drivers installed.
A binary that "just works" is what I personally prefer.
There was a problem hiding this comment.
Because it entirely defeats the idea of a shared/dynamic library?
To me, it seems this is really a philosophical issue. On Unix, 30 years ago, everyone was asking for shared libraries, because they can be shared between applications. That's why they have the suffix .so, for a "shared object".
On Windows, everyone tends to ship their own version of the DLL anyway. That completely defeats the idea of sharing something. It's then really just a complicated way of doing what could have been achieved with a static linking in the first place.
For that, I'm perfectly fine if we separate it that way: link statically on Windows, and used shared libs an Unix.
There was a problem hiding this comment.
Surely, for AVRDUDE, it's not really a concern, unless we might have a second consumer of the shared lib some day. Even running multiple AVRDUDE processes doesn't gain from a shared library since the "text" (program code) will be shared between these processes anyway.
There was a problem hiding this comment.
It makes it way easier
This is the whole point here. Static libraries are so much easier to use. And it is not just bundling packages. Shared objects or DLLs are a lot of work. You either have to keep your interfaces stable, or you better make sure you get your versioning straight. On Windows, this is why everybody uses COM, i.e. you have immutable interfaces that work very much like pure virtual C++ classes, including all the benefits this implies. And this is what I meant earlier, when I said the avrdude codebase is in no shape for a shared object: We currently have no abstraction at all, let alone immutable interfaces.
I think we should focus on adding features to avrdude, not maintaining shared objects that are not going to be used.
There was a problem hiding this comment.
It makes it way easier
This is the whole point here. Static libraries are so much easier to use.
I think we are in strong disagreement here. ;-)
And it is not just bundling packages. Shared objects or DLLs are a lot of work.
I don't think they are more work than static ones. (OK, Windows DLLs seem to be more work, since it seems they cannot refer to global symbols from the resulting executable. That's another matter.)
You either have to keep your interfaces stable,
We need the same for a static library. If we flip the API all the time, nobody could rely on the static version as well. The only difference is, for a static library, the failure becomes already obvious at link time – but it remains a failure nevertheless.
If the API is only getting extended from release to release, there's no fear of incompatibilty and versioning.
Actually, many of the AVRDUDE APIs have been quite stable in recent time.
Segregating the library backend has been a long-standing project, and I wouldn't want to give it up. It helps getting some structure into the code that hasn't been there before. Dynamic libraries are just a side-product then. If you don't like them, don't use them.
There was a problem hiding this comment.
A binary that "just works" is what I personally prefer.
So do I.
But IIRC, the executable that is built currently by using autotools is not fully static.
$ ldd `command -v avrdude`
linux-vdso.so.1 (0x00007fffeadf4000)
libusb-1.0.so.0 => /usr/lib/libusb-1.0.so.0 (0x00007f2ff5be5000)
libusb-0.1.so.4 => /usr/lib64/libusb-0.1.so.4 (0x00007f2ff5bde000)
libftdi1.so.2 => /usr/lib64/libftdi1.so.2 (0x00007f2ff5bcd000)
libelf.so.1 => /usr/lib/libelf.so.1 (0x00007f2ff5bb2000)
libpthread.so.0 => /usr/lib/libpthread.so.0 (0x00007f2ff5b91000)
libm.so.6 => /usr/lib/libm.so.6 (0x00007f2ff5a4c000)
libhidapi-libusb.so.0 => /usr/lib64/libhidapi-libusb.so.0 (0x00007f2ff5a40000)
libc.so.6 => /usr/lib/libc.so.6 (0x00007f2ff587a000)
libudev.so.1 => /usr/lib/libudev.so.1 (0x00007f2ff5852000)
/lib64/ld-linux-x86-64.so.2 => /usr/lib64/ld-linux-x86-64.so.2 (0x00007f2ff5c97000)
libz.so.1 => /usr/lib/libz.so.1 (0x00007f2ff5838000)
librt.so.1 => /usr/lib/librt.so.1 (0x00007f2ff582d000)
See? It still depends on lib{usb,ftdi1,elf,hidapi}
If we decide to make avrdude static, then we'll have to link those (and the libc too, IMO) statically. Only then shall we have an "it just works" executable.
But then, static linking of libc is another can of worms.
Last I checked, you simply couldn't statically link glibc.
But that's not what my original question is about. My original question is about dynamically linking to libavrdude.
My point was that, since we're dynamically linking to the other libraries, why not link dynamically to libavrdude?
That was the rationale behind this code -
if (sharedlib IN_LIST BUILT_LIBS)
target_link_libraries(avrdude PUBLIC sharedlib)
else()
target_link_libraries(avrdude PUBLIC staticlib)
endif()and it caused the resulting executable to be dynamically linked with libavrdude
$ ldd `command -v avrdude`
linux-vdso.so.1 (0x00007ffe321f6000)
libavrdude.so.1 => /usr/lib64/libavrdude.so.1 (0x00007fcadf372000)
libc.so.6 => /usr/lib/libc.so.6 (0x00007fcadf1ac000)
libm.so.6 => /usr/lib/libm.so.6 (0x00007fcadf067000)
libelf.so.1 => /usr/lib/libelf.so.1 (0x00007fcadf04c000)
libusb-0.1.so.4 => /usr/lib64/libusb-0.1.so.4 (0x00007fcadf045000)
libusb-1.0.so.0 => /usr/lib/libusb-1.0.so.0 (0x00007fcadf026000)
libhidapi-libusb.so.0 => /usr/lib64/libhidapi-libusb.so.0 (0x00007fcadf01a000)
libftdi1.so.2 => /usr/lib64/libftdi1.so.2 (0x00007fcadf009000)
/lib64/ld-linux-x86-64.so.2 => /usr/lib64/ld-linux-x86-64.so.2 (0x00007fcadf425000)
libz.so.1 => /usr/lib/libz.so.1 (0x00007fcadefef000)
libudev.so.1 => /usr/lib/libudev.so.1 (0x00007fcadefc7000)
libpthread.so.0 => /usr/lib/libpthread.so.0 (0x00007fcadefa6000)
librt.so.1 => /usr/lib/librt.so.1 (0x00007fcadef9b000)
There was a problem hiding this comment.
the executable that is built currently by using autotools is not fully static
Lets talk about CMake, but even with the Automake, this is per design. Jörg and I actually talked briefly about this and thought this would make the most sense. Reason being:
- On a Un*x system, we have a controlled environment, i.e. a package manager with dependency management. avrdude will be build for a specific environment, i.e. we know which shared object are present and avrdude will work with. Alternatively, it is relatively easy to build locally, where again avrdude is run in a matching environment. That is also the reason that the release artifacts do not contain any Un*x binaries, because dynamically linked executables won't just work in an arbitrary Un*x system.
- On Windows, we do not have a controlled environment. avrdude is typically distributed via a download, so we do not know the version of Windows it is run, or which or which versions of the DLLs are present. Hence, we link everything (i.e. libusb, libhid, libftdi, libelf) but the system DLLs static. The crt libraries are static as well because we would have to distribute them otherwise.
Now, with the Arduino team, we have a new request here: We want to be able to install avrdude in an environment we do not control: For instance, download the Arduino IDE and execute it on any Linux system. So now, we have the same set of problems as on Windows, and now Linux people do something similar what Windows people are used to: 1) Either make sure the dependencies are met through some sort of installer, or 2) install shared objects side-by-side with the application, or 3) link everything (but the system/crt libraries) statically.
For a standalone application like avrdude, 3) is a good choice.
There was a problem hiding this comment.
My point was that, since we're dynamically linking to the other libraries, why not link dynamically to libavrdude?
Well, if you specify BUILD_SHARED_LIBS=1 you should.
|
@dl8dtl The macOS |
|
Question: Why are you building both static and shared libraries in the same build? For CMake, I think people are very much used to doing separate builds while passing |
|
Actually, I find that strange. I'm really used to have compilations building both, dynamic libraries "for real live", static libraries for special purposes (e.g. debugging). |
I have been experimenting a bit with your branch on my private fork, and could also resolve it there by applying the respective linker options. Still, for me at least, the problem with not being able to declare global symbols as to be imported from the (future) application in the Windows DLL remained. I started to tinker a bit with rewriting the code to rework all that into a callback/configuration mode, but haven't finished. |
Probably, that is because you work mostly with automake projects? CMake people would probably claim the opposite if they can no longer tweak their builds. I think we should not make a CMake project look like an automake project and deviate from what people are used to. |
No, independent of that. All FreeBSD-built libraries have both, .a and .so versions. – They don't use automake. I think automake simply derived that habit from what used to be there before. |
I... don't know. I've never used CMake personally, so I have no idea of the conventions, if any.
Because that's what used to happen previously, i.e. when we built with autotools. And also because it makes packaging easier on Void Linux 😅 |
|
I'm not saying we should do it the way I've done in this PR. I opened this PR simply because I wanted to. It seemed like a good challenge, and an opportunity for me to dip my toes into CMake world. 😄 I'm not an avrdude developer (yet 😉) so I am not going to voice any opinion on the structural decisions of the project. |
This comment was marked as off-topic.
This comment was marked as off-topic.
So, do you suggest that we wait till that code is merged, and then fix the CMake builds? @dl8dtl |
|
It seems to be a convention that CMake by projects will tend not to build dynamic and static libraries at the same time (even though some projects like libftdi does that). Interestingly this is used as a point against CMake by Meson developer here (ignore the heated discussion -- libusb project will use auto-tools and not use CMake or Meson anytime soon). |
This is why there is BUILD_SHARED_LIBS. The reason being is that you want to be able to build different objects depending on the value of Moreover, if you build both shared and static libraries in one shot, the build options will apply to both. For instance, if you pass a compiler option that applies to static builds only, you will break the entire build because linking the shared library will break. Or vice versa. |
|
I guess I am repeating myself, but I like to see the extra code to build both static and dynamic libraries (including the platform conditional code) gone from the CMake files.
Just to avoid misunderstandings here, we should still be able to build both static and dynamic libraries, just not in one shot, for the reasons I outlined above. |
|
One more thing: I created PR #962 for everybody to review where I addressed some of the issues raised. More to come. |
|
Yes I can confirm this pull request works for building shared lib under macOS whereas #962 does not yet fix that one. |
|
It also works nicely under FreeBSD 13.0 Release. |
|
Not so sure why the author close this issue, the pull request is actually still good for macOS. Results: |
|
@subnut mentioned that he did not closed the pull requests but his comment does not appear here. Strange. |
|
@subnut I am not so sure if this is closed because you have deleted your branch. |
Try the changes from avrdudes#960
|
This PR can no longer be applied to the git main. The following is roughly the same but missing library installation. |
Closes #952
Continuation of #953, because GitHub didn't allow me to re-open that PR.