qt-build-utils: auto pick qmake or qt minimal (#1424)

* qt_minimal: have a common method to filter by version, arch, os

* qt_minimal: add grouping of artifacts and pick latest local install

* qt-build-utils: enable qt_version features when qt_minimal is enabled

* qt-build-utils: add getter to access the inner QtInstallation

* qt_minimal: add various cargo rerun-if lines

* qt-build-utils: bump qt-version to 0.1.3

* qt-build-utils: use which to find full qmake path

* qt-build-utils: ensure that Command has clean environment

Otherwise when executing Qt binaries if LD_LIBRARY_PATH is set
collisions can occur with differing Qt binary and libraries.

Author: ahayzen-kdab

Cargo.lock

@@ -26,7 +26,8 @@ tempfile = { version = "3.25.0", optional = true }
 tar = { version = "0.4.44", optional = true }
 flate2 = { version = "1.1.9", optional = true }
 qt-artifacts = { version = "0.1.0", optional = true }
-qt-version = { version = "0.1.2", optional = true }
+qt-version = { version = "0.1.3", default-features = false, optional = true }
+which = { version = "8.0.0", optional = true }
 
 [features]
 # TODO: should we default to qmake or let downstream crates specify, such as cxx-qt-build
@@ -42,8 +43,8 @@ default = ["qmake"]
 #
 # When linking Qt dynamically, this makes no difference.
 link_qt_object_files = []
-qmake = []
-qt_minimal = ["dep:serde", "serde/derive", "semver/serde", "dep:reqwest", "dep:sha2", "dep:tempfile", "dep:serde_json", "dep:flate2", "dep:tar", "dep:qt-artifacts", "dep:dirs"]
+qmake = ["dep:which"]
+qt_minimal = ["qt_version", "dep:serde", "serde/derive", "semver/serde", "dep:reqwest", "dep:sha2", "dep:tempfile", "dep:serde_json", "dep:flate2", "dep:tar", "dep:qt-artifacts", "dep:dirs"]
 qt_version = ["dep:qt-version"]
 serde = ["dep:serde"]
 

crates/qt-build-utils/Cargo.toml

@@ -74,16 +74,20 @@ impl QtInstallationQMake {
             // Use the first non-errored installation
             // If there are no valid installations we display the last error
             .fold(None, |acc, qmake_path| {
-                Some(acc.map_or_else(
-                    // Value is None so try to create installation
-                    || QtInstallationQMake::try_from(PathBuf::from(qmake_path)),
-                    // Value is Some so pass through or create if Err
-                    |prev: anyhow::Result<Self>| {
-                        prev.or_else(|_|
+                if let Ok(qmake_path) = which::which(qmake_path) {
+                    Some(acc.map_or_else(
+                        // Value is None so try to create installation
+                        || QtInstallationQMake::try_from(qmake_path.clone()),
+                        // Value is Some so pass through or create if Err
+                        |prev: anyhow::Result<Self>| {
+                            prev.or_else(|_|
                             // Value is Err so try to create installation
-                            QtInstallationQMake::try_from(PathBuf::from(qmake_path)))
-                    },
-                ))
+                            QtInstallationQMake::try_from(qmake_path.clone()))
+                        },
+                    ))
+                } else {
+                    None
+                }
             })
             .unwrap_or_else(|| Err(QtBuildError::QtMissing.into()))
     }
@@ -114,6 +118,9 @@ impl TryFrom<PathBuf> for QtInstallationQMake {
         // Attempt to read the QT_VERSION from qmake
         let qmake_version = match Command::new(&qmake_path)
             .args(["-query", "QT_VERSION"])
+            // Binaries should work without environment and this prevents
+            // LD_LIBRARY_PATH from causing different Qt version clashes
+            .env_clear()
             .output()
         {
             Err(e) if e.kind() == ErrorKind::NotFound => Err(QtBuildError::QtMissing),
@@ -207,6 +214,9 @@ impl QtInstallationQMake {
         String::from_utf8_lossy(
             &Command::new(&self.qmake_path)
                 .args(["-query", var_name])
+                // Binaries should work without environment and this prevents
+                // LD_LIBRARY_PATH from causing different Qt version clashes
+                .env_clear()
                 .output()
                 .unwrap()
                 .stdout,
@@ -263,7 +273,12 @@ impl QtInstallationQMake {
         // Find the first valid executable path
         .find_map(|qmake_query_var| {
             let executable_path = PathBuf::from(self.qmake_query(qmake_query_var)).join(tool_name);
-            let test_output = Command::new(&executable_path).args(["-help"]).output();
+            let test_output = Command::new(&executable_path)
+                .args(["-help"])
+                // Binaries should work without environment and this prevents
+                // LD_LIBRARY_PATH from causing different Qt version clashes
+                .env_clear()
+                .output();
             match test_output {
                 Err(_err) => {
                     failed_paths.push(executable_path);

crates/qt-build-utils/src/installation/qmake.rs

@@ -8,6 +8,7 @@ mod checksum;
 mod download;
 mod extract;
 
+use std::collections::HashMap;
 use std::fs::DirEntry;
 use std::path::{Path, PathBuf};
 
@@ -24,6 +25,8 @@ impl TryFrom<PathBuf> for QtInstallationQtMinimal {
     type Error = anyhow::Error;
 
     fn try_from(path_qt: PathBuf) -> Result<Self, Self::Error> {
+        println!("cargo::rerun-if-changed={}", path_qt.display());
+
         // Verify that the expected folders exist
         for folder in ["bin", "include", "lib", "libexec"] {
             if !path_qt.join(folder).exists() {
@@ -70,29 +73,8 @@ impl TryFrom<semver::Version> for QtInstallationQtMinimal {
         let manifest: artifact::ParsedQtManifest =
             serde_json::from_str(qt_artifacts::QT_MANIFEST_JSON)?;
 
-        // Find artifacts matching Qt version
-        //
-        // Arch could be x86_64
-        // OS could be linux
-        // https://doc.rust-lang.org/cargo/appendix/glossary.html#target
-        //
-        // TODO: is there a better way to find the arch and os ?
-        // and should this be configurable via env var overrides?
-        let target = std::env::var("TARGET").expect("TARGET to be set");
-        let target_parts: Vec<_> = target.split("-").collect();
-        let arch = target_parts
-            .first()
-            .expect("TARGET to have a <arch><sub> component");
-        let os = target_parts
-            .get(2)
-            .expect("TARGET to have a <sys> component");
-        let artifacts: Vec<ParsedQtArtifact> = manifest
-            .artifacts
-            .into_iter()
-            .filter(|artifact| {
-                artifact.arch == *arch && artifact.os == *os && artifact.version == version
-            })
-            .collect();
+        // Find artifacts for the Qt version
+        let artifacts = Self::match_artifact_requirements(manifest.artifacts, &[version.clone()]);
 
         // Find the first bin / include
         let artifact_bin = artifacts
@@ -110,8 +92,8 @@ impl TryFrom<semver::Version> for QtInstallationQtMinimal {
                 "{}.{}.{}",
                 version.major, version.minor, version.patch
             ))
-            .join(os)
-            .join(arch);
+            .join(&artifact_bin.os)
+            .join(&artifact_bin.arch);
         artifact_bin.download_and_extract(&extract_target_dir);
         if artifact_bin != artifact_include {
             artifact_include.download_and_extract(&extract_target_dir);
@@ -174,6 +156,7 @@ impl QtInstallation for QtInstallationQtMinimal {
 impl QtInstallationQtMinimal {
     fn qt_minimal_root() -> PathBuf {
         // Check if a custom root has been set
+        println!("cargo::rerun-if-env-changed=QT_MINIMAL_DOWNLOAD_ROOT");
         let path = if let Ok(root) = std::env::var("QT_MINIMAL_DOWNLOAD_ROOT") {
             PathBuf::from(root)
         } else {
@@ -183,14 +166,18 @@ impl QtInstallationQtMinimal {
                 .join("qt_minimal_download")
         };
 
-        std::fs::create_dir_all(&path).expect("Could not create Qt minimal root path");
+        // Only create when it doesn't exist to avoid file modifications
+        if !path.exists() {
+            std::fs::create_dir_all(&path).expect("Could not create Qt minimal root path");
+        }
 
         path
     }
 
     /// Get a collection of the locally installed Qt artifacts
     pub(crate) fn local_artifacts() -> anyhow::Result<Vec<ParsedQtArtifact>> {
         let base_dir = Self::qt_minimal_root();
+        println!("cargo::rerun-if-changed={}", base_dir.display());
 
         // Expects folder structure like:
         // version/os/arch/qt/{bin, include}
@@ -199,17 +186,23 @@ impl QtInstallationQtMinimal {
 
         // Iterate versions
         for version in list_dirs(&base_dir) {
+            println!("cargo::rerun-if-changed={}", version.path().display());
+
             let path = version;
             // TODO: Later skip unknown folders,
             // this will error if a directory exists which isn't a version number
             let semver = semver::Version::parse(path.file_name().to_str().unwrap())
                 .expect("Could not parse semver from directory name");
 
             for os in list_dirs(&path.path()) {
+                println!("cargo::rerun-if-changed={}", os.path().display());
+
                 let path = os;
                 let os = path.file_name().to_str().unwrap().to_string();
 
                 for arch in list_dirs(&path.path()) {
+                    println!("cargo::rerun-if-changed={}", arch.path().display());
+
                     let path = arch;
                     let dir_entries = list_dirs(&path.path());
 
@@ -219,6 +212,7 @@ impl QtInstallationQtMinimal {
                         .filter(|dir| dir.file_name() == "qt")
                         .last()
                         .expect("Expected to find a Qt dir in this folder");
+                    println!("cargo::rerun-if-changed={}", qt_dir_path.path().display());
 
                     let qt_folders = list_dirs(&qt_dir_path.path());
                     for dir in qt_folders {
@@ -248,6 +242,64 @@ impl QtInstallationQtMinimal {
 
         Ok(artifacts)
     }
+
+    /// Find artifacts matching Qt version
+    pub(crate) fn match_artifact_requirements(
+        artifacts: Vec<ParsedQtArtifact>,
+        versions: &[semver::Version],
+    ) -> Vec<ParsedQtArtifact> {
+        // Arch could be x86_64
+        // OS could be linux
+        // https://doc.rust-lang.org/cargo/appendix/glossary.html#target
+        //
+        // TODO: is there a better way to find the arch and os ?
+        // and should this be configurable via env var overrides?
+        println!("cargo::rerun-if-env-changed=TARGET");
+        let target = std::env::var("TARGET").expect("TARGET to be set");
+        let target_parts: Vec<_> = target.split("-").collect();
+        let arch = target_parts
+            .first()
+            .expect("TARGET to have a <arch><sub> component");
+        let os = target_parts
+            .get(2)
+            .expect("TARGET to have a <sys> component");
+
+        artifacts
+            .into_iter()
+            .filter(|artifact| {
+                artifact.arch == *arch && artifact.os == *os && versions.contains(&artifact.version)
+            })
+            .collect()
+    }
+
+    /// Merge together artifacts with the same version
+    /// so that we do not have bin/ and include/ split
+    //
+    // NOTE: later we may support bin and include folders being in different places
+    pub(crate) fn group_artifacts(artifacts: Vec<ParsedQtArtifact>) -> Vec<ParsedQtArtifact> {
+        artifacts.into_iter().fold(
+                            HashMap::<semver::Version, ParsedQtArtifact>::default(),
+                            |mut acc, mut artifact| {
+                                acc.entry(artifact.version.clone())
+                                    .and_modify(|value| {
+                                        if value.url == artifact.url {
+                                            value.content.append(&mut artifact.content)
+                                        } else {
+                                            println!("cargo::warning=Found multiple minimal installations of the same version but different urls: {} and {}", value.url, artifact.url);
+                                        }
+                                    })
+                                    .or_insert(artifact);
+                                acc
+                            },
+                        )
+            .into_values()
+            // Ensure that artifacts contain bin/ and include/
+            .filter(|artifact| {
+                artifact.content.contains(&"bin".to_string())
+                    && artifact.content.contains(&"include".to_string())
+            })
+            .collect()
+    }
 }
 
 /// Get all valid directories in path bufs, ignoring errors