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

Improve Windows setup script #673

Merged
merged 15 commits into from Apr 10, 2023
Merged

Improve Windows setup script #673

merged 15 commits into from Apr 10, 2023

Conversation

HeavenVolkoff
Copy link
Member

@HeavenVolkoff HeavenVolkoff commented Apr 4, 2023

Hello,

During testing of my QuickPreview implementation (PR #665) on Windows, I encountered some problems setting up the project. This PR addresses those issues by changing the Windows setup script to make it more robust, less redundant, and to fix recent problems caused by major updates to PNPM and LLVM that were breaking installation and compilation on Windows.

These changes include:

  • Ensuring Powershell is running as an administrator (required by Chocolatey)
  • Retrieving the project directory relative to the script path, instead of using the current directory
  • Installing pnpm version 7 (version 8 is incompatible with some dependencies)
  • Installing LLVM 15 (LLVM 16 breaks compilation of ffmpeg-sys-next)
  • Installing Chocolatey if not available
  • Updating ffmpeg version from 5.0.1 to 5.1.1
  • Formatting the script with PSScriptAnalyzer

 - Change LLVM install logic to use version 15 instead of the latest, due to incompatibilities of new LLVM (16) with `ffmpeg-sys-next` build script
 - Add check to ensure script is executed as administrator, due to chocolatey requiring admin privileges to install `protoc`
 - Update ffmpeg to 5.1.1 to match used `ffmpeg-sys-next` version
 - Don't attempt to reinstall LLVM when it is already installed
@vercel
Copy link

vercel bot commented Apr 4, 2023

@HeavenVolkoff is attempting to deploy a commit to the Spacedrive Team on Vercel.

A member of the Team first needs to authorize it.

@HeavenVolkoff HeavenVolkoff marked this pull request as draft April 4, 2023 19:14
 - Remove dependency in Chocolatey
 - Download ffmpeg version based on ffmpeg-sys-next version
 - Improve version checks
 - Install protobuf directly from github
 - Update rimraf so globbing works in Windows
 - Avoid dist/.gitignore being deleted by rimraf
- Check for connectivity with GitHub before proceeding with setup.
- Implement a workaround for pnpm bootstrap problems on Windows.
- Check if Cargo packages are up to date before parsing the metadata.
- Ensure that the FFMPEG_DIR environment variable is set.
- Use double quotes for rimraf arguments on Windows.
 - Fix protoc envvars not being set in CI
 - Improve logs
 - Remove sleeps
 - Uninstall any imcompatible LLVM version present
 - Ensure path is fresh after installing rust and cargo
 - Install node with winget and pnpm with npm
 - Rewrite initial script message
 - Don't wait for user interaction when running in CI
 - Warn users if they have already installed incompatible versions of LLVM and pnpm instead of silently removing them
…tup script

 - Improve error handling
 - Improve logs
@HeavenVolkoff
Copy link
Member Author

So, I ended up rewriting the entire script. I'm sorry about that, but now it will bootstrap the entire development environment from scratch without the need for user intervention, as long as they have winget installed (which is the default for Windows 11 and recent Windows 10 updates) and no errors occur. I believe this will allow for easier setup for new Windows contributors.

For the major changes I made to the original script, I replaced the pnpm installer with installing node from winget and installing pnpm afterward through npm. This was necessary because the pnpm installer has some annoying unresolved bugs on Windows right now (see pnpm/pnpm#5700 and pnpm/pnpm#5266). Additionally, I changed the ffmpeg install logic to get the necessary version to install from the cargo declared version for ffmpeg-sys-next.

@HeavenVolkoff HeavenVolkoff marked this pull request as ready for review April 6, 2023 19:02
Copy link
Member

@jamiepine jamiepine left a comment

Choose a reason for hiding this comment

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

This is huge as Windows has been the hardest platform for new developers to jump in with. Thank you!

@HeavenVolkoff HeavenVolkoff self-assigned this Apr 7, 2023
@HeavenVolkoff HeavenVolkoff added this pull request to the merge queue Apr 10, 2023
Merged via the queue into spacedriveapp:main with commit fd76705 Apr 10, 2023
7 of 9 checks passed
@HeavenVolkoff HeavenVolkoff deleted the fix-windows-setup-script branch April 10, 2023 13:39
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.

None yet

2 participants