Skip to content

Conversation

dunglas
Copy link

@dunglas dunglas commented Nov 12, 2024

To prevent issues such as php/frankenphp#1156 (comment)

Copy link
Owner

@e-dant e-dant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

I think this is better placed in the "Consume" section, and can be linked to in the C and C++ Quick Start sections.

My hesitation with having it front and center is that folks might be using this project in a variety of languages, or integrating into various languages. C/C++ isn't necessarily the most important information to folks.

Thoughts?

@dunglas
Copy link
Author

dunglas commented Nov 13, 2024

I may be mistaken but you'll need the system library installed regardless of the language binding you use, right? Or are other languages downloading or compiling the C++ library automatically?

@e-dant
Copy link
Owner

e-dant commented Nov 13, 2024

Python bundles it into the package and loads it with dlopen at runtime; Node/Javascript is a native addon, which seems to be kind of (exactly?) a shared library, and it's bundled up in their package wrapping, too.

We have Python wheels for a dozen or so platform/architecture/libc triples.

@dunglas
Copy link
Author

dunglas commented Nov 13, 2024

So they ship their own .so or compile it at install?

While it can be convenient, it is less efficient and harder to secure than reusing an existing global system library (as libcurl for instance).

First, if you have let's say a JS, a Python and a C program using watcher, the library will be loaded 3 times in memory instead of one.

Secondly, it will be a headache for distributions. Package managers (even Homebrew), always prefer shipping libraries like this as a common dependency between all packages using it than shipping a copy in each package. It's more efficient (see 1), but also easier to maintain and secure because you only have to update one package (the library) when it is upgraded instead of N.

IMHO, bindings should prefer using the global system library if available and fallback to compiling/downloading their own copies only if not present. AFAIK, this is also the common practice for similar libraries such as SQLite or libcurl.

@e-dant
Copy link
Owner

e-dant commented Nov 13, 2024

I’m open to preferring (or mandating) that a global system library is used, but there are some tradeoffs

  • Unintentionally breaking backwards compatibility is a pain. Versioning the shared libraries (and their symbols, on Linux) only goes as far as helping to smooth over intentional ABI breaks.

  • Asking people to setup clang/msvc/gcc toolchains, and optionally either meson or cmake, is a pain. This could be helped by asking people to instead download the library off our GitHub release page. (And extracting it correctly.)

  • Our library is around 100k. On Darwin it’s about 60k. It’s really small. For comparison, the readme itself is almost 30k.


All that said, I can appreciate the points you make. Especially about securing it. If there is a security advisory, updating stuff uniformly would be a great relief. Still, that depends a bit on a package manager nudging folks to do it.

Perhaps this is better to prefer as a system-wide library if package managers pick us up? (We currently don’t have them.)

@e-dant
Copy link
Owner

e-dant commented Nov 13, 2024

So they ship their own .so or compile it at install?

Python ships its own .so, node/js is a “native addon” (no libwatcher-c.so involved)

@dunglas
Copy link
Author

dunglas commented Nov 13, 2024

Maybe could we have both? Bindings could check if a global installation is available, and fallback on a local install if it's not the case?

@dunglas
Copy link
Author

dunglas commented Nov 13, 2024

But I get your point. Even for FrankenPHP, we could just vendor the library (or ship precompiled .a files, even if I don't like this solution as it will fail on exotic systems), that would be simpler for end users, but harder to maintain for us (we'll have to publish a new release each time the library is updated or contains a security fix).

@e-dant
Copy link
Owner

e-dant commented Nov 13, 2024

Maybe could we have both? Bindings could check if a global installation is available, and fallback on a local install if it's not the case?

Yeah, that sounds good to me

@e-dant
Copy link
Owner

e-dant commented Nov 13, 2024

Node and Rust could prefer a system library at build-time with some careful linker arguments. I need to see how exactly libraries are resolved currently if identically named libwatcher-c's exist, but at different paths (for ex, one in the build dir, another in /usr)

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.

2 participants