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

Use stock Rtools toolchain by default for RTools44+ #1054

Merged
merged 9 commits into from
Jan 7, 2025

Conversation

andrjohns
Copy link
Collaborator

Submission Checklist

  • Run unit tests
  • Declare copyright holder and agree to license (see below)

Summary

From CmdStan 2.35 and RTools44 onwards we do not need to download/install any additional toolchains/utilities (e.g., mingw32-make). This PR updates our handling of toolchains to default to stock RTools (for rtools44 only), unless manually requested by the user setting the CMDSTANR_USE_MSYS_TOOLCHAIN environment variable.

Copyright and Licensing

Please list the copyright holder for the work you are submitting
(this will be you or your assignee, such as a university or company):
Andrew Johnson

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the following licenses:

@andrjohns andrjohns force-pushed the stock-rtools-default branch from 56ee6b3 to fe07ccd Compare January 5, 2025 03:30
Copy link
Member

@jgabry jgabry left a comment

Choose a reason for hiding this comment

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

Great, thanks! Looks good. Should we add something about setting CMDSTANR_USE_MSYS_TOOLCHAIN to the doc before merging?

@andrjohns
Copy link
Collaborator Author

Great, thanks! Looks good. Should we add something about setting CMDSTANR_USE_MSYS_TOOLCHAIN to the doc before merging?

Ah good point! I've fleshed out the documentation in install_cmdstan, let me know if it would be better in a different place (or with more detail)

@jgabry
Copy link
Member

jgabry commented Jan 7, 2025

Looks good, thanks!

@andrjohns andrjohns merged commit 79d3779 into master Jan 7, 2025
11 checks passed
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.

2 participants