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

(GH-1689) Remove packaging script before upgrade #1751

Closed
wants to merge 2 commits into from

Conversation

gep13
Copy link
Member

@gep13 gep13 commented Mar 12, 2019

It is possible, due to the way that Package Reducer works, that there
are extra files left in the package directory when doing an upgrade
of a package. This can cause a package upgrade to fail. Prior to
performing the upgrade, delete all packaging scripts (chocolateyInstall,
chocolateyUninstall, and chocolateyBeforeModify) from the package
folder, these will then be replaced during the upgrade, if they are
still required.

Closes #1689

Copy link
Member

@ferventcoder ferventcoder left a comment

Choose a reason for hiding this comment

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

👍

@ferventcoder
Copy link
Member

ferventcoder commented Mar 12, 2019

Of course there would be an integration test that verifies what is removed and what should not be, so it will need adjusted to account for this. I can manage grabbing that if I finish it tonight. Otherwise you may want to review the failed AppVeyor log and take a look at the test to update it for this behavior.

gep13 added 2 commits March 13, 2019 09:31
It is possible, due to the way that Package Reducer works, that there
are extra files left in the package directory when doing an upgrade
of a package.  This can cause a package upgrade to fail.  Prior to
performing the upgrade, delete all packaging scripts (chocolateyInstall,
chocolateyUninstall, and chocolateyBeforeModify) from the package
folder, these will then be replaced during the upgrade, if they are
still required..
@gep13
Copy link
Member Author

gep13 commented Mar 13, 2019

@ferventcoder I went down the route of changing the tests, but then I realised that the function that I added wasn't constrained to only happen during an upgrade, which I believe it should be. I have added this in, and the tests that were failing are now all passing again.

Let me know if you want additional changes.

@ferventcoder
Copy link
Member

This was rebased against stable and merged in at cb2799b. Thanks!

@gep13 gep13 deleted the issue-1689 branch September 9, 2019 18:36
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.

2 participants