Add Windows CI #689

jhale · 2024-04-30T19:59:37Z

Brings back macOS test.
Makes pygraphviz truly optional.
All C builds done in C17 standard mode.
Support for Visual C compiler on Windows (does not support _Complex).
Does not preclude use of ffcx on Windows for complex code generation with another compiler invoked via e.g. CMake or at the command line.

minrk · 2024-05-21T07:43:16Z

is there a possibility option to use clang on Windows via CFFI?

clang is available on Windows, though I've never used it, so don't know what the implications would be. I believe if you use msvc, it would means folks need to install a compiler outside of conda. I think clang should avoid this, but there may still be an SDK requirement, I'm not sure.

Question is: what packages would need to use clang:

all fenics packages (basix, dolfinx)?
none, just the runtime requirement?

It's going to take me a bit to figure out testing beyond basix on conda-forge, since the dependency means the unreleased builds need to be published somewhere that's not the conda-forge default channel, so downstream can depend on them.

jhale · 2024-05-21T08:14:09Z

is there a possibility option to use clang on Windows via CFFI?

clang is available on Windows, though I've never used it, so don't know what the implications would be. I believe if you use msvc, it would means folks need to install a compiler outside of conda. I think clang should avoid this, but there may still be an SDK requirement, I'm not sure.

Question is: what packages would need to use clang:

all fenics packages (basix, dolfinx)?

none, just the runtime requirement?

It's going to take me a bit to figure out testing beyond basix on conda-forge, since the dependency means the unreleased builds need to be published somewhere that's not the conda-forge default channel, so downstream can depend on them.

