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 a build/test workflow for Windows #3985

Open
ScottTodd opened this issue Jan 27, 2025 · 2 comments
Open

Add a build/test workflow for Windows #3985

ScottTodd opened this issue Jan 27, 2025 · 2 comments

Comments

@ScottTodd
Copy link
Member

We've seen several downstream build breaks from torch-mlir due to missing test coverage. Having at least a nightly CI build using GitHub-hosted runners would provide earlier signal for build issues.

Existing workflows

Expanding to new platforms

I see scripts used by ci.yml that could be forked or generalized:

A workflow could also be added that runs commands directly. I've been moving https://github.com/iree-org/iree (a downstream user of torch-mlir) away from such scripts, instead opting to make the build system work better out of the box using default options, with things like ccache and the choice of compiler (e.g. gcc/clang) delegated to the user's choice or a github action that configures environment variables.

@sahas3
Copy link
Contributor

sahas3 commented Feb 19, 2025

I gave this a shot by reusing https://github.com/llvm/torch-mlir/blob/main/build_tools/python_deploy/build_windows_ci.sh which is used for building the windows wheels in torch-mlir-release repo. The build worked fine but testing fails because signal.SIGALRM (used for timeout handling -- not much familiar with this piece of testing code) is not available for Windows. That's a different problem to solve to enable testing on Windows even outside of the CI workflow, so will create a separate issue for that. However, is there a benefit to still add a build only CI for now?

@ScottTodd
Copy link
Member Author

However, is there a benefit to still add a build only CI for now?

I would say yes. I would expect most platform differences to come into play at build time, not test time. Could also run a subset of tests (e.g. unit tests, but not integration tests).


As for that failure due to

class timeout:
def __init__(self, seconds=1, error_message="Timeout"):
self.seconds = seconds
self.error_message = error_message
def handle_timeout(self, signum, frame):
raise TimeoutError(self.error_message)
def __enter__(self):
signal.signal(signal.SIGALRM, self.handle_timeout)
signal.alarm(self.seconds)
def __exit__(self, type, value, traceback):
signal.alarm(0)

I'd suggest (to whoever looks into it) checking out https://pypi.org/project/pytest-timeout/. Ideally just use that plugin with pytest and not implement something custom. Short of that, see the notes about timeout methods and portability.

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

No branches or pull requests

2 participants