Merge pull request #507 from jdblischak/build-win-py

Build tiledbvcf-py on Windows

.github/workflows/windows.yml

@@ -0,0 +1,60 @@
+name: Build Python client for Windows
+on:
+  push:
+    paths:
+      - '.github/workflows/windows.yml'
+      - 'apis/python/**'
+      - 'ci/*.bat'
+      - 'ci/gha-win-env.yml'
+      - 'libtiledbvcf/**'
+  pull_request:
+    paths:
+      - '.github/workflows/windows.yml'
+      - 'apis/python/**'
+      - 'ci/*.bat'
+      - 'ci/gha-win-env.yml'
+      - 'libtiledbvcf/**'
+  workflow_dispatch:
+# Unfortunately, simply setting the default shell is currently insufficient to
+# activate the conda env for the cmd shell. This is a known, unresolved issue
+#
+# https://github.com/mamba-org/provision-with-micromamba/issues/39
+# https://github.com/mamba-org/provision-with-micromamba/pull/43
+# https://github.com/mamba-org/provision-with-micromamba/pull/56
+#
+# A workaround is to manually call `@CALL micromamba activate <name of env>`
+#
+# https://github.com/blue-yonder/turbodbc/pull/380/files#diff-72b980456954eec0c566d17ce7746f2c1f7c6b9b98b1ad01395b6f7cb8c4ca0b
+defaults:
+  run:
+    shell: cmd /C CALL {0}
+jobs:
+  build:
+    runs-on: windows-2022
+    steps:
+      - uses: actions/checkout@v3
+      - name: Install conda env
+        uses: mamba-org/provision-with-micromamba@main
+        with:
+          environment-file: ci/gha-win-env.yml
+          cache-env: true
+      - name: Build libtiledbvcf
+        run: |
+          @CALL micromamba activate win-env
+          cmd /C CALL ci\build-libtiledbvcf.bat
+      - name: libtiledbvcf version
+        run: |
+          @CALL micromamba activate win-env
+          tiledbvcf.exe version
+      - name: Build tiledbvcf-py
+        run: |
+          @CALL micromamba activate win-env
+          cmd /C CALL ci\build-tiledbvcf-py.bat
+      - name: tiledbvcf-py version
+        run: |
+          @CALL micromamba activate win-env
+          python -c "import tiledbvcf; print(tiledbvcf.version)"
+      - name: Test
+        run: |
+          @CALL micromamba activate win-env
+          pytest apis\python\tests

apis/python/setup.py

@@ -101,7 +101,7 @@ def find_libtiledbvcf():
         p.native_lib_install_dirs = [LIBTILEDBVCF_PATH]
 
     libdirs = ["lib"]
-    libnames = ["libtiledbvcf.dylib", "libtiledbvcf.so"]
+    libnames = ["libtiledbvcf.dylib", "libtiledbvcf.so", "tiledbvcf.lib"]
     for root in p.native_lib_install_dirs:
         for libdir in libdirs:
             for libname in libnames:
@@ -223,11 +223,19 @@ class BuildExtCmd(build_ext):
     """Builds the Pybind11 extension module."""
 
     def build_extensions(self):
-        opts = ["-std=c++11", "-g"]
-        if TILEDBVCF_DEBUG_BUILD:
-            opts.extend(["-O0"])
-        else:
-            opts.extend(["-O2"])
+        if sys.platform != "win32":
+            opts = ["-std=c++11", "-g"]
+            if TILEDBVCF_DEBUG_BUILD:
+                opts.extend(["-O0"])
+            else:
+                opts.extend(["-O2"])
+        else:  # windows
+            # Note: newer versions of msvc cl may not recognize -std:c++11
+            opts = ["-std:c++11", "-Zi"]
+            if TILEDBVCF_DEBUG_BUILD:
+                opts.extend(["-Od"])
+            else:
+                opts.extend(["-O2"])
 
         link_opts = []
         for ext in self.extensions:
@@ -243,7 +251,7 @@ def build_extensions(self):
             ext.include_dirs.append(pyarrow.get_include())
 
             # don't overlink the arrow core library
-            if "arrow" in ext.libraries:
+            if (sys.platform != "win32") and ("arrow" in ext.libraries):
                 ext.libraries.remove("arrow")
             ext.library_dirs.extend(pyarrow.get_library_dirs())
 

apis/python/src/tiledbvcf/__init__.py

@@ -10,6 +10,8 @@ def _load_libs():
     """Loads the required TileDB-VCF native library."""
     if sys.platform == "darwin":
         lib_name = "libtiledbvcf.dylib"
+    elif sys.platform == "win32":
+        lib_name = "tiledbvcf.dll"
     else:
         lib_name = "libtiledbvcf.so"
 

