FIX: building on Windows-ARM64

[TTsangSC]

Closes #390... hopefully

• Updated pyproject.toml::[tool.cibuildwheel.windows] to also target ARM64.
• Added a corresponding windows-11-arm job for each windows-latest (i.e. x64) CI job in .github/workflows/test.yml::test_binpy_wheels except for the following Python versions:
  • 3.8, because cibuildwheel doesn't support that)
  • 3.9 and .10, because apparently 3.11 is the oldest version available on the required machines
• Updated line_profiler/c_trace_callback.h to:
  • Only use the #undef hacks required on 3.12 when we're actually on 3.12, and only use the #undef HAVE_STD_ATOMIC hack on Linux where it is necessary
  • Vendor in the required components from cpython/Include/internal/pycore_atomic.h when on 3.12 + Windows + ARM64, so that the problematic definitions in said files will not be included by the preprocessor
• Updated tests/test_cython.py so that the tests can be skipped when on Windows-ARM64, which didn't seem to agree with installing Cython packages in an editable manner (EDIT: fixed; it was a CI problem, not a code/test/Cython problem)

[da-woods]

I'm wondering (but not certain) if the includes can be moved into the .c file and that would simplify things: #392 Looks like it doesn't unfortunately

[TTsangSC]

As of 6f3a09e I've gotten line_profiler to build on Windows-ARM64 (example passed build job), but ironically the installation of other Cython packages seems to be problematic (example failed test job).

IDK if the problem lies with how the test fixture was written (#352; installing a local Cython package in editable mode), said test Cython package, or with Cython itself – but I guess that was the original intent behind #390, to make sure that Cython works reliably on the platform so downstream applications can use it.

Guess that it's our turn to turn off some tests so that we can proceed. ;)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.25%. Comparing base (60e928f) to head (95063ef).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #391      +/-   ##
==========================================
+ Coverage   87.56%   90.25%   +2.68%     
==========================================
  Files          18       20       +2     
  Lines        1641     2073     +432     
  Branches      348      447      +99     
==========================================
+ Hits         1437     1871     +434     
- Misses        149      151       +2     
+ Partials       55       51       -4     

see 5 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6153697...95063ef. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[da-woods]

From a quick look, you may have been failing to link to the Python library I think. Although I'm not sure why.

I don't think there's huge issues with Cython on Windows ARM in general. The vast majority of our own test suite works. Apart from the line_profiler test, the only other issue we ran into was an MSVC compiler bug when constructing large tuples or lists, which affected 2 or 3 tests.

[TTsangSC]

... you're right, in hindsight it should've been obvious that (most of) Cython works as intended since the build for line_profiler itself succeeded.* Most probably I misconfigured the environment on the test instances... will double check the CI.

In any case thanks for the input and for writing the initial issue!

* The caveat of course being that the wheel builds happened didn't happen on ARM64.

EDIT: crucial typo... but you probably guessed what was meant anyway.

[Erotemic]

I will need to think about how to incorporate this CI change into my xcookie CI templates so I we can continue to maintain this as the CI updates. Shouldn't be too hard.

I'm also noticing that we don't seem to have Linux ARM64 builds, and I don't remember if there was a reason they were turned off, or they were just never enabled.

@da-woods Have you tested if this patch fixes #390?

[da-woods]

Yes - if I point our CI at this patch commit then it runs successfully (at least on 3.13; we skip most profiling on 3.12 because it was awkwardly caught between the old system and sys.monitoring, but that isn't your fault...)

[TTsangSC]

A couple things:

• The Cython test failing is probably due to Native ARM support ilammy/msvc-dev-cmd#90 – as it is now the Action doesn't auto-detect when it's on an ARM runner, and instead sets the arch to x64 by default. This probably explains why the linking went south... if that's the culprit (will test in a moment) EDIT: it is, we may want to ensure that the fix is also included in xcookie.

• As for the Linux build situation...

  • Back in 3.5.1 we did have runners with arch=aarch64, and apparently QEMU was used to build therefor. Hence we had wheels for x86, i686, and aarch64 on PyPI.
  • In 4.0.0 we/you got rid of the aarch64 stuff in Update build process #157 (9afaed2), which correspondingly caused those wheels to disappear.

  I suppose once we're sure that this works we can more or less do the same for Linux and just switch the os to ubuntu-24.04-arm.

EDIT: and of course building more Linux wheels will probably lengthen the pipeline duration (by 50% since we're building and testing for one more arch). But then again it seems that nowadays the macOS job of build_binpy_wheels is usually the rate-limiting one (probably because the universal2 wheel has to be tested on both x86_64 and arm64), so maybe it will be alright.

[TTsangSC]

Hold on, I noticed that for some reasons we stopped building Linux-i686 wheels after merging #369. Was that because of the cibuildwheel version bump from 2.X to 3.X? @Erotemic

[Erotemic]

This line 51 is included, and then included again unconditionally on 53.

[Erotemic]

Suggested change

	# `msvc-dev-cmd` sets this envvar, which intereferes with
	# `msvc-dev-cmd` sets this envvar, which interferes with

[Erotemic]

So explain to me again why we are defining these here. Is anything in our library actually using these structures, or is this all just to make the compiler happy and ultimately this code is unused?

IIUC, maybe it makes sense to make these fail a little louder if they are ever used?

Would something like this make sense?

/* If any stub is referenced, fail the build with an unresolved external.
 * This ensures we never ship wheels that "use" these placeholders. */
#ifdef _MSC_VER
__declspec(dllimport) void lp_link_error__stubbed_cpython_atomic_LOAD_relaxed_was_used_this_is_a_bug(void);
__declspec(dllimport) void lp_link_error__stubbed_cpython_atomic_STORE_relaxed_was_used_this_is_a_bug(void);
#else
extern void lp_link_error__stubbed_cpython_atomic_LOAD_relaxed_was_used_this_is_a_bug(void);
extern void lp_link_error__stubbed_cpython_atomic_STORE_relaxed_was_used_this_is_a_bug(void);
#endif
#define _LP_ATOMIC_PANIC_LOAD_EXPR()  (lp_link_error__stubbed_cpython_atomic_LOAD_relaxed_was_used_this_is_a_bug(), 0)
#define _LP_ATOMIC_PANIC_STORE_STMT() do { lp_link_error__stubbed_cpython_atomic_STORE_relaxed_was_used_this_is_a_bug(); } while (0)

/* Panic-on-use shims (expression/statement forms) */
#undef  _Py_atomic_load_relaxed
#undef  _Py_atomic_store_relaxed
#define _Py_atomic_load_relaxed(obj)       ((void)(obj), _LP_ATOMIC_PANIC_LOAD_EXPR())
#define _Py_atomic_store_relaxed(obj,val)  do { (void)(obj); (void)(val); _LP_ATOMIC_PANIC_STORE_STMT(); } while (0)

[TTsangSC]

• We need PyInterpreterState's declaration (i.e. struct _is) from internals/pycore_interp.h. Other than the concrete declaration of PyFrameObject (from internals/pycore_frame.h) #included in Python_wrapper.h, this is the only other internals/pycore_* API that we use in the project.
• In Python 3.12, said header file#include internals/pycore_atomic.h and uses these names therefrom:
  • _Py_atomic_address is directly used in the declaration.
  • We don't actually need _Py_atomic_load_relaxed() and _Py_atomic_store_relaxed() per se (they aren't related to the declaration), but the names have to be available because they are used in macros defined below in internals/pycore_frame.h.
• The header file also #include internals/pycore_ceval_state.h, which uses _Py_atomic_int.

In principle it's "safe" to keep things as-is since we're only doing this for 3.12, which is ossified at this point. But yes it'd be nicer to have some failsafes.

[Erotemic]

Thanks for the changes. I think this makes it clearer what the workaround is just by reading the code.

[TTsangSC]

Thanks for the review!