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.

Sign up for GitHub

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

Jump to bottom

[3006.x] Don't remove extras dir on uninstall #65038

Merged
merged 7 commits into from
Aug 31, 2023
Merged

[3006.x] Don't remove extras dir on uninstall #65038

merged 7 commits into from
Aug 31, 2023

Conversation

twangboy
Copy link
Contributor

What does this PR do?

  • Don't remove the entire salt directory by default.

What issues does this PR fix or reference?

Fixes: #64957

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

@twangboy twangboy requested a review from a team as a code owner August 23, 2023 20:17
@twangboy twangboy requested review from MKLeb and removed request for a team August 23, 2023 20:17
@salt-project-bot-prod-environment salt-project-bot-prod-environment bot changed the title Don't remove extras dir on uninstall [3006.x] Don't remove extras dir on uninstall Aug 23, 2023
@twangboy twangboy added this to the Sulfur v3006.3 milestone Aug 23, 2023
MKLeb
MKLeb previously approved these changes Aug 23, 2023
View reviewed changes
@twangboy twangboy temporarily deployed to ci August 23, 2023 20:48 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 23, 2023 20:48 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 23, 2023 20:48 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 23, 2023 20:49 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 23, 2023 21:03 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 23, 2023 21:09 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 23, 2023 22:04 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 23, 2023 22:04 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 23, 2023 22:04 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 23, 2023 22:04 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 23, 2023 22:04 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 23, 2023 22:04 — with GitHub Actions Inactive
Ch3LL
Ch3LL suggested changes Aug 24, 2023
View reviewed changes
Copy link
Contributor

@Ch3LL Ch3LL left a comment

Choose a reason for hiding this comment

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

Can we add test coverage?

s0undt3ch
s0undt3ch previously approved these changes Aug 25, 2023
View reviewed changes
@MKLeb
Copy link
Contributor

MKLeb commented Aug 28, 2023

This may be a matter of semantics, but the title says to not remove the directory on uninstall. Is this what happens during upgrades and downgrades (i.e. installing the desired package while the undesired one is still on the system), or is that process different than explicitly uninstalling the package?

@twangboy twangboy dismissed stale reviews from MKLeb and s0undt3ch via bfb806e August 29, 2023 22:55
@twangboy twangboy temporarily deployed to ci August 29, 2023 23:28 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 29, 2023 23:29 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 30, 2023 00:15 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 30, 2023 00:15 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 30, 2023 00:15 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 30, 2023 00:15 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 30, 2023 00:15 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 30, 2023 00:15 — with GitHub Actions Inactive
@twangboy
Copy link
Contributor Author

@MKLeb When the installer finds an existing package on the system, it launches the existing package's uninstaller. So, the logic to leave behind the extras-3.10 directory lies there.

@twangboy twangboy temporarily deployed to ci August 30, 2023 15:01 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 30, 2023 15:01 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 30, 2023 15:01 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 30, 2023 15:01 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 30, 2023 15:01 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 30, 2023 15:02 — with GitHub Actions Inactive
@twangboy twangboy requested review from Ch3LL, s0undt3ch and MKLeb August 30, 2023 15:08
@twangboy twangboy temporarily deployed to ci August 31, 2023 16:35 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 31, 2023 16:35 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 31, 2023 16:35 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 31, 2023 16:35 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 31, 2023 16:53 — with GitHub Actions Inactive
@twangboy twangboy temporarily deployed to ci August 31, 2023 16:54 — with GitHub Actions Inactive
@garethgreenaway garethgreenaway merged commit 2d5584a into saltstack:3006.x Aug 31, 2023
@Ch3LL Ch3LL mentioned this pull request Sep 13, 2023
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MKLeb MKLeb MKLeb approved these changes

@garethgreenaway garethgreenaway garethgreenaway approved these changes

@s0undt3ch s0undt3ch s0undt3ch approved these changes

@Ch3LL Ch3LL Awaiting requested review from Ch3LL

Assignees
No one assigned
Labels
Sulfur v3006.3
Projects
None yet
Milestone
Sulfur v3006.3
Development

Successfully merging this pull request may close these issues.
5 participants
@twangboy @MKLeb @s0undt3ch @garethgreenaway @Ch3LL