Implement support for requiring dependency version ranges

This now supports expressions in the form

  * "1.2" or ">= 1.2" for at least version 1.2
  * ">= 1.2, < 2.0" for at least version 1.2 and less than version 2.0

Fixes #60

Author: sdroege

src/lib.rs

@@ -28,6 +28,18 @@
 //! }
 //! ```
 //!
+//! # Version format
+//!
+//! Versions can be expressed in the following formats
+//!
+//!   * "1.2" or ">= 1.2": At least version 1.2
+//!   * ">= 1.2, < 2.0": At least version 1.2 but less than version 2.0
+//!
+//! In the future more complicated version expressions might be supported.
+//!
+//! Note that these versions are not interpreted according to the semver rules, but based on the
+//! rules defined by pkg-config.
+//!
 //! # Feature-specific dependency
 //! You can easily declare an optional system dependency by associating it with a feature:
 //!
@@ -159,8 +171,8 @@
 //! fn main() {
 //!     system_deps::Config::new()
 //!         .add_build_internal("testlib", |lib, version| {
-//!             // Actually build the library here
-//!             system_deps::Library::from_internal_pkg_config("build/path-to-pc-file", lib, version)
+//!             // Actually build the library here that fulfills the passed in version requirements
+//!             system_deps::Library::from_internal_pkg_config("build/path-to-pc-file", lib, "1.2.4")
 //!          })
 //!         .probe()
 //!         .unwrap();
@@ -195,6 +207,7 @@ use heck::{ToShoutySnakeCase, ToSnakeCase};
 use std::collections::HashMap;
 use std::env;
 use std::fmt;
+use std::ops::RangeBounds;
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
 
