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

fix: wheel for windows #111

Merged
merged 4 commits into from
Jan 4, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
84 changes: 84 additions & 0 deletions .github/workflows/wheel.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
name: Wheel builder

on:
pull_request:
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we do just pull_request and push into main? I think that is our standard on our other workflows

Copy link
Contributor Author

@Tieqiong Tieqiong Jan 4, 2025

Choose a reason for hiding this comment

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

@sbillinge I'll delete this workflow file before merging. This is just for showing the changes work here, and it would be easier for @bobleesj to copy to the central release-script. So details shouldn't matter too much.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Tieqiong Billingegroup/release-scripts#123

I copied and saved in the issue. pls confirm if that's all good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bobleesj please see comment there

Copy link
Contributor

Choose a reason for hiding this comment

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

As a general rule I like taking care of details when we think of them, otherwise technical debt tends to just start piling up. But if this is going to be completely gone through again later and everything fixed, i htink it is ok to let things slide here.

push:
workflow_dispatch:

jobs:
build_wheels:

defaults:
run:
shell: bash -l {0}

name: Build wheel ${{ matrix.python[0] }}-${{ matrix.buildplat[0] }}
runs-on: ${{ matrix.buildplat[0] }}
strategy:
fail-fast: false
matrix:
buildplat:
- [ubuntu-latest, manylinux_x86_64]
- [macos-13, macosx_x86_64]
- [macos-14, macosx_arm64]
- [windows-latest, win_amd64]
python:
- ["3.11", "cp311"]
- ["3.12", "cp312"]

steps:
- name: Check out #${{ inputs.project }}
uses: actions/checkout@v4

- name: Set up Python ${{ matrix.python[0] }}
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python[0] }}

- name: Build wheels for Linux
if: runner.os == 'Linux'
uses: pypa/[email protected]
env:
CIBW_BUILD: ${{ matrix.python[1] }}-${{ matrix.buildplat[1] }}
CIBW_BEFORE_BUILD: yum install -y gsl-devel && pip install -e .
with:
output-dir: wheelhouse

- name: Build wheels for macOS
if: runner.os == 'macOS'
uses: pypa/[email protected]
env:
CIBW_BUILD: ${{ matrix.python[1] }}-${{ matrix.buildplat[1] }}
MACOSX_DEPLOYMENT_TARGET: 13.0
CIBW_BEFORE_BUILD: brew install gsl && pip install -e .
with:
output-dir: wheelhouse

- name: Set up conda for Windows
if: runner.os == 'Windows'
uses: conda-incubator/setup-miniconda@v3
with:
activate-environment: gsl
auto-update-conda: true
environment-file: environment.yml
auto-activate-base: false

- name: install gsl for Windows
if: runner.os == 'Windows'
run: |
conda config --set always_yes yes --set changeps1 no
conda install gsl

- name: Build wheels for Windows
if: runner.os == 'Windows'
uses: pypa/[email protected]
env:
CIBW_BUILD: ${{ matrix.python[1] }}-${{ matrix.buildplat[1] }}
CONDA_PREFIX: ${{ env.CONDA_PREFIX }}
with:
output-dir: wheelhouse

- uses: actions/upload-artifact@v4
with:
name: ${{ matrix.python[0] }}-${{ matrix.buildplat[0] }}
path: ./wheelhouse/*.whl
16 changes: 16 additions & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@
import glob
import os
import re
import shutil
import sys
import warnings

from setuptools import Extension, setup
from setuptools.command.build_ext import build_ext

# Use this version when git data are not available, like in git zip archive.
# Update when tagging a new release.
Expand Down Expand Up @@ -83,6 +85,19 @@ def get_gsl_config_win():
return {"include_dirs": [inc], "library_dirs": [lib]}


class CustomBuildExt(build_ext):
def run(self):
super().run()
gsl_path = os.environ.get("GSL_PATH") or os.path.join(os.environ.get("CONDA_PREFIX", ""), "Library")
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer if we could use pathlib.Path() as much as possible for these operations, for easier future maintainability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sbillinge I changed most of the os.path to pathlib. Please check, thanks


bin_path = os.path.join(gsl_path, "bin")
dest_path = os.path.join(self.build_lib, "diffpy", "pdffit2")
os.makedirs(dest_path, exist_ok=True)

for dll_file in glob.glob(os.path.join(bin_path, "gsl*.dll")):
shutil.copy(dll_file, dest_path)


# ----------------------------------------------------------------------------

# compile and link options
Expand Down Expand Up @@ -134,6 +149,7 @@ def create_extensions():

setup_args = dict(
ext_modules=[],
cmdclass={"build_ext": CustomBuildExt},
)

if __name__ == "__main__":
Expand Down
Loading