Fix python releases

.github/workflows/python_build.yml

@@ -11,45 +11,18 @@ defaults:
     working-directory: ./python
 
 jobs:
-  build:
+  lint:
     runs-on: ubuntu-latest
-
     steps:
     - uses: actions/checkout@v2
-    # the whole target dir is over 400MB cache limit, so we have to split up
-    # the cache step to avoid caching deps dir
-    - name: Cache Rust build
-      uses: actions/cache@v1.0.1
-      with:
-        path: target/debug/build
-        key: python-${{ runner.OS }}-target-build-${{ hashFiles('**/Cargo.lock') }}
-        restore-keys: |
-          python-${{ runner.OS }}-target-build-
-    - name: Cache Rust incremental build
-      uses: actions/cache@v1.0.1
-      with:
-        path: target/debug/incremental
-        key: python-${{ runner.OS }}-target-incremental-${{ hashFiles('**/Cargo.lock') }}
-        restore-keys: |
-          python-${{ runner.OS }}-target-incremental-
-
-    - name: Format Python code with Black
+    - name: Setup Python
       uses: actions/setup-python@v2
       with:
         python-version: 3.6
-
-    - uses: psf/black@stable
+    - name: Format Python code with Black
+      uses: psf/black@stable
       with:
-        black_args: ". --check"
-
-    - name: Setup virtualenv
-      run: |
-        pip install virtualenv
-        virtualenv venv
-
-    - name: 'Install Python devel headers'
-      run: sudo apt install -y python3-dev
-
+        args: ". --check"
     - name: Install minimal stable with clippy and rustfmt
       uses: actions-rs/toolchain@v1
       with:
@@ -58,18 +31,37 @@ jobs:
         override: true
     - name: Check
       run: cargo clippy
-    - name: Build
-      run: cargo build
-    # - name: Run Rust tests
-    #   run: cargo test
+    - name: Format
+      run: cargo fmt -- --check
 
-    - name: Install matruin
+  test:
+    runs-on: ubuntu-latest
+    # use the same environment we have for python release
+    container: quay.io/pypa/manylinux2010_x86_64:2020-12-31-4928808
+    steps:
+    # actions/checkout@v2 is a node action, which runs on a fairly new
+    # version of node. however, manylinux environment's glibc is too old for
+    # that version of the node. so we will have to use v1 instead, which is a
+    # docker based action.
+    - uses: actions/checkout@v1
+
+    # actions-rs/toolchain@v1 is a node action, which runs on a fairly new
+    # version of node. however, manylinux environment's glibc is too old for
+    # that version of the node. so we will have to install rust manually here.
+    - name: Install Rust
       run: |
-        pip install maturin==0.9.0
+        curl https://sh.rustup.rs -sSf | sh -s -- -y
+        $HOME/.cargo/bin/rustup default stable
+        echo "$HOME/.cargo/bin" >> $GITHUB_PATH
+
+    - name: Enable manylinux Python targets
+      run: echo "/opt/python/cp36-cp36m/bin" >> $GITHUB_PATH
+
+    - name: Install matruin
+      run: pip install maturin==0.9.0
 
     - name: Run Python tests
       run: |
-        source venv/bin/activate
         # disable manylinux audit checks for test builds
         maturin build --manylinux off
         ls -lh ../target/wheels

Cargo.toml

@@ -3,4 +3,5 @@ members = [
     "ruby",
     "rust",
     "python",
+    "glibc_version",
 ]

glibc_version/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "glibc_version"
+version = "0.1.0"
+authors = ["Qingping Hou <qph@scribd.com>"]
+edition = "2018"
+
+[dependencies]
+regex = "1"

glibc_version/src/lib.rs

@@ -0,0 +1,88 @@
+pub struct Version {
+    pub major: usize,
+    pub minor: usize,
+}
+
+#[cfg(all(target_os = "linux", target_env = "gnu"))]
+mod imp {
+    use super::Version;
+    use std::process::Command;
+
+    use regex::Regex;
+
+    // glibc version is taken from std/sys/unix/os.rs
+    pub fn get_version() -> Result<Version, String> {
+        let output = Command::new("ldd")
+            .args(&["--version"])
+            .output()
+            .expect("failed to execute ldd");
+        let output_str = std::str::from_utf8(&output.stdout).unwrap();
+        let version_str = ldd_output_to_version_str(output_str)?;
+
+        parse_glibc_version(version_str)
+            .ok_or_else(|| format!("Invalid version string from ldd output: {}", version_str,))
+    }
+
+    fn ldd_output_to_version_str(output_str: &str) -> Result<&str, String> {
+        let version_reg = Regex::new(r#"ldd \(.+\) ([0-9]+\.[0-9]+)"#).unwrap();
+        if let Some(captures) = version_reg.captures(output_str) {
+            Ok(captures.get(1).unwrap().as_str())
+        } else {
+            Err(format!(
+                "ERROR: failed to detect glibc version. ldd output: {}",
+                output_str,
+            ))
+        }
+    }
+
+    // Returns Some((major, minor)) if the string is a valid "x.y" version,
+    // ignoring any extra dot-separated parts. Otherwise return None.
+    fn parse_glibc_version(version: &str) -> Option<Version> {
+        let mut parsed_ints = version.split('.').map(str::parse::<usize>).fuse();
+        match (parsed_ints.next(), parsed_ints.next()) {
+            (Some(Ok(major)), Some(Ok(minor))) => Some(Version { major, minor }),
+            _ => None,
+        }
+    }
+
+    #[cfg(test)]
+    mod tests {
+        use super::*;
+
+        #[test]
+        fn parse_ldd_output() {
+            let ver_str = ldd_output_to_version_str(
+                r#"ldd (GNU libc) 2.12
+Copyright (C) 2010 Free Software Foundation, Inc.
+This is free software; see the source for copying conditions.  There is NO
+warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+Written by Roland McGrath and Ulrich Drepper."#,
+            )
+            .unwrap();
+            assert_eq!(ver_str, "2.12");
+
+            let ver_str = ldd_output_to_version_str(
+                r#"ldd (Ubuntu GLIBC 2.31-0ubuntu9.2) 2.31
+  Copyright (C) 2020 Free Software Foundation, Inc.
+  This is free software; see the source for copying conditions.  There is NO
+  warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+  Written by Roland McGrath and Ulrich Drepper."#,
+            )
+            .unwrap();
+            assert_eq!(ver_str, "2.31");
+        }
+    }
+}
+
+#[cfg(not(all(target_os = "linux", target_env = "gnu")))]
+mod imp {
+    use super::Version;
+    pub fn get_version() -> Result<Version, String> {
+        unimplemented!();
+    }
+}
+
+#[inline]
+pub fn get_version() -> Result<Version, String> {
+    imp::get_version()
+}

rust/Cargo.toml

@@ -55,6 +55,9 @@ datafusion-ext = ["datafusion", "crossbeam"]
 azure = ["azure_core", "azure_storage", "reqwest"]
 s3 = ["rusoto_core", "rusoto_credential", "rusoto_s3", "rusoto_sts"]
 
+[build-dependencies]
+glibc_version = { path = "../glibc_version" }
+
 [dev-dependencies]
 utime = "0.3"
 serial_test = "*"

rust/build.rs

@@ -0,0 +1,23 @@
+#[cfg(all(target_os = "linux", target_env = "gnu"))]
+mod platform_cfg {
+    fn detect_glibc_renameat2() {
+        let ver = glibc_version::get_version().unwrap();
+        if ver.major >= 2 && ver.minor >= 28 {
+            println!("cargo:rustc-cfg=glibc_renameat2");
+        }
+    }
+
+    pub fn set() {
+        detect_glibc_renameat2();
+    }
+}
+
+#[cfg(not(all(target_os = "linux", target_env = "gnu")))]
+mod platform_cfg {
+    pub fn set() {}
+}
+
+fn main() {
+    println!("cargo:rerun-if-changed=build.rs");
+    platform_cfg::set();
+}

rust/src/storage/file/rename.rs

@@ -20,30 +20,14 @@ mod imp {
     use super::*;
     use std::ffi::CString;
 
-    #[cfg(target_os = "linux")]
-    const RENAME_NOREPLACE: libc::c_uint = 1;
-
-    #[cfg(target_os = "linux")]
-    extern "C" {
-        fn renameat2(
-            olddirfd: libc::c_int,
-            oldpath: *const libc::c_char,
-            newdirfd: libc::c_int,
-            newpath: *const libc::c_char,
-            flags: libc::c_uint,
-        ) -> libc::c_int;
-    }
-
     fn to_c_string(p: &str) -> Result<CString, StorageError> {
         CString::new(p).map_err(|e| StorageError::Generic(format!("{}", e)))
     }
 
     pub fn atomic_rename(from: &str, to: &str) -> Result<(), StorageError> {
-        let ret = unsafe {
-            let from = to_c_string(from)?;
-            let to = to_c_string(to)?;
-            platform_specific_rename(from.as_ptr(), to.as_ptr())
-        };
+        let cs_from = to_c_string(from)?;
+        let cs_to = to_c_string(to)?;
+        let ret = unsafe { platform_specific_rename(cs_from.as_ptr(), cs_to.as_ptr()) };
 
         if ret != 0 {
             let e = errno::errno();
@@ -60,10 +44,30 @@ mod imp {
         Ok(())
     }
 
+    #[allow(unused_variables)]
     unsafe fn platform_specific_rename(from: *const libc::c_char, to: *const libc::c_char) -> i32 {
         cfg_if::cfg_if! {
-            if #[cfg(target_os = "linux")] {
-                renameat2(libc::AT_FDCWD, from, libc::AT_FDCWD, to, RENAME_NOREPLACE)
+            if #[cfg(all(target_os = "linux", target_env = "gnu"))] {
+                cfg_if::cfg_if! {
+                    if #[cfg(glibc_renameat2)] {
+                        const RENAME_NOREPLACE: libc::c_uint = 1;
+
+                        extern "C" {
+                            fn renameat2(
+                                olddirfd: libc::c_int,
+                                oldpath: *const libc::c_char,
+                                newdirfd: libc::c_int,
+                                newpath: *const libc::c_char,
+                                flags: libc::c_uint,
+                            ) -> libc::c_int;
+                        }
+
+                        renameat2(libc::AT_FDCWD, from, libc::AT_FDCWD, to, RENAME_NOREPLACE)
+                    } else {
+                        // target has old glibc (< 2.28), we would need to invoke syscall manually
+                        unimplemented!()
+                    }
+                }
             } else if #[cfg(target_os = "macos")] {
                 libc::renamex_np(from, to, libc::RENAME_EXCL)
             } else {