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

Remove claims that Cygwin is supported #34301

Closed
mkoeppe opened this issue Aug 7, 2022 · 39 comments
Closed

Remove claims that Cygwin is supported #34301

mkoeppe opened this issue Aug 7, 2022 · 39 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Aug 7, 2022

In the 9.6 release cycle, https://groups.google.com/g/sage-release/c/GOGWk66zaCQ/m/M_jQiOjaAAAJ:
Users/developers who like to continue using the Cygwin port are asked to step up to investigating and fixing these bugs on Cygwin. Otherwise, it is unlikely that we'll be able to continue offering the Cygwin port.

This has not happened, and on top of the problems at runtime in Sage 9.6, there are now several issues that prevent a successful installation on Cygwin.

To avoid user frustration (2022 sage-devel posts: https://groups.google.com/g/sage-devel/c/_z1Fv_OH1mk/m/nYZFkRi7AQAJ, https://groups.google.com/g/sage-devel/c/JNDU9Fa0Ths/m/2zDrFH91AQAJ, https://groups.google.com/g/sage-devel/c/qB8CU0JdC1I/m/jFUeImPZAAAJ, https://groups.google.com/g/sage-devel/c/pXN2B1NKpsQ/m/Vbn0HHN_FQAJ), we update README.md and manuals to reflect reality.

Announcement:

CC: @jhpalmieri @tscrim @slel

Component: porting: Cygwin

Author: Matthias Koeppe

Branch/Commit: cd72e6b

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/34301

@mkoeppe mkoeppe added this to the sage-9.7 milestone Aug 7, 2022
@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented Aug 7, 2022

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented Aug 7, 2022

New commits:

d2a0373Remove Cygwin from installation guide and README

@mkoeppe
Copy link
Member Author

mkoeppe commented Aug 7, 2022

Author: Matthias Koeppe

@mkoeppe
Copy link
Member Author

mkoeppe commented Aug 7, 2022

Commit: d2a0373

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented Aug 7, 2022

comment:8

Also https://wiki.sagemath.org/SageWindows needs to be updated

And https://github.com/sagemath/sage-windows should be archived

And https://www.sagemath.org/download.html needs to be updated

@mkoeppe
Copy link
Member Author

mkoeppe commented Aug 7, 2022

comment:9

Replying to @mkoeppe:

Also https://wiki.sagemath.org/SageWindows needs to be updated

I've removed a big chunk of outdated information.

There is a bit of info on there that should be reviewed for usefulness for #31156 (Document running Sage + Jupyter in WSL, with a browser in Windows)

@tscrim
Copy link
Collaborator

tscrim commented Aug 8, 2022

comment:11

It seems like we have a more nuanced description of support we have for it. It seems like the biggest issue is in docbuilding and (recently) giac. If we fully drop support for it as the ticket proposes, then almost certainly we will never get it back.

I am willing to do what I can to help as I have a Windows machine (that previously built Sage successfully on at least 9.5 IIRC, but I think it was one of the 9.6 beta versions). Although I am a bit stretched thin right now and package building is not something I know much about. However, I do think it is not something we should drop for WSL for the next few years until we are more certain most people have migrated to a Windows version that has WSL.

If we do decide to fully drop support (rather than just being honest about what we know doesn't work), we might want to include instructions to build the last stable version that we know works.

@tscrim
Copy link
Collaborator

tscrim commented Aug 8, 2022

comment:12

Replying to @mkoeppe:

Replying to @mkoeppe:

Also https://wiki.sagemath.org/SageWindows needs to be updated

I've removed a big chunk of outdated information.

There is a bit of info on there that should be reviewed for usefulness for #31156 (Document running Sage + Jupyter in WSL, with a browser in Windows)

Does that also work for pdfs (e.g., view(x^2 + cos(x))) and animate() outputs?

@mkoeppe
Copy link
Member Author

mkoeppe commented Aug 8, 2022

comment:13

Replying to @tscrim:

If we fully drop support for it as the ticket proposes, then almost certainly we will never get it back.

It's trivial to revert the doc changes when someone fixes the build.

Until then it's crucial that we don't mislead prospective users.

@tscrim
Copy link
Collaborator

tscrim commented Aug 8, 2022

comment:14

Replying to @mkoeppe:

Replying to @tscrim:

If we fully drop support for it as the ticket proposes, then almost certainly we will never get it back.

It's trivial to revert the doc changes when someone fixes the build.

The documentation change is trivial, yes. However, it is the policy change of no longer putting any effort to care about Cygwin builds means is will further bitrot. So it will likely take much more effort to resupport Cygwin in the future.

Until then it's crucial that we don't mislead prospective users.

+1 but this ticket isn't just about that. The biggest issue with Cygwin right now is the building of giac #34269, right? Once this is resolved, it is expected that Sage builds, starts, and passes (most) tests on Cygwin, right?

@mkoeppe
Copy link
Member Author

mkoeppe commented Aug 8, 2022

comment:15

Replying to @tscrim:

The biggest issue with Cygwin right now is the building of giac #34269, right?
Once this is resolved, it is expected that Sage builds, starts, and passes (most) tests on Cygwin, right?

No, it was already badly broken in the 9.6 series. Per ​https://groups.google.com/g/sage-release/c/GOGWk66zaCQ/m/M_jQiOjaAAAJ (March 2022) "various segfaults, likely related to GAP", in the testsuite.

The broken Giac build stops us from seeing other build problems with packages that we updated in the 9.7 series. Giac upstream does not test Cygwin, and I have no line of communication with them. There's also another critical bug in Giac (#33848) and no reaction from upstream.

@mkoeppe
Copy link
Member Author

mkoeppe commented Aug 8, 2022

comment:16

Replying to @tscrim:

If we do decide to fully drop support (rather than just being honest about what we know doesn't work), we might want to include instructions to build the last stable version that we know works.

The release notes (https://trac.sagemath.org/wiki/ReleaseTours/sage-9.7#Sage9.7doesnotsupportCygwinuseWindowsSubsystemforLinuxinstead) are a good place for this, but not the documentation

@mkoeppe
Copy link
Member Author

mkoeppe commented Aug 9, 2022

comment:17

Replying to @tscrim:

Replying to @mkoeppe:

Replying to @tscrim:

If we fully drop support for it as the ticket proposes, then almost certainly we will never get it back.

It's trivial to revert the doc changes when someone fixes the build.

The documentation change is trivial, yes. However, it is the policy change of no longer putting any effort to care about Cygwin builds means is will further bitrot. So it will likely take much more effort to resupport Cygwin in the future.

Until then it's crucial that we don't mislead prospective users.

+1 but this ticket isn't just about that.

I worded https://trac.sagemath.org/wiki/ReleaseTours/sage-9.7#Sage9.7doesnotsupportCygwinuseWindowsSubsystemforLinuxinstead carefully -- it does not say that we have "dropped" Cygwin support; it only says that Sage 9.7 doesn't support it, and it lists the issues.

Also note that this ticket does not remove the Cygwin testing infrastructure -- in fact, I made major investments in it during the 9.7 cycle - #33791, which makes it very easy for upstream projects to reuse this infrastructure to test the portability of their project. (see for example https://github.com/latte-int/latte/blob/master/.github/workflows/ci-sage.yml#L93)
But we need engagement of developers on our side and in upstream projects with the Cygwin port.

@mkoeppe

This comment has been minimized.

@tscrim
Copy link
Collaborator

tscrim commented Aug 11, 2022

comment:19

Thanks. I weakened the language should -> can to further indicate that we hope to bring back Cygwin support in future versions.

I will also try to be more active in maintaining Cygwin support, but starting sometime next month, I will become very busy with family stuff. At the very least, I can test stuff on my Windows machine.

@tscrim
Copy link
Collaborator

tscrim commented Aug 11, 2022

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Aug 11, 2022

comment:20

I should point out that we should make similar changes here for this ticket. In particular, saying Cygwin support has discontinued means, to me, that we will not bring back support later on.

Thinking a bit more, I think we should leave the Cygwin installation guide in the docs because someone might want to install, say, 9.5 using Cygwin. This way the instructions are still there for people who might need to use Cygwin and are fine with an older version.

@tscrim
Copy link
Collaborator

tscrim commented Aug 11, 2022

comment:29

Thank you. This is okay with me.

@mkoeppe
Copy link
Member Author

mkoeppe commented Aug 11, 2022

comment:30

Doc does not build -

[sagemath_doc_html-none] [installat] /__w/sagetrac-mirror/sagetrac-mirror/src/doc/en/installation/source.rst:359: WARNING: Inline interpreted text or phrase reference start-string without end-string.
[sagemath_doc_html-none] [installat] /__w/sagetrac-mirror/sagetrac-mirror/src/doc/en/installation/source.rst:359: WARNING: hardcoded link 'https://trac.sagemath.org/query?status=closed&status=needs_info&status=needs_review&status=needs_work&status=new&status=positive_review&component=porting%3A+Cygwin&milestone=sage-9.8&milestone=sage-9.7&milestone=sage-9.6&milestone=sage-9.5&milestone=sage-9.4&milestone=sage-9.3&milestone=sage-9.2&milestone=sage-9.1&col=id&col=summary&col=milestone&col=status&col=priority&col=changetime&col=author&col=reviewer&desc=1&order=changetime' could be replaced by an extlink (try using ':trac:`query?status=closed&status=needs_info&status=needs_review&status=needs_work&status=new&status=positive_review&component=porting%3A+Cygwin&milestone=sage-9.8&milestone=sage-9.7&milestone=sage-9.6&milestone=sage-9.5&milestone=sage-9.4&milestone=sage-9.3&milestone=sage-9.2&milestone=sage-9.1&col=id&col=summary&col=milestone&col=status&col=priority&col=changetime&col=author&col=reviewer&desc=1&order=changetime`' instead)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 11, 2022

Changed commit from 550b337 to e3af6c9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 11, 2022

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

e3af6c9src/doc/en/installation/index.rst: Fix markup

@vbraun
Copy link
Member

vbraun commented Aug 28, 2022

comment:33

PDF docs don't build

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2022

Branch pushed to git repo; I updated commit sha1. New commits:

cd72e6bsrc/doc/en/installation/source.rst: Fix up markup of material converted from README.md

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 28, 2022

Changed commit from e3af6c9 to cd72e6b

@mkoeppe
Copy link
Member Author

mkoeppe commented Aug 28, 2022

comment:35

Sorry, fixed now

@vbraun
Copy link
Member

vbraun commented Aug 30, 2022

Changed branch from u/mkoeppe/remove_claims_that_cygwin_is_supported to cd72e6b

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

No branches or pull requests

3 participants