apis/python/tests/test_tiledbvcf.py

@@ -1106,6 +1106,13 @@ def test_ingest_mode_merged(tmp_path):
     assert ds.count(regions=["chrX:9032893-9032893"]) == 0
 
 
+# Ok to skip is missing bcftools in Windows CI job
+@pytest.mark.skipif(
+    os.environ.get("CI") == "true"
+    and platform.system() == "Windows"
+    and shutil.which("bcftools") is None,
+    reason="no bcftools",
+)
 def test_ingest_with_stats(tmp_path):
     tmp_path_contents = os.listdir(tmp_path)
     if "stats" in tmp_path_contents:

ci/build-libtiledbvcf.bat

@@ -0,0 +1,21 @@
+@echo on
+
+mkdir libtiledbvcf-build
+cd libtiledbvcf-build
+
+rem configure
+cmake ^
+  -DCMAKE_INSTALL_PREFIX:PATH="%CONDA_PREFIX%\Library" ^
+  -DOVERRIDE_INSTALL_PREFIX=OFF ^
+  -DCMAKE_BUILD_TYPE=Release ^
+  -DFORCE_EXTERNAL_HTSLIB=OFF ^
+  ../libtiledbvcf
+if %ERRORLEVEL% neq 0 exit 1
+
+rem build
+cmake --build . --config Release --parallel %CPU_COUNT%
+if %ERRORLEVEL% neq 0 exit 1
+
+rem install
+cmake --build . --target install-libtiledbvcf --config Release
+if %ERRORLEVEL% neq 0 exit 1

ci/build-tiledbvcf-py.bat

@@ -0,0 +1,9 @@
+@echo on
+
+cd apis\python
+
+python setup.py install ^
+  --single-version-externally-managed ^
+  --record record.txt ^
+  --libtiledbvcf="%CONDA_PREFIX%\Library"
+if %ERRORLEVEL% neq 0 exit 1

ci/gha-win-env.yml

@@ -0,0 +1,28 @@
+name: win-env
+channels:
+  - conda-forge
+  - tiledb
+  - nodefaults
+dependencies:
+  # build libtiledbvcf
+  - cmake
+  - git
+  - m2w64-htslib
+  - tiledb=2.15
+  - vs2019_win-64
+  # build tiledbvcf-py
+  - numpy
+  - pandas
+  - pyarrow=9.0
+  - pybind11
+  - python
+  - rpdb
+  - setuptools
+  - setuptools_scm=6.0.1
+  - setuptools_scm_git_archive
+  - wheel
+  # test tiledbvcf-py
+  - dask
+  - fsspec<2023.3.0
+  - pytest
+  - tiledb-py

libtiledbvcf/src/CMakeLists.txt

@@ -207,7 +207,11 @@ add_executable(tiledbvcf-bin
   ${CMAKE_CURRENT_SOURCE_DIR}/cli/tiledbvcf.cc
 )
 
-set_target_properties(tiledbvcf-bin PROPERTIES OUTPUT_NAME tiledbvcf)
+if (WIN32)
+  set_target_properties(tiledbvcf-bin PROPERTIES OUTPUT_NAME tiledbvcfcli)
+else()
+  set_target_properties(tiledbvcf-bin PROPERTIES OUTPUT_NAME tiledbvcf)
+endif()
 
 if(!WIN32)
   target_link_libraries(tiledbvcf-bin
@@ -309,8 +313,28 @@ if (WIN32)
 endif()
 # TBD: how to handle the tiledbvcf4test, don't really want it part of ultimate distribution...
 # ... does release packaging simply need to ignore it?
+
+if(WIN32)
+  # Having the .dll and the .exe with the same name was resulting in a .exp
+  # file for the .exe being 'last out' (replacing one of same name for the .dll)
+  # and was causing the python api extension
+  # to link to the .exe, which was not functional.
+  # So, tiledbvcf-bin now sets an output (above somewhere) of tiledbvcfcli.exe
+  # and we install it here RENAMEing it to match the known name in pre-existing
+  # *nix world.
+  install(PROGRAMS $<TARGET_FILE:tiledbvcf-bin>
+    RENAME tiledbvcf.exe
+    DESTINATION ${CMAKE_INSTALL_BINDIR}
+    )
+else()
+  install(
+    TARGETS tiledbvcf-bin
+    RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
+    )
+endif()
+
 install(
-  TARGETS tiledbvcf tiledbvcf-bin ${TILEDBVCF_EXTRA_INSTALL_TARGETS}
+  TARGETS tiledbvcf ${TILEDBVCF_EXTRA_INSTALL_TARGETS}
   RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
   LIBRARY DESTINATION lib
   ARCHIVE DESTINATION lib