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

Fix gem.py, hang on uninstall specific gem version #5751

Merged
merged 3 commits into from
Jan 7, 2023
Merged

Fix gem.py, hang on uninstall specific gem version #5751

merged 3 commits into from
Jan 7, 2023

Conversation

rietvelde
Copy link
Contributor

move 'cmd.append('--executable')' to all uninstalls rather than only all versions

SUMMARY

When uninstalling a specific version of a gem the playbook execution will hang if there are executables to be removed as gem waits for user input.

Issue does not occur when uninstalling all versions of a gem as '--executable' is added to the command line.
The option --executable should be added with any uninstall regardless if uninstalling a specific version or all versions.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

gem

ADDITIONAL INFORMATION
# results in hanging playbook
ansible.builtin.gem:
    name: abc
    state: absent
    version: 1.2.3

@rietvelde
Update gem.py
935cc8d 
move 'cmd.append('--executable')' to all uninstalls rather than only all versions
@ansibullbot
Copy link
Collaborator

cc @None @johanwiren
click here for bot help

@ansibullbot ansibullbot added bug This issue/PR relates to a bug language module module new_contributor Help guide this first time contributor packaging plugins plugin (any type) small_patch Hopefully easy to review labels Jan 2, 2023
@felixfontein
Copy link
Collaborator

Thanks for your contribution. Could you please add a changelog fragment? Thanks.

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-5 labels Jan 2, 2023
@felixfontein
Copy link
Collaborator

I don't use gem and am not familiar with it, but couldn't this cause a problem if multiple versions of that gem are installed? I.e. couldn't it remove the executables for all versions?

@ansibullbot ansibullbot removed the small_patch Hopefully easy to review label Jan 3, 2023
@rietvelde
Copy link
Contributor Author

Added changelog

Gem will only remove an executable if there are no versions left that provide it

For example

gem install rake -v 13.0.5
gem install rake -v 13.0.6
gem uninstall rake -v 13.0.5 --executable

Will not remove the rake executable

Only when uninstalling the last version that provided the executable will it interactively prompt

gem uninstall rake -v 13.0.6
Remove executables:
        rake

in addition to the gem? [Yn]

@ansibullbot

This comment was marked as outdated.

Sign in to view
@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jan 3, 2023
@ansibullbot ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jan 3, 2023
@felixfontein
Copy link
Collaborator

Thanks for the clarification!

felixfontein
felixfontein approved these changes Jan 3, 2023
View reviewed changes
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

I'll merge this for the next release (at the end of this month) if nobody objects.

(I'm not merging it for tomorrow's release since I want to give others the chance to comment on this, as I really don't know the gem module...)

russoz
russoz approved these changes Jan 7, 2023
View reviewed changes
Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

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

LGTM

@felixfontein felixfontein merged commit 2670215 into ansible-collections:main Jan 7, 2023
@patchback
Copy link

patchback bot commented Jan 7, 2023 •
edited
Loading

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/2670215c8ad6b18aac2220e1261818ed7f5f45cb/pr-5751

Backported as #5779

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jan 7, 2023
patchback bot pushed a commit that referenced this pull request Jan 7, 2023
@rietvelde @patchback
Fix gem.py, hang on uninstall specific gem version (#5751)
109659d 
* Update gem.py

move 'cmd.append('--executable')' to all uninstalls rather than only all versions

* Create 5751-gem-fix-uninstall-hang

* Rename 5751-gem-fix-uninstall-hang to 5751-gem-fix-uninstall-hang.yml

(cherry picked from commit 2670215)
@patchback patchback bot mentioned this pull request Jan 7, 2023
Merged
@patchback
Copy link

patchback bot commented Jan 7, 2023 •
edited
Loading

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/2670215c8ad6b18aac2220e1261818ed7f5f45cb/pr-5751

Backported as #5780

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jan 7, 2023
@rietvelde @patchback
Fix gem.py, hang on uninstall specific gem version (#5751)
2f96ff3 
* Update gem.py

move 'cmd.append('--executable')' to all uninstalls rather than only all versions

* Create 5751-gem-fix-uninstall-hang

* Rename 5751-gem-fix-uninstall-hang to 5751-gem-fix-uninstall-hang.yml

(cherry picked from commit 2670215)
@patchback patchback bot mentioned this pull request Jan 7, 2023
Merged
@felixfontein
Copy link
Collaborator

@rietvelde thanks for your contribution!
@russoz thanks for reviewing!

felixfontein pushed a commit that referenced this pull request Jan 7, 2023
@patchback @rietvelde
[PR #5751/2670215c backport][stable-6] Fix gem.py, hang on uninstall …
16e05ab 
…specific gem version (#5780)

Fix gem.py, hang on uninstall specific gem version (#5751)

* Update gem.py

move 'cmd.append('--executable')' to all uninstalls rather than only all versions

* Create 5751-gem-fix-uninstall-hang

* Rename 5751-gem-fix-uninstall-hang to 5751-gem-fix-uninstall-hang.yml

(cherry picked from commit 2670215)

Co-authored-by: rietvelde <99407273+rietvelde@users.noreply.github.com>
felixfontein pushed a commit that referenced this pull request Jan 7, 2023
@patchback @rietvelde
[PR #5751/2670215c backport][stable-5] Fix gem.py, hang on uninstall …
2a6f91b 
…specific gem version (#5779)

Fix gem.py, hang on uninstall specific gem version (#5751)

* Update gem.py

move 'cmd.append('--executable')' to all uninstalls rather than only all versions

* Create 5751-gem-fix-uninstall-hang

* Rename 5751-gem-fix-uninstall-hang to 5751-gem-fix-uninstall-hang.yml

(cherry picked from commit 2670215)

Co-authored-by: rietvelde <99407273+rietvelde@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@russoz russoz russoz approved these changes

@felixfontein felixfontein felixfontein approved these changes

Assignees
No one assigned
Labels
bug This issue/PR relates to a bug language module module new_contributor Help guide this first time contributor packaging plugins plugin (any type)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rietvelde @ansibullbot @felixfontein @russoz