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
th/build_meson_on_travis #54
Conversation
various options are still disabled in meson build (in fact, even with autotools, we don't build everything in our CI). Of course, we should test/build as much as possible... TODO later. |
…itdir=no The variable enable_consolekit is used below, outside the if. We always must set it.
nm_settings_docs is only defined with enable_introspection.
I think it's because meson doesn't run the tests in their own D-Bus session, hence the use the system service. automake solves this running all tests via ./tools/run-nm-test.sh, which knows how to prepare a suitable environment for the tests.
We always need to declare the linker_script_* variables, because they are used (unconditionally) as a dependency, even without have_version_script.
We also unconditionally use them with autotools. Also, the detection for have_version_script does not seem correct to me. At least, it didn't work with clang.
a16fae3
to
4bf77ea
Compare
LGTM |
@@ -44,7 +44,7 @@ option('bluez5_dun', type: 'boolean', value: false, description: 'enable Bluez5 | |||
|
|||
# configuration plugins | |||
option('config_plugins_default', type: 'string', value: '', description: 'Default configuration option for main.plugins setting, used as fallback if the configuration option is unset') | |||
option('config_plugin_ibft', type: 'boolean', value: true, description: 'enable ibft configuration plugin') | |||
option('ibft', type: 'boolean', value: false, description: 'enable ibft configuration plugin') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a couple whitespaces in front of type that would be nice to remove.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the white space is intentional, to vertically align with the neighbouring lines, that do something similar.
@@ -19,6 +13,11 @@ cflags = [ | |||
'-DNMRUNDIR="@0@"'.format(nm_pkgrundir) | |||
] | |||
|
|||
if have_fake_typelibs | |||
deps += [ gir_dep ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Square brackets are not necessary when adding a dependency to a list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed by commit 98b4653
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't aware before when modifying the meson files one by one, but looking all the changes at once, the only ld flag used over and over is the version script.
Why not create each linker script strings in the main meson.build file and reuse them when needed?
This way it take less spaces so the build files are more maintainable.
done. See commit 34cb6f9 |
Some fixes for meson build, and enable meson build on travis.
Please review