pyo3-build-config: increased test coverage

.github/workflows/ci.yml

@@ -42,16 +42,33 @@ jobs:
     name: python${{ matrix.python-version }}-${{ matrix.platform.python-architecture }} ${{ matrix.platform.os }} ${{ matrix.msrv }}
     runs-on: ${{ matrix.platform.os }}
     strategy:
-      fail-fast: false  # If one platform fails, allow the rest to keep testing.
+      fail-fast: false # If one platform fails, allow the rest to keep testing.
       matrix:
         rust: [stable]
         python-version: [3.6, 3.7, 3.8, 3.9, 3.10-dev, pypy-3.6, pypy-3.7]
-        platform: [
+        platform:
-          { os: "macos-latest",   python-architecture: "x64", rust-target: "x86_64-apple-darwin" },
+          [
-          { os: "ubuntu-latest", python-architecture: "x64", rust-target: "x86_64-unknown-linux-gnu" },
+            {
-          { os: "windows-latest", python-architecture: "x64", rust-target: "x86_64-pc-windows-msvc" },
+              os: "macos-latest",
-          { os: "windows-latest", python-architecture: "x86", rust-target: "i686-pc-windows-msvc" },
+              python-architecture: "x64",
-        ]
+              rust-target: "x86_64-apple-darwin",
+            },
+            {
+              os: "ubuntu-latest",
+              python-architecture: "x64",
+              rust-target: "x86_64-unknown-linux-gnu",
+            },
+            {
+              os: "windows-latest",
+              python-architecture: "x64",
+              rust-target: "x86_64-pc-windows-msvc",
+            },
+            {
+              os: "windows-latest",
+              python-architecture: "x86",
+              rust-target: "i686-pc-windows-msvc",
+            },
+          ]
         exclude:
           # There is no 64-bit pypy on windows for pypy-3.6
           - python-version: pypy-3.6
@@ -63,7 +80,12 @@ jobs:
           # Test minimal supported Rust version
           - rust: 1.41.1
             python-version: 3.9
-            platform: { os: "ubuntu-latest", python-architecture: "x64", rust-target: "x86_64-unknown-linux-gnu" }
+            platform:
+              {
+                os: "ubuntu-latest",
+                python-architecture: "x64",
+                rust-target: "x86_64-unknown-linux-gnu",
+              }
             msrv: "MSRV"
 
     steps:
@@ -146,6 +168,9 @@ jobs:
       - name: Test proc-macro code
         run: cargo test --manifest-path=pyo3-macros-backend/Cargo.toml
 
+      - name: Test build config
+        run: cargo test --manifest-path=pyo3-build-config/Cargo.toml
+
       - name: Install python test dependencies
         run: python -m pip install -U pip tox
 
@@ -193,8 +218,10 @@ jobs:
           override: true
           profile: minimal
           components: llvm-tools-preview
-      - run: LLVM_PROFILE_FILE="coverage-%p-%m.profraw" cargo test --no-default-features --no-fail-fast
+      - run: cargo test --no-default-features --no-fail-fast
-      - run: LLVM_PROFILE_FILE="coverage-features-%p-%m.profraw" cargo test --no-default-features --no-fail-fast --features "macros num-bigint num-complex hashbrown serde multiple-pymethods"
+      - run: cargo test --no-default-features --no-fail-fast --features "macros num-bigint num-complex hashbrown serde multiple-pymethods"
+      - run: cargo test --manifest-path=pyo3-macros-backend/Cargo.toml
+      - run: cargo test --manifest-path=pyo3-build-config/Cargo.toml
       # can't yet use actions-rs/grcov with source-based coverage: https://github.com/actions-rs/grcov/issues/105
       # - uses: actions-rs/grcov@v0.1
       #   id: coverage
@@ -210,3 +237,4 @@ jobs:
       CARGO_TERM_VERBOSE: true
       RUSTFLAGS: "-Zinstrument-coverage"
       RUSTDOCFLAGS: "-Zinstrument-coverage"
+      LLVM_PROFILE_FILE: "coverage-%p-%m.profraw"

pyo3-build-config/src/impl_.rs