@@ -721,7 +734,18 @@ impl Config {
                 optional = dep.optional;
             } else {
                 enabled_feature_overrides.sort_by(|a, b| {
-                    version_compare::compare(&a.version, &b.version)
+                    fn min_version(r: metadata::VersionRange) -> &str {
+                        match r.start_bound() {
+                            std::ops::Bound::Unbounded => unreachable!(),
+                            std::ops::Bound::Excluded(_) => unreachable!(),
+                            std::ops::Bound::Included(b) => b,
+                        }
+                    }
+
+                    let a = min_version(metadata::parse_version(&a.version));
+                    let b = min_version(metadata::parse_version(&b.version));
+
+                    version_compare::compare(a, b)
                         .expect("failed to compare versions")
                         .ord()
                         .expect("invalid version")
@@ -758,10 +782,11 @@ impl Config {
             } else {
                 let mut config = pkg_config::Config::new();
                 config
-                    .atleast_version(version)
                     .print_system_libs(false)
                     .cargo_metadata(false)
+                    .range_version(metadata::parse_version(version))
                     .statik(statik);
+
                 match Self::probe_with_fallback(config, lib_name, fallback_lib_names) {
                     Ok((lib_name, lib)) => Library::from_pkg_config(lib_name, lib),
                     Err(e) => {
@@ -826,23 +851,55 @@ impl Config {
         }
     }
 
-    fn call_build_internal(&mut self, name: &str, version: &str) -> Result<Library, Error> {
+    fn call_build_internal(&mut self, name: &str, version_str: &str) -> Result<Library, Error> {
         let lib = match self.build_internals.remove(name) {
-            Some(f) => {
-                f(name, version).map_err(|e| Error::BuildInternalClosureError(name.into(), e))?
+            Some(f) => f(name, version_str)
+                .map_err(|e| Error::BuildInternalClosureError(name.into(), e))?,
+            None => {
+                return Err(Error::BuildInternalNoClosure(
+                    name.into(),
+                    version_str.into(),
+                ))
             }
-            None => return Err(Error::BuildInternalNoClosure(name.into(), version.into())),
         };
 
         // Check that the lib built internally matches the required version
-        match version_compare::compare(&lib.version, version) {
-            Ok(version_compare::Cmp::Lt) => Err(Error::BuildInternalWrongVersion(
+        let version = metadata::parse_version(version_str);
+        fn min_version(r: metadata::VersionRange) -> &str {
+            match r.start_bound() {
+                std::ops::Bound::Unbounded => unreachable!(),
+                std::ops::Bound::Excluded(_) => unreachable!(),
+                std::ops::Bound::Included(b) => b,
+            }
+        }
+        fn max_version(r: metadata::VersionRange) -> Option<&str> {
+            match r.end_bound() {
+                std::ops::Bound::Included(_) => unreachable!(),
+                std::ops::Bound::Unbounded => None,
+                std::ops::Bound::Excluded(b) => Some(*b),
+            }
+        }
+
+        let min = min_version(version.clone());
+        if version_compare::compare(&lib.version, min) == Ok(version_compare::Cmp::Lt) {
+            return Err(Error::BuildInternalWrongVersion(
                 name.into(),
                 lib.version,
-                version.into(),
-            )),
-            _ => Ok(lib),
+                version_str.into(),
+            ));
         }
+
+        if let Some(max) = max_version(version) {
+            if version_compare::compare(&lib.version, max) == Ok(version_compare::Cmp::Ge) {
+                return Err(Error::BuildInternalWrongVersion(
+                    name.into(),
+                    lib.version,
+                    version_str.into(),
+                ));
+            }
+        }
+
+        Ok(lib)
     }
 
     fn has_feature(&self, feature: &str) -> bool {
@@ -1008,9 +1065,9 @@ impl Library {
     /// ```
     /// let mut config = system_deps::Config::new();
     /// config.add_build_internal("mylib", |lib, version| {
-    ///   // Actually build the library here
+    ///   // Actually build the library here that fulfills the passed in version requirements
     ///   system_deps::Library::from_internal_pkg_config("build-dir",
-    ///       lib, version)
+    ///       lib, "1.2.4")
     /// });
     /// ```
     pub fn from_internal_pkg_config<P>(

src/metadata.rs

@@ -242,6 +242,14 @@ impl MetaData {
         match value {
             // somelib = "1.0"
             toml::Value::String(ref s) => {
+                if !validate_version(s) {
+                    return Err(MetadataError::UnexpectedVersionSetting(
+                        key.into(),
+                        name.into(),
+                        value.type_str().to_owned(),
+                    ));
+                }
+
                 dep.version = Some(s.clone());
             }
             toml::Value::Table(ref t) => {
@@ -267,6 +275,14 @@ impl MetaData {
                     dep.feature = Some(s.clone());
                 }
                 ("version", toml::Value::String(s)) => {
+                    if !validate_version(s) {
+                        return Err(MetadataError::UnexpectedVersionSetting(
+                            format!("{}.{}", p_key, name),
+                            key.into(),
+                            value.type_str().to_owned(),
+                        ));
+                    }
+
                     dep.version = Some(s.clone());
                 }
                 ("name", toml::Value::String(s)) => {
@@ -287,6 +303,14 @@ impl MetaData {
                     for (k, v) in version_settings {
                         match (k.as_str(), v) {
                             ("version", toml::Value::String(feat_vers)) => {
+                                if !validate_version(feat_vers) {
+                                    return Err(MetadataError::UnexpectedVersionSetting(
+                                        format!("{}.{}", p_key, name),
+                                        k.into(),
+                                        v.type_str().to_owned(),
+                                    ));
+                                }
+
                                 builder.version = Some(feat_vers.into());
                             }
                             ("name", toml::Value::String(feat_name)) => {
@@ -337,6 +361,53 @@ impl MetaData {
     }
 }
 
+fn validate_version(version: &str) -> bool {
+    if let Some((min, max)) = version.split_once(',') {
+        if !min.trim_start().starts_with(">=") || !max.trim_start().starts_with('<') {
+            return false;
+        }
+
+        true
+    } else {
+        true
+    }
+}
+
+#[derive(Debug, Clone)]
+pub(crate) enum VersionRange<'a> {
+    Range(std::ops::Range<&'a str>),
+    RangeFrom(std::ops::RangeFrom<&'a str>),
+}
+
+impl<'a> std::ops::RangeBounds<&'a str> for VersionRange<'a> {
+    fn start_bound(&self) -> std::ops::Bound<&&'a str> {
+        match self {
+            VersionRange::Range(r) => r.start_bound(),
+            VersionRange::RangeFrom(r) => r.start_bound(),
+        }
+    }
+
+    fn end_bound(&self) -> std::ops::Bound<&&'a str> {
+        match self {
+            VersionRange::Range(r) => r.end_bound(),
+            VersionRange::RangeFrom(r) => r.end_bound(),
+        }
+    }
+}
+
+pub(crate) fn parse_version(version: &str) -> VersionRange {
+    if let Some((min, max)) = version.split_once(',') {
+        // Format checked when parsing
+        let min = min.trim_start().strip_prefix(">=").unwrap().trim();
+        let max = max.trim_start().strip_prefix('<').unwrap().trim();
+        VersionRange::Range(min..max)
+    } else if let Some(min) = version.trim_start().strip_prefix(">=") {
+        VersionRange::RangeFrom(min..)
+    } else {
+        VersionRange::RangeFrom(version..)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/test.rs

@@ -110,6 +110,38 @@ cargo:rerun-if-env-changed=SYSTEM_DEPS_LINK
     );
 }
 
+#[test]
+fn version_range() {
+    let (libraries, _flags) = toml("toml-version-range", vec![]).unwrap();
+    let testlib = libraries.get_by_name("testlib").unwrap();
+    assert_eq!(testlib.version, "1.2.3");
+    assert_eq!(
+        testlib.defines.get("BADGER").unwrap().as_deref(),
+        Some("yes")
+    );
+    assert!(testlib.defines.get("AWESOME").unwrap().is_none());
+
+    let testdata = libraries.get_by_name("testdata").unwrap();
+    assert_eq!(testdata.version, "4.5.6");
+
+    assert_eq!(libraries.iter().len(), 2);
+}
+
+#[test]
+fn version_range_unsatisfied() {
+    let err = toml_err("toml-version-range-unsatisfied");
+
+    assert_matches!(err, Error::PkgConfig(_));
+
+    let err_msg = err.to_string();
+    // pkgconf and pkg-config give different error messages
+    if !err_msg.contains("Package 'testlib' has version '1.2.3', required version is '< 1.2'")
+        && !err_msg.contains("Requested 'testlib < 1.2' but version of Test Library is 1.2.3")
+    {
+        panic!("Error not as expected: {:?}", err);
+    }
+}
+
 fn toml_err(path: &str) -> Error {
     toml(path, vec![]).unwrap_err()
 }

src/tests/toml-version-range-unsatisfied/Cargo.toml

@@ -0,0 +1,3 @@
+[package.metadata.system-deps]
+testdata = "4"
+testlib = { version = ">= 1, < 1.2", feature = "test-feature" }

src/tests/toml-version-range/Cargo.toml

@@ -0,0 +1,3 @@
+[package.metadata.system-deps]
+testdata = "4"
+testlib = { version = ">= 1, < 2", feature = "test-feature" }