I took a look at how FFCx is currently using cffi.compile() and on Windows MSVC is the only option without implementing our own JIT compiler wrappers (which I don't want to do, unless it is absolutely necessary) - so users will need to install MSVC. Most of our users will be covered by the Community License anyway.

This issue (how can I use a different compiler with cffi?) is discussed here:

https://foss.heptapod.net/pypy/cffi/-/issues/560

Our only runtime dependency on a compiler is when using CFFI via FFCx, but it is an important core feature and cannot be skipped.

https://github.com/conda-forge/cffi-feedstock/blob/main/recipe/meta.yaml

You can see here there is no runtime dependency on a C compiler, so it must be assumed provided by the system.

jhale · 2024-05-21T08:19:50Z

On the last point, @chrisrichardson has a Windows machine setup for development, so perhaps we could work together to try a conda build locally?

minrk · 2024-05-21T10:15:03Z

That's possible. I'm going to try to download the artifacts from unmerged conda-forge CI and upload them to my channel, which should work, but there seems to be a hiccup with downloading the artifacts right now.

minrk · 2024-05-21T10:17:37Z

I asked, and even with clang, users would need Windows SDKs to compile, so there would be little benefit to depending on clang instead of msvc.

jhale · 2024-05-21T13:35:47Z

Thanks! I read the thread - we can revisit clang as an option at a later date. For now MSVC2022 using cl.exe will be the only officially supported options on Windows.

jhale · 2024-05-21T13:41:25Z

Just to clarify a few points from https://conda.discourse.group/t/runtime-requirement-for-c-compiler-on-windows/614/3

FFCx has a runtime dependency on a C (not C++) compiler implicitly via CFFI when using our JIT compilation.
Our demo runner for testing has a dependency on a C compiler, but that is not strictly necessary for a user install. If a user creates .c and .h files via ffcx command line they will need a C compiler to build something with them but I hope that is self-explanatory.
We do not link with anything other than the standard links required by C/CFFI (stdc, Python) etc. and libm (only on Linux/Unix platforms). So there is no linking with Basix or DOLFINx.

minrk · 2024-05-21T13:50:32Z

Thanks! That clarifies things.

jhale · 2024-05-21T13:51:24Z

Great. This patch is now ready for final review and preliminary testing in Conda.

…e/windows-test

minrk · 2024-05-22T07:26:35Z

Haven't quite worked out all the kinks in getting basix from my channel into conda-forge CI, but so far looks like ffcx (d3ecb54) works fine on Windows in conda-forge with no modifications.

garth-wells · 2024-05-22T07:51:10Z

demo/test_demos.py

+        assert (
+            os.system(
+                f"cd {demo_dir} && "
+                f"gcc -I../ffcx/codegeneration "


Suggested change

f"gcc -I../ffcx/codegeneration "

f"cc -I../ffcx/codegeneration "

fwiw, I think cc = os.environ.get("CC", sysconfig.get_config_var("CC")) is more likely to get the right thing and accepts the ubiquitous standard env override.

On balance I'd rather go with a combination cc = os.environ.get("CC", "cc") - what Python thinks about how to build extension modules doesn't seem hugely relevant to this test which is about our C/C++ pipeline.

minrk · 2024-05-22T11:26:09Z

All tests pass on conda on Windows without any special handling: conda-forge/fenics-ffcx-feedstock#7

Windows folks should be able to test installation with conda with:

conda install -c conda-forge -c minrk/label/fenics-windows fenics-ffcx=0.9.0.dev

jhale · 2024-05-22T14:07:31Z

This is now ready to merge.

garth-wells

Nice!

garth-wells · 2024-05-22T14:12:09Z

.github/workflows/pythonapp.yml

@@ -22,40 +22,75 @@ jobs:
      matrix:
        os: [ubuntu-latest]
        python-version: ['3.9', '3.10', '3.11', '3.12']
+        include:
+          - os: windows-latest
+            python-version: '3.11'


Any reason to test with 3.11 rather than 3.12?

Not really, just providing some coverage because I will run against 3.12 on Windows elsewhere, e.g. DOLFINx.

garth-wells · 2024-05-22T14:12:43Z

.github/workflows/pythonapp.yml

@@ -64,10 +99,15 @@ jobs:
        # Use always() to always run this step to publish test results
        # when there are test failures
        if: always()
+
+      - name: Setup cl.exe (Windows)


Is cl.exe clang?

No, it's Microsoft's Visual C Compiler (MSVC).

Note that Microsoft also offer clang-cl.exe which is a wrapper over clang to a provide drop-in replacement for cl.exe. This looked exciting in the first instance, because it supports _Complex. However, clang-cl.exe is not supported by Conda or CFFI, where MSVC is the only supported option today on Windows.

I did however add a test of clang-cl.exe to our C demo tests, and it works fine.

garth-wells · 2024-05-22T14:12:59Z

demo/test_demos.py

@@ -17,6 +17,10 @@
 @pytest.mark.parametrize("scalar_type", ["float64", "float32", "complex128", "complex64"])
 def test_demo(file, scalar_type):
    """Test a demo."""
+    if sys.platform.startswith("win32") and "complex" in scalar_type:
+        # Skip complex demos on win32
+        pytest.skip(reason="_Complex not supported on Windows")


What a pain . . . .

Indeed, MSVC's lack of _Complex support has been the only major issue blocking a straightforward port.

jhale added 10 commits April 30, 2024 12:35

Whitespace.

8dce066

Try Windows.

dd1bc93

try without graphviz

4613f99

Refine optional dependencies.

e91bdf8

Verbose.

4cd48b7

Check pygraphviz exists

cdf17e4

Not clear why this fails on Windows.

07382ea

Update conftest.py

f4d5306

Remove to debug on win32

951b325

Ruff format.

4220d64

jhale added packaging ci Tasks related to Continuous Integration labels Apr 30, 2024

jhale added 18 commits May 1, 2024 20:51

Complex number support is optional part of C11 standard.

edb4fac

Merge remote-tracking branch 'origin/main' into jhale/c11compliance

970f6b4

Define __restrict for MSVC.

0ac4269

Invert logic

9043f1d

Merge branch 'jhale/c11compliance' into jhale/windows-test

7c9a34f

Few notes.

a137173

Merge remote-tracking branch 'origin/main' into jhale/windows-test

bcba03a

Works, hacky.

7684d82

Format.

40e34d4

Fix.

3eeee6c

Adjust code generation.

bc027e3

Throw NotImplementedError on Windows.

7db2a46

Attempt jit form pass on Windows.

6f86cc9

Ruff check fix.

c29b682

More adjustments to form code gen.

49160fb

Add proper restrict keyword for MSVC.

5a87c50

Compile in C17 mode.

a6b0e34

Make compile args default empty list.

58153a4

Add replacement for SOABI on Windows.

d6551a8

Merge branch 'main' into jhale/windows-test

0d58549

jhale added 2 commits May 21, 2024 16:32

get_config_vars potentially returns None, typing error.

bc9a94d

Merge branch 'jhale/windows-test' of github.com:FEniCS/ffcx into jhal…

d3ecb54

…e/windows-test

garth-wells reviewed May 22, 2024

View reviewed changes

jhale added 7 commits May 22, 2024 14:19

Use CC environment variable or generic Unix cc symlink.

fd1ee2f

Type hints for these public facing methods.

2aa9abd

Fix mypy errors.

6f972d0

Typo.

57ef86b

Get optional dependencies installed on Linux.

bf0e2f1

Improve test error messages.

8c2f350

Fix.

5b735b4

garth-wells reviewed May 22, 2024

View reviewed changes

garth-wells approved these changes May 22, 2024

View reviewed changes

jhale added this pull request to the merge queue May 22, 2024

Merged via the queue into main with commit 86a8cc2 May 22, 2024
13 checks passed

jhale deleted the jhale/windows-test branch May 22, 2024 14:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Windows CI #689

Add Windows CI #689

jhale commented Apr 30, 2024 •

edited

Loading

minrk commented May 21, 2024

jhale commented May 21, 2024

jhale commented May 21, 2024

minrk commented May 21, 2024

minrk commented May 21, 2024

jhale commented May 21, 2024

jhale commented May 21, 2024 •

edited

Loading

minrk commented May 21, 2024

jhale commented May 21, 2024

minrk commented May 22, 2024 •

edited

Loading

garth-wells May 22, 2024

minrk May 22, 2024

jhale May 22, 2024 •

edited

Loading

minrk commented May 22, 2024

jhale commented May 22, 2024

garth-wells left a comment

garth-wells May 22, 2024

jhale May 22, 2024

garth-wells May 22, 2024

jhale May 22, 2024 •

edited

Loading

garth-wells May 22, 2024

jhale May 22, 2024 •

edited

Loading

	f"gcc -I../ffcx/codegeneration "
	f"cc -I../ffcx/codegeneration "

Add Windows CI #689

Add Windows CI #689

Conversation

jhale commented Apr 30, 2024 • edited Loading

minrk commented May 21, 2024

jhale commented May 21, 2024

jhale commented May 21, 2024

minrk commented May 21, 2024

minrk commented May 21, 2024

jhale commented May 21, 2024

jhale commented May 21, 2024 • edited Loading

minrk commented May 21, 2024

jhale commented May 21, 2024

minrk commented May 22, 2024 • edited Loading

garth-wells May 22, 2024

Choose a reason for hiding this comment

minrk May 22, 2024

Choose a reason for hiding this comment

jhale May 22, 2024 • edited Loading

Choose a reason for hiding this comment

minrk commented May 22, 2024

jhale commented May 22, 2024

garth-wells left a comment

Choose a reason for hiding this comment

garth-wells May 22, 2024

Choose a reason for hiding this comment

jhale May 22, 2024

Choose a reason for hiding this comment

garth-wells May 22, 2024

Choose a reason for hiding this comment

jhale May 22, 2024 • edited Loading

Choose a reason for hiding this comment

garth-wells May 22, 2024

Choose a reason for hiding this comment

jhale May 22, 2024 • edited Loading

Choose a reason for hiding this comment

jhale commented Apr 30, 2024 •

edited

Loading

jhale commented May 21, 2024 •

edited

Loading

minrk commented May 22, 2024 •

edited

Loading

jhale May 22, 2024 •

edited

Loading

jhale May 22, 2024 •

edited

Loading

jhale May 22, 2024 •

edited

Loading