Skip to content

qt-build-utils can invoke qmake when it is a script #1284

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: main
Choose a base branch
from
+19 −6
Open

qt-build-utils can invoke qmake when it is a script #1284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
031f080
qt-build-utils: support qmake wrapper scripts
SanderVocke Jun 11, 2025
7db4d04
Merge branch 'main' into support_qmake_scripts
SanderVocke Jun 20, 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
10 changes: 4 additions & 6 deletions crates/qt-build-utils/src/installation/qmake.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,8 @@ impl TryFrom<PathBuf> for QtInstallationQMake {

fn try_from(qmake_path: PathBuf) -> anyhow::Result<Self> {
// Attempt to read the QT_VERSION from qmake
let qmake_version = match Command::new(&qmake_path)
.args(["-query", "QT_VERSION"])
.output()
{
let qmake_command: String = format!("{} -query QT_VERSION", qmake_path.to_string_lossy());
let qmake_version = match utils::native_shell_command(&qmake_command).output() {
Err(e) if e.kind() == ErrorKind::NotFound => Err(QtBuildError::QtMissing),
Err(e) => Err(QtBuildError::QmakeFailed(e)),
Ok(output) if !output.status.success() => Err(QtBuildError::QtMissing),
Expand Down Expand Up @@ -373,9 +371,9 @@ impl QtInstallationQMake {
}

fn qmake_query(&self, var_name: &str) -> String {
let qmake_command = format!("{} -query {}", self.qmake_path.to_string_lossy(), var_name);
String::from_utf8_lossy(
&Command::new(&self.qmake_path)
.args(["-query", var_name])
&utils::native_shell_command(&qmake_command)
.output()
.unwrap()
.stdout,
Expand Down
15 changes: 15 additions & 0 deletions crates/qt-build-utils/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
//
// SPDX-License-Identifier: MIT OR Apache-2.0

use std::process::Command;

/// Whether apple is the current target
pub(crate) fn is_apple_target() -> bool {
std::env::var("TARGET")
Expand All @@ -14,3 +16,16 @@ pub(crate) fn is_apple_target() -> bool {
pub(crate) fn is_emscripten_target() -> bool {
std::env::var("CARGO_CFG_TARGET_OS") == Ok("emscripten".to_owned())
}

/// Wrap a command in a native subshell
pub(crate) fn native_shell_command(command: &str) -> Command {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe passing the command as a plain string here will fail if the path to qmake contains a space.

As this path is provided by the user, we must ensure this works as well.
Please make sure that we can handle paths with spaces.

I suggest adding a testcase for this by adding a test scripts/ folder that contains a qmake.sh and qmake.bat script that just echo a string or forward to qmake.

let mut result: Command;
if cfg!(target_os = "windows") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we do this at build time with #[cfg(...)] rather than at runtime with cfg! ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ahayzen-kdab I actually prefer sticking with cfg! here.
AFAIK, the cfg! macro evaluates to a bool literal.
This should allow the compiler to optimize out the branches that are not used by the current cfg, but will still check that the alternative branches compile.

With #[cfg()] if you compile on Linux you will not notice if the windows part of the branch has an error that could be caught at compile-time.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess, just elsewhere where we have similar things they are at compile time and in theory CI picks up any Windows failures :-) But sure we can keep it this way for now...

result = Command::new("cmd");
result.args(["/C", command]);
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use an else if that actively lists the operating systems that we expect to have an sh binary and fall back to a plain Command in the other cases, as otherwise this will break on any OS that does not have an sh binary.

result = Command::new("sh");
result.args(["-c", command]);
}
result
}
Loading