-
Notifications
You must be signed in to change notification settings - Fork 865
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
chore: helper scripts for updating chocolatey #1360
Conversation
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.
Thank you, automating this is super useful!
Before we merge, I want to address a small concern below.
appveyor.yml
Outdated
@@ -22,4 +22,14 @@ build_script: | |||
- npm run test:e2e | |||
- npm run build | |||
|
|||
on_success: | |||
- ps: | | |||
if ($env:APPVEYOR_REPO_COMMIT_MESSAGE -Match "pubchoco") { |
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.
Ugh, github lost my comment, so writing again...
@hacdias Appveyor runs on every PR. In theory someone could PR a downgrade attack with nuspec
pointing at older version. What would Cocolatey do in that case? Reject? Override latest version with old one?
Perhaps we could harden this to run only if branch name is master
and build is not a PR
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.
Changed. However, couldn't someone just change the code? And mame a PR and hijack the releases anyways? There's a way to define scripts that does not include .appveyor.yml
(see docs). However, someone with enough knowledge could trigger the update too...
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.
Actually not, if we define like they do on the docs (on the UI itself), it won't be on .appveyor.yml
thus it will be locked to master
.
@lidel so I removed that bit from .appveyor.yml and added it to the interface: This way no one can change it! |
License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>
License: MIT Signed-off-by: Henrique Dias <hacdias@gmail.com>
6c4f075
to
333bae2
Compare
@hacdias I tried I know you don't have bandwidth for looking into this, so let's leave automation for now. |
@lidel done. I'm sad it didn't work. Looking at the logs, it doesn't even seem that the script was executed... |
Automation set up on AppVeyor is broken, let's go with manual steps for now: #1360 (comment) License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
Automation set up on AppVeyor is broken, let's go with manual steps for now: #1360 (comment) License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
This PR creates an helper script to help with the chocolatey update procedure. After releasing a version, wait for the binaries to be available on the releases page. Then:
node pkgs/chocolatey/update.js $version
git commit -m "chore: update choco [pubchoco]"
- thepubchoco
part must be theregit push
and wait for AppVeyor which should publish the package.Obviously I could not test this. The script for updating the nuspec file works well. We just need to test the third step on the next release.