@@ -135,16 +135,24 @@ impl InterpreterConfig {
 
     #[doc(hidden)]
     pub fn to_writer(&self, mut writer: impl Write) -> Result<()> {
+        macro_rules! writeln_option {
+            ($writer:expr, $opt:expr) => {
+                match &$opt {
+                    Some(value) => writeln!($writer, "{}", value),
+                    None => writeln!($writer, "null"),
+                }
+            };
+        }
         writeln!(writer, "{}", self.version.major)?;
         writeln!(writer, "{}", self.version.minor)?;
-        writeln!(writer, "{:?}", self.libdir)?;
+        writeln_option!(writer, self.libdir)?;
         writeln!(writer, "{}", self.shared)?;
         writeln!(writer, "{}", self.abi3)?;
-        writeln!(writer, "{:?}", self.ld_version)?;
+        writeln_option!(writer, self.ld_version)?;
-        writeln!(writer, "{:?}", self.base_prefix)?;
+        writeln_option!(writer, self.base_prefix)?;
-        writeln!(writer, "{:?}", self.executable)?;
+        writeln_option!(writer, self.executable)?;
-        writeln!(writer, "{:?}", self.calcsize_pointer)?;
+        writeln_option!(writer, self.calcsize_pointer)?;
-        writeln!(writer, "{:?}", self.implementation)?;
+        writeln!(writer, "{}", self.implementation)?;
         for flag in &self.build_flags.0 {
             writeln!(writer, "{}", flag)?;
         }
@@ -156,12 +164,10 @@ fn parse_option_string<T: FromStr>(string: String) -> Result<Option<T>>
 where
     <T as FromStr>::Err: std::error::Error + 'static,
 {
-    if string == "None" {
+    if string == "null" {
         Ok(None)
-    } else if string.starts_with("Some(") && string.ends_with(')') {
-        Ok(string[5..(string.len() - 1)].parse().map(Some)?)
     } else {
-        Err("expected None or Some(value)".into())
+        Ok(string.parse().map(Some)?)
     }
 }
 
@@ -187,6 +193,15 @@ pub enum PythonImplementation {
     PyPy,
 }
 
+impl Display for PythonImplementation {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            PythonImplementation::CPython => write!(f, "CPython"),
+            PythonImplementation::PyPy => write!(f, "PyPy"),
+        }
+    }
+}
+
 impl FromStr for PythonImplementation {
     type Err = Box<dyn std::error::Error>;
     fn from_str(s: &str) -> Result<Self> {
@@ -311,7 +326,10 @@ pub enum BuildFlag {
 
 impl Display for BuildFlag {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{:?}", self)
+        match self {
+            BuildFlag::Other(flag) => write!(f, "{}", flag),
+            _ => write!(f, "{:?}", self),
+        }
     }
 }
 
@@ -333,7 +351,10 @@ impl FromStr for BuildFlag {
 /// we will pick up and pass to rustc via `--cfg=py_sys_config={varname}`;
 /// this allows using them conditional cfg attributes in the .rs files, so
 ///
-///     #[cfg(py_sys_config="{varname}"]
+/// ```rust
+/// #[cfg(py_sys_config="{varname}")]
+/// # struct Foo;
+/// ```
 ///
 /// is the equivalent of `#ifdef {varname}` in C.
 ///
@@ -915,7 +936,7 @@ mod tests {
     use super::*;
 
     #[test]
-    pub fn test_read_write_roundtrip() {
+    fn test_read_write_roundtrip() {
         let config = InterpreterConfig {
             abi3: true,
             base_prefix: Some("base_prefix".into()),
@@ -935,5 +956,98 @@ mod tests {
             config,
             InterpreterConfig::from_reader(Cursor::new(buf)).unwrap()
         );
+
+        // And some different options, for variety
+
+        let config = InterpreterConfig {
+            abi3: false,
+            base_prefix: None,
+            build_flags: {
+                let mut flags = HashSet::new();
+                flags.insert(BuildFlag::Py_DEBUG);
+                flags.insert(BuildFlag::Other(String::from("Py_SOME_FLAG")));
+                BuildFlags(flags)
+            },
+            calcsize_pointer: None,
+            executable: None,
+            implementation: PythonImplementation::PyPy,
+            ld_version: None,
+            libdir: None,
+            shared: true,
+            version: PythonVersion {
+                major: 3,
+                minor: 10,
+            },
+        };
+        let mut buf: Vec<u8> = Vec::new();
+        config.to_writer(&mut buf).unwrap();
+
+        assert_eq!(
+            config,
+            InterpreterConfig::from_reader(Cursor::new(buf)).unwrap()
+        );
+    }
+
+    #[test]
+    fn build_flags_from_config_map() {
+        let mut config_map = HashMap::new();
+
+        assert_eq!(BuildFlags::from_config_map(&config_map).0, HashSet::new());
+
+        for flag in &BuildFlags::ALL {
+            config_map.insert(flag.to_string(), "0".into());
+        }
+
+        assert_eq!(BuildFlags::from_config_map(&config_map).0, HashSet::new());
+
+        let mut expected_flags = HashSet::new();
+        for flag in &BuildFlags::ALL {
+            config_map.insert(flag.to_string(), "1".into());
+            expected_flags.insert(flag.clone());
+        }
+
+        assert_eq!(BuildFlags::from_config_map(&config_map).0, expected_flags);
+    }
+
+    #[test]
+    fn build_flags_fixup_py36_debug() {
+        let mut build_flags = BuildFlags(HashSet::new());
+        build_flags.0.insert(BuildFlag::Py_DEBUG);
+
+        build_flags = build_flags.fixup(
+            PythonVersion { major: 3, minor: 6 },
+            PythonImplementation::CPython,
+        );
+
+        // On 3.6, Py_DEBUG implies Py_REF_DEBUG and Py_TRACE_REFS
+        assert!(build_flags.0.contains(&BuildFlag::Py_REF_DEBUG));
+        assert!(build_flags.0.contains(&BuildFlag::Py_TRACE_REFS));
+    }
+
+    #[test]
+    fn build_flags_fixup_py37_debug() {
+        let mut build_flags = BuildFlags(HashSet::new());
+        build_flags.0.insert(BuildFlag::Py_DEBUG);
+
+        build_flags = build_flags.fixup(PythonVersion::PY37, PythonImplementation::CPython);
+
+        // On 3.7, Py_DEBUG implies Py_REF_DEBUG
+        assert!(build_flags.0.contains(&BuildFlag::Py_REF_DEBUG));
+
+        // 3.7 always has WITH_THREAD
+        assert!(build_flags.0.contains(&BuildFlag::WITH_THREAD));
+    }
+
+    #[test]
+    fn build_flags_fixup_pypy() {
+        let mut build_flags = BuildFlags(HashSet::new());
+
+        build_flags = build_flags.fixup(
+            PythonVersion { major: 3, minor: 6 },
+            PythonImplementation::PyPy,
+        );
+
+        // PyPy always has WITH_THREAD
+        assert!(build_flags.0.contains(&BuildFlag::WITH_THREAD));
     }
 }