From 4fdd0e9159b5abe3ad08a8ababfad1f8fae69a89 Mon Sep 17 00:00:00 2001 From: John Blischak Date: Thu, 16 Mar 2023 12:56:02 -0400 Subject: [PATCH 1/3] Build Python client on Windows --- apis/python/setup.py | 2 +- apis/python/src/tiledbvcf/__init__.py | 2 ++ apis/python/tests/test_tiledbvcf.py | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/apis/python/setup.py b/apis/python/setup.py index a1237d1da..bf67312ee 100644 --- a/apis/python/setup.py +++ b/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: diff --git a/apis/python/src/tiledbvcf/__init__.py b/apis/python/src/tiledbvcf/__init__.py index e5c865f1f..b64baca0c 100644 --- a/apis/python/src/tiledbvcf/__init__.py +++ b/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" diff --git a/apis/python/tests/test_tiledbvcf.py b/apis/python/tests/test_tiledbvcf.py index c4046c801..bcbadeb50 100755 --- a/apis/python/tests/test_tiledbvcf.py +++ b/apis/python/tests/test_tiledbvcf.py @@ -1106,6 +1106,7 @@ def test_ingest_mode_merged(tmp_path): assert ds.count(regions=["chrX:9032893-9032893"]) == 0 +@pytest.mark.skipif(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: From a50e76371b4cb731514b76018962406921172d61 Mon Sep 17 00:00:00 2001 From: dhoke4tdb <78012908+dhoke4tdb@users.noreply.github.com> Date: Thu, 16 Mar 2023 12:57:42 -0400 Subject: [PATCH 2/3] Fix linking for Python client build on Windows --- apis/python/setup.py | 20 ++++++++++++++------ libtiledbvcf/src/CMakeLists.txt | 28 ++++++++++++++++++++++++++-- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/apis/python/setup.py b/apis/python/setup.py index bf67312ee..65d82456d 100644 --- a/apis/python/setup.py +++ b/apis/python/setup.py @@ -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()) diff --git a/libtiledbvcf/src/CMakeLists.txt b/libtiledbvcf/src/CMakeLists.txt index 471e8de1e..a78f2d921 100644 --- a/libtiledbvcf/src/CMakeLists.txt +++ b/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 $ + 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 From c8c43c64e9c0cd3bdfef1c7f8f5b11588c55a0dd Mon Sep 17 00:00:00 2001 From: John Blischak Date: Thu, 16 Mar 2023 12:58:37 -0400 Subject: [PATCH 3/3] Build and test on Windows with GitHub Actions --- .github/workflows/windows.yml | 60 +++++++++++++++++++++++++++++ apis/python/tests/test_tiledbvcf.py | 8 +++- ci/build-libtiledbvcf.bat | 21 ++++++++++ ci/build-tiledbvcf-py.bat | 9 +++++ ci/gha-win-env.yml | 28 ++++++++++++++ 5 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/windows.yml create mode 100644 ci/build-libtiledbvcf.bat create mode 100644 ci/build-tiledbvcf-py.bat create mode 100644 ci/gha-win-env.yml diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml new file mode 100644 index 000000000..248169635 --- /dev/null +++ b/.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 ` +# +# 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 diff --git a/apis/python/tests/test_tiledbvcf.py b/apis/python/tests/test_tiledbvcf.py index bcbadeb50..5efb73346 100755 --- a/apis/python/tests/test_tiledbvcf.py +++ b/apis/python/tests/test_tiledbvcf.py @@ -1106,7 +1106,13 @@ def test_ingest_mode_merged(tmp_path): assert ds.count(regions=["chrX:9032893-9032893"]) == 0 -@pytest.mark.skipif(shutil.which("bcftools") is None, reason="no bcftools") +# 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: diff --git a/ci/build-libtiledbvcf.bat b/ci/build-libtiledbvcf.bat new file mode 100644 index 000000000..9c7626997 --- /dev/null +++ b/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 diff --git a/ci/build-tiledbvcf-py.bat b/ci/build-tiledbvcf-py.bat new file mode 100644 index 000000000..df0513bd9 --- /dev/null +++ b/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 diff --git a/ci/gha-win-env.yml b/ci/gha-win-env.yml new file mode 100644 index 000000000..c1aab8a92 --- /dev/null +++ b/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