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

Limit search pattern for remove command #1126

Merged
merged 3 commits into from
Jan 8, 2024

Conversation

j-mracek
Copy link
Contributor

@j-mracek j-mracek commented Jan 4, 2024

It prevents additional removal after succesful remove using the same spec. Spec foo. The original code during the first run removes (dnf5 remove foo) all packages with the name foo. In the next run (dnf5 remove foo) it removes all packages providing foo. The next run will remove packages providing binary foo. This behavior is inconsistent.

Resolves: https://issues.redhat.com/browse/RHEL-5747

CI-tests - rpm-software-management/ci-dnf-stack#1430

@j-mracek j-mracek force-pushed the libsolv-error branch 2 times, most recently from 6d49ac8 to 55b0c1a Compare January 4, 2024 10:37
@ppisar
Copy link
Contributor

ppisar commented Jan 4, 2024

Could you make a commit subject more specific? Like "Limit search pattern for remove command to NEVRAs and files"

@ppisar
Copy link
Contributor

ppisar commented Jan 4, 2024

This is a quite serious change which deserves a test. Do you plan enhancing ci-dnf-stack?
I noticed that testing-farm tests fail recently. It would be great to fix them before merging this patch.

It prevents additional removal after succesful remove using the same spec.
Spec `foo`. The original code during the first run removes (dnf5 remove foo)
all packages with the name `foo`. In the next run (dnf5 remove foo) it
removes all packages providing `foo`. The next run will remove packages
providing binary `foo`. This behavior is inconsistent.

Resolves: https://issues.redhat.com/browse/RHEL-5747
@j-mracek
Copy link
Contributor Author

j-mracek commented Jan 4, 2024

This is a quite serious change which deserves a test. Do you plan enhancing ci-dnf-stack? I noticed that testing-farm tests fail recently. It would be great to fix them before merging this patch.

The code require following change of CI - rpm-software-management/ci-dnf-stack#1430. It removes all tests for formal behavior and add one test for the new behavior.

j-mracek added a commit to j-mracek/ci-dnf-stack that referenced this pull request Jan 5, 2024
The new behavior is more stable for cases when the same spec is used multiple
times with remove command.

Requires: rpm-software-management/dnf5#1126
@j-mracek
Copy link
Contributor Author

j-mracek commented Jan 5, 2024

@ppisar Done

@ppisar ppisar self-assigned this Jan 5, 2024
It unify style with remove command, because it uses the same argument type.
@j-mracek
Copy link
Contributor Author

j-mracek commented Jan 8, 2024

@ppisar Thank you for couching it. Fixed.

@ppisar ppisar added this pull request to the merge queue Jan 8, 2024
Merged via the queue into rpm-software-management:main with commit 870a8a0 Jan 8, 2024
5 of 9 checks passed
j-mracek added a commit to j-mracek/ci-dnf-stack that referenced this pull request Jan 8, 2024
The new behavior is more stable for cases when the same spec is used multiple
times with remove command.

Requires: rpm-software-management/dnf5#1126
ppisar pushed a commit to rpm-software-management/ci-dnf-stack that referenced this pull request Jan 8, 2024
The new behavior is more stable for cases when the same spec is used multiple
times with remove command.

Requires: rpm-software-management/dnf5#1126
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants