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

[Bug #18667] Define RUBY_API_VERSION on Windows #5736

Merged
merged 1 commit into from Mar 30, 2022

Conversation

peterzhu2118
Copy link
Member

On other platforms, RUBY_SO_NAME is defined from RUBY_API_VERSION.
ruby_version contains the ABI version, which is not needed.
RUBY_API_VERSION is defined as MAJOR.MINOR.

@MSP-Greg
Copy link
Contributor

@peterzhu2118 thanks for the fix. I ran it in Actions, and the log shows:

linking shared-library x64-vcruntime140-ruby32.dll

I think we want the following?

linking shared-library x64-vcruntime140-ruby320.dll

On other platforms, RUBY_SO_NAME is defined from RUBY_API_VERSION.
ruby_version contains the ABI version, which is not needed.
RUBY_API_VERSION is defined as MAJOR.MINOR.
@peterzhu2118
Copy link
Member Author

I've changed it to behave the same as mingw.

@MSP-Greg
Copy link
Contributor

MSP-Greg commented Mar 30, 2022

Thanks, I think that's probably best. I'll check right now.

@MSP-Greg
Copy link
Contributor

LGTM. Thanks for the quick fix.

JFYI, Window's Rubies have one dll in the bin folder, and, rather than start Ruby, setup-ruby for GitHub Actions checks that file to determine the platform (mingw, ucrt, or mswin). I was considering a change to the code and noticed the issue.

@peterzhu2118 peterzhu2118 merged commit 9f30661 into ruby:master Mar 30, 2022
@peterzhu2118 peterzhu2118 deleted the pz-windows-so-name branch March 30, 2022 22:08
@peterzhu2118
Copy link
Member Author

🤦‍♂️ I forgot to update the commit message..... oh well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants