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

Add Meson build system #193

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Add Meson build system #193

wants to merge 4 commits into from

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Feb 3, 2024

Resurrecting #170

  • Checked that meson dist output looks reasonable.
  • Verified that build outputs the same set of files.
  • Checked that pkg-config files are equivalent.
  • Double-check that Meson build scripts match the autotools.

Fixes: #112

.travis.yml Outdated Show resolved Hide resolved
doc/meson.build Outdated Show resolved Hide resolved
meson.build Outdated
abi_revision = 0 # increment on every release
abi_current = 0 # inc when add/remove/change interfaces
abi_age = 0 # inc only if changes backward compat
# FIXME: Not correct.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what is supposed to be incorrect here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It produces the correct .so suffix so it is probably fine.

Copy link
Member

@QuLogic QuLogic Feb 5, 2024

Choose a reason for hiding this comment

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

It's wrong because that's not how libtool converts those values to soversion. It seems to work only because everything is 0.

I believe this code is the correct method. That repo also shows how you do it if you want to drop libtool-isms as well.

meson.build Show resolved Hide resolved
meson.build Outdated Show resolved Hide resolved
@jtojnar jtojnar force-pushed the meson branch 2 times, most recently from f6ea5b9 to db9ea10 Compare February 4, 2024 04:10
meson.build Show resolved Hide resolved
Comment on lines 83 to 84
# test-gegl-surface takes very long without optimizations.
timeout: 0,

Choose a reason for hiding this comment

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

Would it make sense to force optimizations then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even then, it will take ~300 sec (IIRC) rather than ~500 sec.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we should make the timeouts settable per test? There is one other that takes over 100 sec but most of them finish reasonably quickly.

Choose a reason for hiding this comment

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

Sure, makes sense.

Even for the one that takes 500 seconds, I would suggest defining that or something near it as the upper bound, so meson can still abort things if it notices the test has taken 40 minutes and counting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not look like there is an upper bound. Each time I increase it and run it few times, I get some timeouts. Maybe it depends on how busy the CPU is but I encountered it taking more than 1000 seconds, even with optimizations.

Choose a reason for hiding this comment

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

Yikes. :(

If it is too hard to figure out a reliable timeout indicator then sure, it makes sense to suppress detecting the timeout entirely.

@jtojnar jtojnar force-pushed the meson branch 3 times, most recently from 457b5fc to 8af70db Compare February 6, 2024 08:45
Since Meson uses out-of-tree builds, we need to make the paths
in the config replaceable before we can switch to Meson.
Comment on lines +103 to +109
gi = dependency(
'gobject-introspection-1.0',
version: f'>=@introspection_required_version@',
required: introspection_feature,
native: true,
)
use_introspection = gi.found()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a fan of unused dependency like this but if we just use boolean defaulting to true, it will fail due to missing glib when auto_features=disabled. And to make auto feature work as expected, we need to pass it to dependency (unfortunately, gnome.generate_gir does not have required kwarg).

QuLogic and others added 2 commits February 6, 2024 10:31
Co-authored-by: Félix Piédallu <felix@piedallu.me>
Co-authored-by: Jan Tojnar <jtojnar@gmail.com>
Use auto feature for introspection.
Comment on lines +6 to +10
# Extract translations from generated file.
# Unfortunately, there is currently no way to express the dependency
# so one will need to build the project before updating the po template.
# https://github.com/mesonbuild/meson/issues/1733
brush_settings_headers[1].full_path(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yuck. Maybe we need to just replicate the Meson’s i18n module with run_target.

Choose a reason for hiding this comment

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

Is it possible to extract the translations from the source code which is used to generate this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe gettext currently doesn't support extracting messages from JSON files. I only found one comment in the issue tracker mentioning it: https://savannah.gnu.org/bugs/?48903#comment1

I think we would need to either switch to XML, since ITS works there, or go back to listing the brushes in the Python script that generates the C header.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

CMake support
3 participants