Skip to content

Remove support for building PE files from hyperlight-guest-bin build.rs #572

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

Merged

Conversation

simongdavies
Copy link
Contributor

This pull request simplifies the build process for the hyperlight_guest_bin by removing support for Windows-specific tools and configurations since we no longer support PE guests.

@simongdavies simongdavies added the kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. label Jun 5, 2025
ludfjig
ludfjig previously approved these changes Jun 5, 2025
Copy link
Contributor

@ludfjig ludfjig left a comment

Choose a reason for hiding this comment

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

Looks reasonable, maybe @syntactically could have a look as she wrote the code

@simongdavies simongdavies force-pushed the remove-pe-support-from-build.rs branch from d16d57c to 1193045 Compare June 5, 2025 21:18
@simongdavies simongdavies force-pushed the remove-pe-support-from-build.rs branch from 1193045 to 04757e5 Compare June 17, 2025 13:24
@simongdavies simongdavies force-pushed the remove-pe-support-from-build.rs branch 2 times, most recently from de7e451 to e9b327d Compare June 17, 2025 19:54
Copy link
Contributor

@syntactically syntactically left a comment

Choose a reason for hiding this comment

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

Mostly LGTM except for a couple of little things :)

@simongdavies simongdavies force-pushed the remove-pe-support-from-build.rs branch from e9b327d to e9f405f Compare June 25, 2025 18:06
@simongdavies
Copy link
Contributor Author

@syntactically @jprendes please take another look when you have a moment

@simongdavies simongdavies force-pushed the remove-pe-support-from-build.rs branch from e9f405f to c37c889 Compare June 25, 2025 18:12
Copy link
Contributor

@syntactically syntactically left a comment

Choose a reason for hiding this comment

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

LGTM.

Your comment about not seeing failures with clang.exe made me realise that I'm not sure if we actually have any tests in core hyperlight that make sure this toolchain works to build something; we used to use it to build WAMR when we were experimenting with that, but I don't think we have anything using it right now. It would be nice to add a test guest that actually builds libz or openssl or something with it, though. I don't know if it makes sense to that as part of this PR, but maybe add an issue for it if not?

@simongdavies simongdavies merged commit 78718c4 into hyperlight-dev:main Jun 26, 2025
107 of 113 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement For PRs adding features, improving functionality, docs, tests, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants