(#1092) Ensure chocolateyBeforeModify.ps1 is run for dependencies #3051
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vexx32
force-pushed
the
1092-beforemodify
branch
2 times, most recently
from
9abae8a to
4c097f7
Compare
This addition is necessary, as many occasions may arise where installing one package involves updating an already-installed dependency. In these cases we should still be running a beforeModify script if it's present, and this facilitates that.
vexx32
force-pushed
the
1092-beforemodify
branch
3 times, most recently
from
663211a to
dde8edf
Compare
vexx32
force-pushed
the
1092-beforemodify
branch
from
dde8edf to
4531342
Compare
corbob
reviewed
Mar 10, 2023
...denciesbeforemodify/hasdependencywithbeforemodify/1.0.0/hasdependencywithbeforemodify.nuspec
Show resolved
Hide resolved
gep13
requested changes
Mar 13, 2023
...denciesbeforemodify/hasdependencywithbeforemodify/1.0.0/hasdependencywithbeforemodify.nuspec
Show resolved
Hide resolved
...denciesbeforemodify/hasdependencywithbeforemodify/2.0.0/hasdependencywithbeforemodify.nuspec
Show resolved
Hide resolved
|
@vexx32 for the tests that you are going to add to this, do we need to add any additional tests for hook packages to ensure that they continue to work as expected, or do the current tests cover the changes that you are making in this PR? |
I'll double check the existing tests for that, but I suspect we'll want to add an extra test for that to make sure they work as expected. EDIT: Yeah, I think we're mostly good, but I will add the hooks package into the test and add some checks for the pre-beforeModify hook to make sure it's running each time. |
Previously we only ran backups and beforeModify scripts on the target package(s) and not their dependencies. This change should ensure we are _always_ running backups on any existing packages from a NuGet source before we try to modify them in any way. Additionally, this should ensure we can run the beforeModify scripts for ALL dependencies of the target package(s).
This test added ensures that when we are upgrading a package, both its own and any dependencies' beforeModify are run so that we can properly update dependencies which might need to shut down a long-running service before being upgraded.
vexx32
force-pushed
the
1092-beforemodify
branch
from
4531342 to
49cc8d7
Compare
|
@gep13 Alright, I think the tests I've added are up to scratch now, we should be good to review this as-is. 🙂 |
|
The failing Ubuntu build here is an issue with NuGet Restore, and nothing to do with the changes in this PR. |
|
@vexx32 thank you very much for getting this done! |
10 tasks
vexx32
added a commit
to vexx32/choco
that referenced
this pull request
Oct 7, 2024
This migration path hasnt been supported or used since pre-1.0 versions of Chocolatey and is dead code. Removing this now to simplify the install process somewhat and get the old code out of the way as it's not used at all.
vexx32
added a commit
to vexx32/choco
that referenced
this pull request
Oct 15, 2024
Rewrite of the Get-ChocolateyPath into C# cmdlet, and associated changes to helper methods.
vexx32
added a commit
to vexx32/choco
that referenced
this pull request
Oct 15, 2024
Rewrite of the Get-ChocolateyPath into C# cmdlet, and associated changes to helper methods.
vexx32
added a commit
to vexx32/choco
that referenced
this pull request
Oct 15, 2024
Rewrite of the Get-ChocolateyPath into C# cmdlet, and associated changes to helper methods.
vexx32
added a commit
to vexx32/choco
that referenced
this pull request
Oct 15, 2024
To ensure behaviour works as expected
vexx32
added a commit
to vexx32/choco
that referenced
this pull request
Oct 18, 2024
Test all the things!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description Of Changes
choco installoperation.Motivation and Context
This improves the overall soundness of our package operations, and fixes an issue where updating Chocolatey licensed packages can trip over the chocolatey-agent package if it ends up being updated during processing of dependencies (for instance, by updating one of its dependencies outside of its currently permitted version range, while a newer agent package is available that will work with the newer dependency version.
Testing
chocolateygui.extensionpackage. With this change, the upgrade should correctly run the chocolatey-agent beforeModify script and succeed smoothly.There are a couple unit tests I'd still like to add, but the currently-added ones cover the main priority here at the moment.
Operating Systems Testing
Win10, Win11
Change Types Made
Change Checklist
Related Issue
Fixes #1092