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

Add win-arm64 support using cl.exe #9

Merged
merged 19 commits into from
Nov 18, 2024
6 changes: 6 additions & 0 deletions .ci_support/win-64.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
target_platform:
- win-64
c_compiler:
- gcc
c_stdlib:
- m2w64-sysroot
6 changes: 6 additions & 0 deletions .ci_support/win-arm64.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
target_platform:
- win-arm64
c_compiler:
- vs
c_stdlib:
- vs
11 changes: 3 additions & 8 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@ jobs:
include:
- os: windows-latest
subdir: win-64
# TODO:
# - os: windows-latest
# subdir: win-arm64
- os: windows-latest
subdir: win-arm64
env:
PYTHONUNBUFFERED: "1"
steps:
Expand All @@ -46,15 +45,11 @@ jobs:
shell: bash -el {0}
run: conda install -y "conda-build!=3.28.0,!=3.28.1" anaconda-client

- name: Fetch pinnings
shell: bash -el {0}
run: |
curl -LO https://raw.githubusercontent.com/conda-forge/conda-forge-pinning-feedstock/main/recipe/conda_build_config.yaml
- name: Build recipe
shell: bash -el {0}
env:
CONDA_BLD_PATH: ${{ runner.temp }}/bld
run: conda build recipe --override-channels -c conda-forge -m conda_build_config.yaml
run: conda build recipe --override-channels -c conda-forge -e .ci_support/${{ matrix.subdir }}.yaml

- uses: actions/upload-artifact@v4
if: github.event_name == 'pull_request'
Expand Down
57 changes: 44 additions & 13 deletions recipe/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,32 +6,63 @@ set -euxo pipefail

_ARCH=${target_platform#*-} # keep chunk after dash (e.g. '64' in 'win-64')

if [[ "${_ARCH}" == "64" ]]; then
_CL_MACHINE=x64
elif [[ "${_ARCH}" == "arm64" ]]; then
_CL_MACHINE=ARM64
fi

# Build resources file
test -f resources.rc && rm -f resources.rc
echo "#include \"winuser.h\"" > resources.rc
echo "1 RT_MANIFEST manifest.xml" >> resources.rc
test -f resources-${_ARCH}.res && rm -f resources-${_ARCH}.res
windres --input resources.rc --output resources-${_ARCH}.res --output-format=coff

if [[ "${c_compiler}" == "gcc" ]]; then
${WINDRES:-windres} --input resources.rc --output resources-${_ARCH}.res --output-format=coff -v
else
which rc.exe
rc.exe resources.rc
mv resources.res resources-${_ARCH}.res
fi

ls -alh .

# Compile launchers
for _TYPE in cli gui; do
if [[ ${_TYPE} == cli ]]; then
CPPFLAGS=
LDFLAGS=
if [[ "${c_compiler}" == "gcc" ]]; then
CPPFLAGS=
LDFLAGS=
else
CPPFLAGS=
LDFLAGS="-SUBSYSTEM:CONSOLE"
fi
else
CPPFLAGS="-D_WINDOWS -mwindows"
LDFLAGS="-mwindows"
if [[ "${c_compiler}" == "gcc" ]]; then
CPPFLAGS="-D_WINDOWS -mwindows"
LDFLAGS="-mwindows"
else
CPPFLAGS="-D_WINDOWS"
LDFLAGS="-SUBSYSTEM:WINDOWS"
fi
fi

# You *could* use MSVC 2008 here, but you'd end up with much larger (~230k) executables.
# cl.exe -opt:nowin98 -D NDEBUG -D "GUI=0" -D "WIN32_LEAN_AND_MEAN" -ZI -Gy -MT -MERGE launcher.c -Os -link -MACHINE:x64 -SUBSYSTEM:CONSOLE version.lib advapi32.lib shell32.lib
${BUILD_PREFIX}/Library/mingw-w64/bin/gcc \
-O2 -DSCRIPT_WRAPPER -DUNICODE -D_UNICODE -DMINGW_HAS_SECURE_API -DMAXINT=INT_MAX ${CPPFLAGS} \
${SRC_DIR}/launcher.c -c -o ${_TYPE}-${_ARCH}.o
# An executable with mingw and `static-libgcc` is 42K
# An executable with vcruntime statically linked (-MT) in is 920K
# An executable with vcruntime dynamically linked (-MD) in is 84K
# For arm64, since we don't have mingw, we are going to use -MD
# to reduce the binary size. Let's hope that all arm64 machines
# have vcruntime140.dll installed.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@finnagin and @zooba, is this correct or should we go with -MT? The executables built here need to able to run without the conda supplied vcurntime140.dll

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, we don't need to run standalone. So let's use MD

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do need them to run from Scripts which doesn't have a vcruntime140.dll.

Copy link
Contributor

Choose a reason for hiding this comment

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

So do we need -MD or -MT? I think we are currently using MD but your last message hints at needing MT as far as I can see? 🤔

Copy link

Choose a reason for hiding this comment

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

I would use -MT for safety for launchers - it's too likely to end up running from a place where it can't find a suitable runtime.

Using -MD matter more for Python itself, because extension modules will also need to load it, and they'll want to share state. The launcher (I assume) is only going to create a new process, rather than dynamically loading DLLs, so it won't be a huge impact.

if [[ "${c_compiler}" == "vs" ]]; then
cl.exe -D NDEBUG -D "WIN32_LEAN_AND_MEAN" ${CPPFLAGS} -ZI -Gy -MD launcher.c -Os -link -MACHINE:${_CL_MACHINE} ${LDFLAGS} resources-${_ARCH}.res user32.lib version.lib advapi32.lib shell32.lib -out:${_TYPE}-${_ARCH}.exe
else
${CC} -O2 -DSCRIPT_WRAPPER -DUNICODE -D_UNICODE -DMINGW_HAS_SECURE_API -DMAXINT=INT_MAX ${CPPFLAGS} \
${SRC_DIR}/launcher.c -c -o ${_TYPE}-${_ARCH}.o

${BUILD_PREFIX}/Library/mingw-w64/bin/gcc \
-Wl,-s --static -static-libgcc -municode ${LDFLAGS} \
${_TYPE}-${_ARCH}.o resources-${_ARCH}.res -o ${_TYPE}-${_ARCH}.exe
${CC} -Wl,-s --static -static-libgcc -municode ${LDFLAGS} \
${_TYPE}-${_ARCH}.o resources-${_ARCH}.res -o ${_TYPE}-${_ARCH}.exe
fi

done

Expand Down
15 changes: 9 additions & 6 deletions recipe/meta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,24 @@ source:
folder: cpython-LICENSE

build:
number: 0
number: 1
skip: true # [not win]
ignore_run_exports_from:
- {{ compiler("m2w64_c") }}
- {{ compiler("c") }}
- {{ stdlib("c") }}

requirements:
build:
- {{ compiler("m2w64_c") }}
- {{ stdlib("m2w64_c") }}
- posix
- {{ compiler("c") }}
- {{ stdlib("c") }}
# For bash and friends
- m2-base
host:
run:
# Any Python works as `%PREFIX%/python.exe` is present
# cli-64.exe (and friends) will look for it as `../python.exe`, so that's
# why we package the executables in `%PREFIX%/Scripts`
- python
- python # [x86_64]
Copy link
Contributor

Choose a reason for hiding this comment

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

We are only selecting this so the solver doesn't error out right now, right? But in the future we should re-enable it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes to both


test:
commands:
Expand Down
Loading