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

hack: undef status_interrupted to fix windows build #467

Merged
merged 2 commits into from
Oct 11, 2023

Conversation

ahmed-irfan
Copy link
Member

@ahmed-irfan ahmed-irfan commented Oct 10, 2023

this hack is a temporary fix for the windows build.

A proper fix (#466) will go into the Yices 2.8 release.

@coveralls
Copy link

coveralls commented Oct 10, 2023

Coverage Status

coverage: 65.098%. remained the same when pulling 4bea5b9 on patch-iss-418-prerelease into 95f13c6 on master.

@markpmitchell
Copy link
Contributor

markpmitchell commented Oct 10, 2023 via email

@ahmed-irfan
Copy link
Member Author

There’s no backwards compatibility problem on MINGW if the code never compiled before. Did it? If it didn’t, you could do this: 1. In the enum definition use YICES_STATUS_INTERRUPTED. 2. Elsewhere in yices.h add: /* To avoid breaking backwards compatibility until Yices 2.8. */ #ifndef MINGW #define STATUS_INTERRUPTED YICES_STATUS_INTERRUPTED #endif Then, change all the .c files to use YICES_STATUS_INTERRUPTED. That avoids changing the API for 2.7 update releases on non-Windows platforms. And it avoids undef’ing a Windows API macro. FWIW,

I like it. Updating the PR accordingly.
Thanks Mark!

@disteph
Copy link
Contributor

disteph commented Oct 10, 2023

I agree with that solution.

Copy link
Contributor

@markpmitchell markpmitchell left a comment

Choose a reason for hiding this comment

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

Looks good!

@ahmed-irfan ahmed-irfan merged commit 36dedf1 into master Oct 11, 2023
23 checks passed
@ahmed-irfan ahmed-irfan deleted the patch-iss-418-prerelease branch October 11, 2023 01:28
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