Skip to content

Conversation

@PProfizi
Copy link
Contributor

@PProfizi PProfizi commented Sep 19, 2023

Closes #1165

Adds a .ci/build_wheel.py Python script which takes a platform argument (as well as a whether to create a wheelhouse)
It is used in separate jobs to create and output the any and manylinux1 wheels.
It is used everywhere else to build, test and output the win_amd64 wheel for Windows runners, and the manylinux_2_17 wheel for Linux runners.
The any wheel is also tested.

…e assets to PyPI, as well as the .tar.gz and .zip sources
@PProfizi PProfizi added the CI/CD Related to CI/CD label Sep 19, 2023
@PProfizi PProfizi added this to the v0.9.1 milestone Sep 19, 2023
@PProfizi PProfizi self-assigned this Sep 19, 2023
@codecov
Copy link

codecov bot commented Sep 19, 2023

Codecov Report

Merging #1166 (723c0b4) into master (83dbd80) will increase coverage by 2.42%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1166      +/-   ##
==========================================
+ Coverage   84.95%   87.38%   +2.42%     
==========================================
  Files          81       81              
  Lines        9224     9224              
==========================================
+ Hits         7836     8060     +224     
+ Misses       1388     1164     -224     

Copy link
Contributor

@cbellot000 cbellot000 left a comment

Choose a reason for hiding this comment

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

@PProfizi could you please give us an overview of what tests/examples are ran on which wheel?
I've not seen any code change to make the any wheel work, were there already ifs in the code to find the binaries in the installer when the pydpf-coe wheel doesn't have any binary?

@PProfizi
Copy link
Contributor Author

PProfizi commented Sep 21, 2023

@PProfizi could you please give us an overview of what tests/examples are ran on which wheel? I've not seen any code change to make the any wheel work, were there already ifs in the code to find the binaries in the installer when the pydpf-coe wheel doesn't have any binary?

@cbellot000 I've added a tests job for the any wheel on Linux and on Windows, and the logic when not finding gatebin binaries is here. There is an error message which might require an update but otherwise it should go look into the installation yes.
Update: I now also remove the gatebin folder entirely when building an any wheel and now the logic is good.

@PProfizi PProfizi requested a review from a team as a code owner September 21, 2023 15:04
@PProfizi PProfizi requested a review from cbellot000 October 10, 2023 13:04
# This script generates the different versions of the ansys-dpf-core wheels based on a given input.
# Input can be one of ["any", "win", "manylinux1", "manylinux_2_17"]

import argparse
Copy link
Contributor

Choose a reason for hiding this comment

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

if I remember correctly, wheels used to be build by dpf-action repo, what is done where now?

Copy link
Contributor

Choose a reason for hiding this comment

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

what about pydpf-post?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if I remember correctly, wheels used to be build by dpf-action repo, what is done where now?

@cbellot000 they indeed used to be created by an action in the dpf-action repo. Now I've changed to have the logic directly in the pydpf-core yml files. I will consider making a common action later but since this is particular to pydpf-core, it would not have a big advantage to move it to the pydpf-actions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what about pydpf-post?

Right now the pydpf-post regular CI just clones the pydpf-core repo and installs it as-is since the dependency is on the git repository directly. I'll have to make a PR on PyDPF-Post to properly build the ansys-dpf-core wheel based on the context, but it can be done after this.
The pydpf-post Release CI gets an ansys-dpf-core wheel from pip, so it requires no change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, will there be any time with a broken pipeline in pydpf-post? (it's just to make sure we capture evrything)

Also if this will be used in pydpf-post, I guess you'll move it to pydpf-actions after?

Copy link
Contributor Author

@PProfizi PProfizi Oct 11, 2023

Choose a reason for hiding this comment

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

Okay, will there be any time with a broken pipeline in pydpf-post? (it's just to make sure we capture evrything)

Also if this will be used in pydpf-post, I guess you'll move it to pydpf-actions after?

Hi @cbellot000, sorry I do not understand the first sentence.
Edit: OK I think I can test running the pydpf-post pipelines on this branch to check what happens.

As for pydpf-post, indeed once I have a build_core action I can move it to the pydpf-actions, but this is really a consolidation step which is not really required at this point.

Copy link
Contributor Author

@PProfizi PProfizi Oct 11, 2023

Choose a reason for hiding this comment

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

See here for a run of the PyDPF-Post CI while targeting this branch specifically in the requirements, meaning this branch will be cloned when pip installing dependencies of ansys-dpf-post. Everything passes.

- name: "Build ansys-dpf-core wheel"
shell: bash
run: |
if [ ${{ matrix.os }} == "ubuntu-latest" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

is there any place where we have all wheels available (link in pypi) and we let pip pick the right one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cbellot000 this is not tested in the CI, no.
We know that pip will pick from most-specific to more-generic, I tested that for other libraries which also do things this way.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we could at least run the example like that, then (at least in the release pipeline)? if not, we are not really testing a user environment

@PProfizi PProfizi merged commit 8e1a271 into master Oct 12, 2023
@PProfizi PProfizi deleted the ci/os_specific_wheels branch October 12, 2023 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI/CD Related to CI/CD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create OS-specific wheels as pipeline artifacts

4 participants