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

Build windows pip wheels #534

Merged
merged 1 commit into from
Nov 24, 2022
Merged

Build windows pip wheels #534

merged 1 commit into from
Nov 24, 2022

Conversation

AbdBarho
Copy link
Contributor

@AbdBarho AbdBarho commented Nov 20, 2022

What does this PR do?

As a continuation of #523, adds windows pip wheels.

Before submitting

PR review

Anyone in the community is free to review the PR once the tests have passed.

Warning

I have no windows machine to test the built wheels, would be great if someone could test it.

You can see the workflow and download the artefacts here:

https://github.com/AbdBarho/xformers-wheels/actions/runs/3518281917

Additional info

Github runners go brrrrrrrrrrrrrrrrrr

github com_AbdBarho_xformers-wheels_actions_runs_3504628513 - Copy

The next thing on my todo list would be to optimize the build setup, maybe through docker images or simiar.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 20, 2022
@codecov-commenter
Copy link

codecov-commenter commented Nov 20, 2022

Codecov Report

Base: 89.81% // Head: 89.81% // No change to project coverage 👍

Coverage data is based on head (43c478c) compared to base (103e863).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #534   +/-   ##
=======================================
  Coverage   89.81%   89.81%           
=======================================
  Files          80       80           
  Lines        4848     4848           
=======================================
  Hits         4354     4354           
  Misses        494      494           
Flag Coverage Δ
Python 89.81% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@danthe3rd danthe3rd left a comment

Choose a reason for hiding this comment

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

Once again thanks a lot! this looks good to me. Are we ready to merge now?

@AbdBarho
Copy link
Contributor Author

AbdBarho commented Nov 24, 2022

@danthe3rd yes, although it would be great if any windows user could test the built wheels, you can merge it and we can deal with the problems later.

@danthe3rd
Copy link
Contributor

"test in prod" - let's do that :)

@danthe3rd danthe3rd merged commit 8910bb5 into facebookresearch:main Nov 24, 2022
@bangonkali
Copy link

Hi @AbdBarho & @danthe3rd , can we test this now? How do i test on windows?

@AbdBarho AbdBarho mentioned this pull request Dec 13, 2022
bertmaher pushed a commit to bertmaher/xformers that referenced this pull request Dec 20, 2024
The script is currently broken in the following situation:
* A merge conflict when importing a fairinternal comit (eg because it modifies a fairinternal file and can't be
applied)
* AND is also removing a file that exists in OSS (without merge conflict)
In that case, we try to resolve to the fairinternal version. This leads to removing the OSS file, but it's already
removed in the tree because there was no conflict, and hence `git rm FILE` fails.

*I also updated the test to reflect this scenario*

Co-authored-by: danthe3rd <danthe3rd@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants