Fix Chocolatey log spam & bad nodejs-lts requirement #6475
Conversation
|
@Daniel15 Any chance you can have a look at this? This PR would allow to use the yarn Chocolatey package again with Node version different than LTS and solve the issue with the log output which confuses users. |
|
Oops, sorry I missed this! This seems mostly good to me. The file is intentionally called
If I remember correctly, this flag was copied as-is from a Chocolatey template for MSI-based installers.
The package intentionally had a Node.js dependency so that it could work out-of-the-box without the user having to know about Node.js. I guess outputting a message is fine. I wish Chocolatey had a way of specifying that both |
| if (-Not (Get-Command "node" -errorAction SilentlyContinue)) { | ||
| Write-Host "Yarn requires NodeJS to be installed. To install, use either of the commands below:" | ||
| Write-Host "choco install nodejs" | ||
| Write-Host "choco install nodejs-lts" |
There was a problem hiding this comment.
I would suggest to add this information also to the nuspec file as a note (like e.g. done here)
| packageName = 'yarn' | ||
| softwareName = 'Yarn*' | ||
| fileType = 'msi' | ||
| silentArgs = "/qn /norestart /l*v `"$env:TEMP\chocolatey\$($packageName)\$($packageName).MsiInstall.log`"" |
There was a problem hiding this comment.
While this solves the issue of the log output, it also disables logging at all. Upstream issue for this is chocolatey/choco#1016
There was a problem hiding this comment.
So our options are either a giant blob of Error looking xml or nothing. I prefer nothing but we can add back the error blob.
Renamed back. I think I just changed it because I was checking syntax highlighting and the PSAnalyzer and forgot to undo.
Yeah, nuget doesn't have the concept of |
3 participants
Summary
Fixes #6439
The chocolately install has always been full of log spam due to an incorrectly written
/l*vflag, but those logs are also unneeded since the output has no useful info so I just removed them (this can be reversed to just fix the flag if y'all really want it back).This also removes the bad
nodejs-ltsnuget dependency added in #5925. It now has no dependency, and instead logs out to inform the user to install node if it's missing.Test plan
choco uninstall yarn{VERSION}and{CHECKSUM}placeholders manually or do a full build (I manually replaced).\chocolateyinstall.ps1