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 mingw CI #267

Merged
merged 2 commits into from
Jul 25, 2022
Merged

add mingw CI #267

merged 2 commits into from
Jul 25, 2022

Conversation

davidhewitt
Copy link
Member

No description provided.

@davidhewitt
Copy link
Member Author

Hmm I think this isn't picking up the right Python

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@davidhewitt
Copy link
Member Author

Hmm seems like nox is misbehaving

@davidhewitt davidhewitt force-pushed the mingw branch 13 times, most recently from e2b8170 to 1b8a27e Compare July 13, 2022 20:41
with:
profile: minimal
toolchain: stable
target: ${{ matrix.rust-target }}
Copy link
Member

Choose a reason for hiding this comment

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

I think we should install Rust in msys2 shell or build it with --target x86_64-pc-windows-gnu.

Looks like it's using x86_64-pc-windows-msvc because it's the default toolchain.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@messense
Copy link
Member

error: Rust build failed; unable to find any *html_py_ever.so in D:\a\setuptools-rust\setuptools-rust\examples\html-py-ever\target/release

Reproduced the issue.

@davidhewitt
Copy link
Member Author

Nice work @messense!

Hmm I tried to use tmate action to get a debugging shell but I can't use it for some reason. Worked earlier, will try again later.

I think that the likely code is here:

if "win" in platform:

I think we probably need to check sys.platform on msys2 and then fixup that condition. Hence why I wanted a debug shell :)

@davidhewitt
Copy link
Member Author

Sounds like from pypa/virtualenv#1338 (comment) that sys.platform might also be msys or maybe msys2.

To be honest I think the artifact suffix is really something controlled by rustc, so I'll refactor this condition to be based on the rust target.

co-authored-by: messense <messense@icloud.com>
@davidhewitt
Copy link
Member Author

@messense so I've got the mingw builds working by reading artifacts from the cargo output messages. This is probably more robust than the previous implementation when cross-compiling too.

However hit a couple of breaking problems. Wonder if you've seen these in maturin & might know how to work around:

  • cross uses different paths in the metadata so the copy fails (I think)
  • cargo-zigbuild doesn't support cargo-zigbuild metadata. Unsure if it should?

(pytest job expected to fall, it's from the debug prints I've added.)

@messense
Copy link
Member

cargo-zigbuild doesn't support cargo-zigbuild metadata. Unsure if it should?

I can add cargo-zigbuild metadata, it'd just delegate to cargo metadata since metadata command does not require special setup when cross compiling.

@messense
Copy link
Member

Copying rust artifact from /target/aarch64-unknown-linux-gnu/release/libnamespace_package_rust.so to build/lib.linux-x86_64-cpython-38/namespace_package/rust.so

cross use docker containers, so it's the path in container not the host.

@davidhewitt
Copy link
Member Author

cross use docker containers, so it's the path in container not the host.

Yeah I was aware of that, I was wondering if you knew how maturin avoided the same problem. Do you invoke cargo metadata and cross metadata so that you can see what each thinks the target directory is, and then map that onto each artifact path reported by cross?

@messense
Copy link
Member

maturin does not support using cross yet 😅. PyO3/maturin#1002

@davidhewitt davidhewitt marked this pull request as ready for review July 24, 2022 21:30
@davidhewitt
Copy link
Member Author

Fair enough! I seem to have reached an approach which works now by adjusting the target directory if using cross.

Copy link
Member

@messense messense left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants