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 support for releasing windows wheels. #2

Merged
merged 3 commits into from
Nov 21, 2024
Merged

Conversation

sahas3
Copy link
Contributor

@sahas3 sahas3 commented Oct 21, 2024

This PR adds support for windows nightly build for releasing torch-mlir python wheels following the work done for linux.
Here's the job for the windows build: https://github.com/sahas3/torch-mlir-release/actions/runs/11425775316

@sahas3
Copy link
Contributor Author

sahas3 commented Oct 21, 2024

Hello @stellaraccident and @sjain-stanford I am hoping to enable windows python wheel releases for torch-mlir repo with this PR. Can you please take a look? I don't have access to request reviews yet, so tagging you both here. Thanks!

@sjain-stanford
Copy link
Member

Thanks. I don't have any concerns as long as you (or someone) is willing to support in keeping this "green" going forward. Looping in @saienduri and @stellaraccident as well in case they have thoughts.

@sahas3
Copy link
Contributor Author

sahas3 commented Oct 21, 2024

Thanks, yes I intend to keep maintaining the windows wheels.

name: Build Release and Publish windows wheels

on:
workflow_dispatch:
Copy link
Collaborator

@saienduri saienduri Oct 23, 2024

Choose a reason for hiding this comment

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

can you add a pull_request: temporarily here? This will trigger a run and that way we can see how long it takes and if it works as intended :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review @saienduri. I've made the suggested edit but I don't have permission to run the workflow. Can you trigger it please when you get a chance? Thanks!

Copy link
Collaborator

@saienduri saienduri Oct 23, 2024

Choose a reason for hiding this comment

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

Yeah, triggered 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.

Thanks @saienduri, looks like it failed due to permissions probably because this is triggered via the PR. The action does run fine in my repo https://github.com/sahas3/torch-mlir-release/actions/runs/11487806632. To debug further, I made a temporary change to elevate all permissions but I can't trigger the workflow myself. What will be best way to debug this further in this repo -- any suggestions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I recently got write access to LLVM but looks like that doesn't cover this repository, so I still cannot trigger the workflow in here to debug the issue further. Is it possible to get write access for this repository? If not, @saienduri do you have any suggestions for debugging the permission issue as the change runs fine in my forked repo. Thanks!

Copy link
Collaborator

@saienduri saienduri Nov 21, 2024

Choose a reason for hiding this comment

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

I don't think the GITHUB_TOKEN itself is accessible on forks. You can revert your permission change and we can land.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @saienduri. Reverted the temporary changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great, approved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @saienduri, can you please merge this PR as well whenever you get a chance.

Copy link
Collaborator

@saienduri saienduri Nov 21, 2024

Choose a reason for hiding this comment

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

@saienduri
Copy link
Collaborator

Thanks, great to see windows wheels!

@stellaraccident
Copy link
Collaborator

No opinion on this, but we should re enable the windows CI on the main project, at least in terms of building. We can do that with the jit importer and stablehlo disabled since they do not seem to get timely windows updates.

@sahas3
Copy link
Contributor Author

sahas3 commented Oct 23, 2024

No opinion on this, but we should re enable the windows CI on the main project, at least in terms of building. We can do that with the jit importer and stablehlo disabled since they do not seem to get timely windows updates.

I can take a stab at this. Are you suggesting to run the windows CI build for each PR change similar to the linux CI?

@saienduri saienduri merged commit e294109 into llvm:main Nov 21, 2024
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.

4 participants