-
Notifications
You must be signed in to change notification settings - Fork 5
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
Deprecate/Remove Cygwin support #134
Comments
I'm supporting (2) also, although a laconic statement similar to this one may suffice: https://github.com/Cantera/cantera/blob/9cee0525d26b6030cc30c469c9c75332d836eb8e/SConstruct#L86-L88 (e.g. replace 'unrecognized' by 'not supported') ... if necessary (which I do not think it is), I would strongly advocate moving to WSL instead, where a GH Action exists for setup. At the same time, there also is a GH Action for Cygwin, but my suspicion is that WSL may already work out of the box. |
@ischoegl Do you think we need to test WSL support? It should be a standard Linux distribution, so equivalent to our other Linux test cases, right? |
@bryanwweber ... agreed (I guess this means a no). I would suspect that it 'just works', but never had a reason to try. I believe cygwin was more relevant when compiler options were less abundant on Windows, but this is probably a thing of the past. |
Yes, I used Cygwin once upon a time... Probably back in the WinXP days before I "upgraded" to Vista and then truly upgraded to Win7 😄 |
Abstract
Cygwin support is untested and Cygwin has uncertain usage for Cantera. With the advent of (potentially) better replacements such as the Windows Subsystem for Linux v2 (WSL2), Cygwin support may not be necessary any more.
Motivation
Cygwin support imposes a mild burden on maintainers, with several untested code paths in various
SCons*
files. Retaining these support features without testing that they work is perhaps misleading to end users, if they don't work. Setting up and maintaining a test setup for Cygwin would likely not be worth the anticipated usage.Possible Solutions
Remove the following lines from the codebase, found by
grep -irn --exclude-dir=ext "cygwin" .
:Do part 1, and also raise a
ValueError
at the top ofSConstruct
if the OS is found to beCygwin
encouraging users to post on this issue that they require ongoing support. Remove theValueError
after the release of Cantera 2.6.Leave the code as-is.
Add a testing environment for Cygwin
My personal preference is for number 2.
The text was updated successfully, but these errors were encountered: