Skip to content

feat(build): Add system_stb option#3275

Open
NickHastings wants to merge 1 commit into
noctalia-dev:mainfrom
NickHastings:feature/system_stb
Open

feat(build): Add system_stb option#3275
NickHastings wants to merge 1 commit into
noctalia-dev:mainfrom
NickHastings:feature/system_stb

Conversation

@NickHastings

Copy link
Copy Markdown

Summary

This allows building against the system stb library instead of the vendored one. The option is disabled by default to retain the existing behavior.

Motivation

Many distributions have guidelines/policies recommending the usage of system copies of shared libraries whenever possible.

Debian has a very strong stance on this. From the Debian policy manual:

Some software packages include in their distribution convenience copies of code from other software packages, generally so that users compiling from source dont have to download multiple packages. Debian packages should not make use of these convenience copies unless the included package is explicitly intended to be used in this way.

Introducing an optional flag to build against the system shared library allows distros to use it if desired, while preserving the vendored approach otherwise.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Refactoring
  • Build / packaging

Testing

On Debian 13 I tested this branch using builds with the system std (meson setup .. -Dsystem_std=true) and also with the vendored third_party/std.

Note that include_type: 'system' was used to suppress some warnings from implicit conversions between integer types in both stb_image_resize2.h and stb_image_write.h.

Manual Coverage

  • Tested on Niri
  • Tested on Hyprland
  • Tested on Sway
  • Tested on another compositor:
  • Tested with different bar positions and density settings
  • Tested at different interface scaling values
  • Tested with multiple monitors

Screenshots / Videos

Checklist

  • This PR is ready for review, or it is marked as Draft.
  • I read and followed the relevant guidance in CONTRIBUTING.md.
  • I ran just format with clang-format v22+ installed, or this PR has no code changes.
  • I ran the relevant build or test commands, or explained why they were not run.
  • I self-reviewed the changes.
  • I checked for new warnings or errors.
  • I will update end-user documentation after merge, or this PR does not change user-facing configuration or behavior.
  • I added or updated assets/translations/en.json, or this PR adds no new user-facing strings.
  • I did not edit non-English translation files unless this PR is explicitly for translation tooling, an import/export sync, or a maintainer-requested locale change.
  • I used the existing canonical names for config keys, IPC names, paths, and identifiers.

This allows building against the system stb library instead of the
vendored one. The option is disabled by default to retain the default
behavior.
Comment thread meson.build
include_directories: include_directories('third_party/stb', is_system: true),
)
if get_option('system_stb')
stb_dep = dependency('stb', include_type: 'system')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

dependency relies on either pkgconfig or cmake files for detection. Debian has a pkgconfig file, but it's the result of a downstream patch. Fedora and Arch at least don't have that pkgconfig file, and probably other distros don't either. Those distros can still keep using the bundled one, but ideally the system_stb flag shouldn't be only useful on Debian.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here's an alternate approach that should be more cross-distro compatible.

Suggested change
stb_dep = dependency('stb', include_type: 'system')
stb_dep = declare_dependency(
include_directories: include_directories('/usr/include/stb', is_system: true),
)

Comment thread meson_options.txt
option('tests', type: 'feature', value: 'auto', description: 'Build unit tests (auto: on for unsanitized debug builds)')
option('system_md4c', type: 'boolean', value: false, description: 'Use system md4c instead of vendored')
option('system_tomlplusplus', type: 'boolean', value: false, description: 'Use system tomlplusplus instead of vendored')
option('system_stb', type: 'boolean', value: false, description: 'Use system tomlplusplus instead of vendored')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
option('system_stb', type: 'boolean', value: false, description: 'Use system tomlplusplus instead of vendored')
option('system_stb', type: 'boolean', value: false, description: 'Use system stb instead of vendored')

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