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

fix: adjust dynamic linking for cmake builds #1454

Merged
merged 3 commits into from
Aug 20, 2023
Merged

Conversation

lgrossi
Copy link
Contributor

@lgrossi lgrossi commented Aug 20, 2023

Description

This PR fixes the building for dynamic libraries. After #1438 CMake was always building our internal library canary_lib as a static library, regardless of the building type. Canary_lib for now is just a way of centralizing all our code to use on tests, executable and others, so it has basically everything on it (external deps, multiple internal modules, pre-compiler headers, etc).

Because of that, linking is now very slow, since it always need to link a huge static lib.

We want to allow flexibility, so if you wanna go static you can still use BUILD_STATIC_LIBRARY bo build everything statically. However, for tests and dynamic local development, it's quite handy that we provide a dynamic library as well.

image

Change

When adding the library to CMake, we now define its state based on `BUILD_STATIC_LIBRARY``.

Context

Dynamic Libraries

  • Think of them as external buddies. Your app calls on them when it runs, so they must be around.
  • They're separate files (.dll, .so, .dylib).
  • Multiple apps can share one copy, saving space.
  • But if the app expects a specific version and doesn't find it, things can break.

Static Libraries

  • They're like your app's built-in muscle. Everything's packed into the app itself.
  • No external files to worry about.
  • Makes your app bigger since it's carrying the library around.
  • Super reliable, since there's no hunting for external files during runtime.

Choosing between them?

If you want a single, beefy file with no external dependencies, go static.
If you're cool with external files and like sharing code between apps, go dynamic.

Bonus

We're breaking down target_source into smaller chunks to improve visibility.

@lgrossi lgrossi force-pushed the lucas/fix-dynamic-linking branch 4 times, most recently from 627c7ea to ba75921 Compare August 20, 2023 10:58
@lgrossi lgrossi force-pushed the lucas/fix-dynamic-linking branch 2 times, most recently from a837de1 to 6463e05 Compare August 20, 2023 10:59
@lgrossi lgrossi force-pushed the lucas/fix-dynamic-linking branch from 6463e05 to a351a94 Compare August 20, 2023 11:07
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@dudantas dudantas changed the title Lucas/fix dynamic linking fix: cmake dynamic linking for linux Aug 20, 2023
@lgrossi lgrossi merged commit c84cffd into main Aug 20, 2023
@lgrossi lgrossi deleted the lucas/fix-dynamic-linking branch August 20, 2023 17:17
@lgrossi lgrossi changed the title fix: cmake dynamic linking for linux fix: setup dynamic linking for cmake builds Aug 20, 2023
@lgrossi lgrossi changed the title fix: setup dynamic linking for cmake builds fix: adjust dynamic linking for cmake builds Aug 20, 2023
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

Successfully merging this pull request may close these issues.

3 participants