`dep.get_variable()` for qmake
dependencies/configtool: customizable tool args
factor out ConfigToolDependency.get_configtool_args_for_variable() from ConfigToolDependency.get_configtool_variable()
This facilitates getting variables from config tools with non-standard argument syntax, both in Python and in Meson's specification language (via dep.get_variable() or dep.get_configtool_variable()), avoiding duplication of error handling & further parsing of tool results that would come from overriding the whole ConfigToolDependency.get_configtool_variable() function.
dependencies/qt: dep.get_variable() for qmake
To get config tool variables with qmake, qmake --{variable_name} cannot be used; instead, qmake -query {variable_name} must be used.
This customizes ConfigToolDependency.get_configtool_args_for_variable() via QmakeQtDependency.get_configtool_args_for_variable() to support dep.get_variable() and dep.get_configtool_variable() for qmake dependencies.
Addresses https://github.com/mesonbuild/meson/issues/8490. For example, qt5_dep.get_variable(configtool : 'QT_INSTALL_PLUGINS) as suggested by @goulf-3m actually works now.
Follow up to https://github.com/mesonbuild/meson/pull/8568.
Refactoring QmakeQtDependency.__init__() to use self.get_configtool_variable() or self.get_configtool_args_for_variable() was considered, but given that it's a special use of self.get_config_value() that eagerly queries all variables at once, which self.get_configtool_variable() doesn't currently support, the function was left alone.
The changes themselves look reasonable enough, but the commit messages are... very information-dense.
Describing the exact implementation classes being modified is generally not necessary, and may detract from readability in git log. This information is already available in the change diff, and doesn't aid the reader in the meta-knowledge of what effective difference the change makes, and why, nor does it add contextual information lacking in the diff itself.
Addresses <https://github.com/mesonbuild/meson/issues/8490> in the middle of a paragraph should be removed, and replaced by Fixes #8490 in its own paragraph at the end of the commit message. This avoids stray <> in the github UI, because raw links are expected by the renderer, and matches the expected pattern for github's auto-linkification keywords (this ensures that merging the PR marks the issue as closed).
Codecov Report
Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
Project coverage is 67.08%. Comparing base (
a2934ca) to head (0bf9ac4). Report is 2897 commits behind head on master.
| Files | Patch % | Lines |
|---|---|---|
| mesonbuild/dependencies/qt.py | 66.66% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #9728 +/- ##
==========================================
- Coverage 67.08% 67.08% -0.01%
==========================================
Files 404 404
Lines 85711 85719 +8
Branches 18907 18907
==========================================
+ Hits 57501 57506 +5
- Misses 23676 23679 +3
Partials 4534 4534
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
does this correctly support qmake --qtconf '<somefullpath' --query <variablename> ?
The changes themselves look reasonable enough, but the commit messages are... very information-dense.
Describing the exact implementation classes being modified is generally not necessary, and may detract from readability in
git log. This information is already available in the change diff, and doesn't aid the reader in the meta-knowledge of what effective difference the change makes, and why, nor does it add contextual information lacking in the diff itself.
Addresses <https://github.com/mesonbuild/meson/issues/8490>in the middle of a paragraph should be removed, and replaced byFixes #8490in its own paragraph at the end of the commit message. This avoids stray<>in the github UI, because raw links are expected by the renderer, and matches the expected pattern for github's auto-linkification keywords (this ensures that merging the PR marks the issue as closed).
@eli-schwartz I'll shorten those up; figured I'd lean generous on commit messages rather than possibly have to write up comprehensive ones later. :)
does this correctly support
qmake --qtconf '<somefullpath' --query <variablename>?
@Neumann-A I've tested only on Qt 5.15.7; --query is an invalid option for qmake. ~/Qt/5.15.7/clang_64/bin/qmake -query QT_INSTALL_LIBS -qtconf ~/Qt/5.15.7/clang_64/bin/qt.conf works as expected; ~/Qt/5.15.7/clang_64/bin/qmake -qtconf ~/Qt/5.15.7/clang_64/bin/qt.conf -query QT_INSTALL_LIBS exits with code 1, prints ***Unknown option -query to stdout, and prints help to stderr; ~/Qt/5.15.7/clang_64/bin/qmake --query QT_INSTALL_LIBS --qtconf ~/Qt/5.15.7/clang_64/bin/qt.conf exits with code 1, prints ***Unknown option --query to stdout, and prints help to stderr.
-qtconf <file> is currently not used by QmakeQtDependency; support for this option can be implemented in a separate patch set.
If any of this is preferable in Qt 6 or required in Qt 6, patches are welcome here.
I think we're still just waiting on a resolution to @dcbaker's review here (and ideally a tweak to the commit message wording). Any news on an update?
@bb010g still interested in finishing this PR? Seems rather close to being done.
Codecov Report
Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
Project coverage is 67.08%. Comparing base (
a2934ca) to head (0bf9ac4). Report is 3038 commits behind head on master.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| mesonbuild/dependencies/qt.py | 66.66% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #9728 +/- ##
==========================================
- Coverage 67.08% 67.08% -0.01%
==========================================
Files 404 404
Lines 85711 85719 +8
Branches 18907 18907
==========================================
+ Hits 57501 57506 +5
- Misses 23676 23679 +3
Partials 4534 4534
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.