-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
feat: remove windows 32-bit support #18630
Conversation
Thanks for taking the time to open a PR!
|
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this line can also be removed:
cypress/scripts/binary/util/upload.js
Line 137 in 187e1c1
'ia32': 'win32-ia32', |
Q: What is the experience like for 32-bit Windows users after we remove 32-bit support? There is not going to be a matching binary to download, so how does the CLI handle this error case?
Q: Is there an accompanying PR to cypress-services
to remove Windows 32 bit support from the download server for Cypress >= 9.0.0?
case 'WIN32_UNSUPPORTED': | ||
return stripIndent`\ | ||
You are running a 32-bit build of Cypress. Cypress will remove Windows 32-bit support in a future release. | ||
You are attempting to run Cypress on Windows 32-bit. Cypress has removed Windows 32-bit support. | ||
|
||
${arg1 ? 'Try installing Node.js 64-bit and reinstalling Cypress to use the 64-bit build.' | ||
: 'Consider upgrading to a 64-bit OS to continue using Cypress in future releases.'} | ||
|
||
For more information, see: https://on.cypress.io/win32-removal | ||
: 'Consider upgrading to a 64-bit OS to continue using Cypress.'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there is a scenario where this error can be hit, since we will not be distributing/building the server for 32-bit Windows. See my review comment about investigating/adding an error in cli
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough, removed: c030f6d
(#18630)
|
@flotwig - I updated the missed line in cypress/scripts/binary/util/upload.js
I added a OS validation check in the install task of the CLI to thrown an error if the user attempts to install on an un-supported OS and/or arch.
I stared working on the changes. I have completed the changes, however I need AWS access to finish the tests. Once I have access I'll wrap that up. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There needs a docs PR for this bit: https://github.com/cypress-io/cypress-documentation/blob/master/content/guides/getting-started/installing-cypress.md#download-urls
I pushed a commit to test building this branch in AppVeyor, though I don't anticipate any issues: 147b921
Also, it looks like some unrelated changes got pulled into this PR, @emilyrohrbough could you clean up the diff?
Other than that LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @emilyrohrbough plz request me for review on the services PR when it's ready
Co-authored-by: Jennifer Shehane <jennifer@cypress.io> Co-authored-by: Zach Bloomquist <git@chary.us>
BREAKING CHANGE: Remove windows 32-bit support. Per @flotwig's investigations in September 2021, we feel this change should not impact users too adversely; 0.3% of Cypress's Windows desktop-gui users have were on Windows 32-bit were as 99.7% of users were using Windows 64-bit.
User facing changelog
Removed Windows 32-bit support.
How has the user experience changed?
Consumes will need to either update to use the Windows 64-bit cypress binary if possible or use a 64-bit OS system to continue to use Cypress.
PR Tasks
cypress-documentation
?