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

Fix apt omitting final result #1022

Merged
merged 3 commits into from
May 2, 2023
Merged

Fix apt omitting final result #1022

merged 3 commits into from
May 2, 2023

Conversation

izzergh
Copy link

@izzergh izzergh commented May 2, 2023

My old issue (#881) was still giving me trouble with the latest version of meta-package-manager. This time I cracked it open and fiddled with the regex until I found the problem: it's expecting a newline in the description. This works great for a verbose search where most descriptions are multiple lines, but in practice (i.e. when using install or search --exact), the descriptions are one line, without a trailing newline. It's kinda hard to catch because an inexact search will show a few results, but it's always one fewer than apt search gets you.

I just changed the literal \n to \n?, so it should be backwards compatible as well.

apt-mint doesn't look for newlines and I don't use mint, so I didn't touch that.

I also added a unit test per the development guidelines. It's my first unit test in Python, let me know if I over- or under-did it. Thanks!

@izzergh izzergh changed the title Fix/apt omitting final result Fix apt omitting final result May 2, 2023
is added 3 commits May 2, 2023 03:10
Before this change, the last element in a search result would always be
omitted. The output of the `apt` command does not have a trailing
newline (at least on my machine with apt 2.0.9), so that literal newline
does not match, so the entire package is skipped.

The crux here is when doing an exact search (either with the mpm
--exact flag or adding ^ and $ to the query), the only entry is also the
last entry, and so there are no matches.

Since installing forces the "exact" search behavior (which we want), the
install command will always fail with no matches, even if there is
exactly one match.

Updates the regex to require 0 or 1 newlines so all the lines still get
captured. Also tweaked the comment to reflect that.
Since I did this in reverse TDD, I reverted the change and confirmed
the test fails for the expected reason.
@kdeldycke
Copy link
Owner

Thanks @izzergh for the detailed troubleshooting. That's a nice catch and a cool PR. And thanks for taking the time to implement a unittest. I'm looking at the details now. I'll try to think of eventual edge-cases but after a first pass it might be good enough to merge upstream. I'll keep you in the loop.

@kdeldycke kdeldycke added 🖥 platform: Linux Linux, Windows Subsystem for Linux v2 🐛 bug Something isn't working, or a fix is proposed 📦 manager: dpkg-based apt, apt-mint, opkg labels May 2, 2023
@kdeldycke
Copy link
Owner

OK so this looks good to me.

I had doubts about the way re.MULTILINE would influence the regexp but I think your changes works as all lines will match the double-space indention.

You made me realize that we should probably dedicate a couple of unittests to the regexps alone, but that is another job altogether.

@kdeldycke kdeldycke merged commit 1024a42 into kdeldycke:main May 2, 2023
35 of 60 checks passed
kdeldycke added a commit that referenced this pull request May 2, 2023
@kdeldycke
Copy link
Owner

For the record, I'm going to tag an release mpm v5.13.1 today. So you should find to fix available in that release.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2023

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐛 bug Something isn't working, or a fix is proposed 📦 manager: dpkg-based apt, apt-mint, opkg 🖥 platform: Linux Linux, Windows Subsystem for Linux v2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants