Skip to content

meson.build fixes #1225

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

meson.build fixes #1225

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
432c1ec
meson.build: Set the correct version 0.10.1 -> 0.11.0
BalkanMadman Nov 1, 2025
82ecdd0
meson.build: Do not hardcode default_library=static
BalkanMadman Nov 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
project(
'quickjs-ng',
'c',
version: '0.10.1',
version: '0.11.0',
default_options: [
'c_std=gnu11,c11',
'warning_level=3',
'default_library=static',
Copy link
Contributor

@saghul saghul Nov 1, 2025

Choose a reason for hiding this comment

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

Does the override work even if this is the default?

Copy link
Author

@BalkanMadman BalkanMadman Nov 4, 2025

Choose a reason for hiding this comment

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

Yes, it does. However, the most use-cases I can think of (distributions, individuals) require shared libraries so hardcoding default_library=static seems like a bug to me.

Copy link
Author

@BalkanMadman BalkanMadman Nov 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Considering that the default for meson is default_library=shared, I don't really see a reason to override it in meson.build. Does your CI require it?

Copy link
Contributor

@saghul saghul Nov 4, 2025

Choose a reason for hiding this comment

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

Yes, that's likely why it failed.

In addition our CMake setup defaults to static builds, so Meson should align to that.

Copy link
Contributor

@bnoordhuis bnoordhuis Nov 4, 2025

Choose a reason for hiding this comment

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

I'd be okay with using meson defaults. cmake's default is more an accident of history that we inherit, I think?

Copy link
Contributor

@bnoordhuis bnoordhuis Nov 4, 2025

Choose a reason for hiding this comment

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

does it make sense to build static libraries by default (in either CMake or meson)

In cmake it's controlled by a flag (-BUILD_SHARED_LIBS=ON or off), it's just that cmake defaults to static if not otherwise specified. Our CMakeListst.txt is agnostic as to whether it should produce static or shared libraries.

(Long-winded way of saying that whatever the cmake build does is not under discussion, it just is.)

Copy link
Contributor

@saghul saghul Nov 4, 2025

Choose a reason for hiding this comment

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

I understand. The question is then does it make sense to build static libraries by default (in either CMake or meson)? I'd imagine most consumers of quickjs need shared libqjs.so rather than static libqjs.a.

I'd argue it's the opposite. Every tool I've seen out there installing JS engines like jsvu makes static builds.

Copy link
Author

@BalkanMadman BalkanMadman Nov 4, 2025

Choose a reason for hiding this comment

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

Well, as far as I understand, tools installing JS engines mostly care about engine executables, not the provided lib. Obviously, the type of libqjs doesn't have influence on the executables.

For the consumers of the actual QuickJS-NG API, i.e. libqjs, it'd most likely be shared not static libraries they'd need.

In any case, that's not the hill to die on. I'm just arguing that, in my opinion, the consumers of your library API would prefer a shared lib, as with most other APIs out there.

The choice is up to you anyways.

Copy link
Contributor

@saghul saghul Nov 5, 2025

Choose a reason for hiding this comment

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

Ah sorry my bad. As long as building the executable statically links the library by default I don't think I care much about what the default for building a library is.

We haven't called a 1.0 yet so I think we have plenty of wiggle room to try things out.

Copy link
Author

@BalkanMadman BalkanMadman Nov 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If the shared library is built, then the executable is linked dynamically to it via executable(..., dependencies: qjs_dep).

For CI, the static (or shared) lib build can always be forced via the cmdline switch -Ddefault_library=(static|shared)

],
license: 'MIT',
license_files: 'LICENSE',
Expand Down
Loading