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

Check if line begins with 'warning' before adding it to package info #1674

Merged
merged 4 commits into from
Mar 26, 2023

Conversation

Lautron
Copy link
Contributor

@Lautron Lautron commented Mar 12, 2023

PR Description:

Skip lines from pacman -Q --info that begin with 'warning' before adding them to package info to avoid getting a TypeError.
The problem is explained with more detail on #1673

Tests and Checks

  • I have tested the code!
    • Tested on live ISO

@Lautron Lautron requested a review from Torxed as a code owner March 12, 2023 18:46
@Torxed
Copy link
Member

Torxed commented Mar 12, 2023

Nice catch. I think a long term solution would be to modify the fix to fix all future possible "odd outputs" from pacman.
Would you mind changing the fix to:

- return LocalPackage(**package_info)
+ import dataclasses
+ return LocalPackage({field.name: package_info.get(field.name) for field in dataclasses.fields(LocalPackage)})

I'm no expert, but I think this would solve it.
It would iterate the fields we've defined in LocalPackage and only populate those fields. Any additional fields (added inte future in the pacman project) would be ignored, including warning: or info: or whatever it outputs :)

I can manually change the patch but to keep the credit of finding and fixing the bug you're free to change if you want to :)

@Lautron
Copy link
Contributor Author

Lautron commented Mar 12, 2023

Nice catch. I think a long term solution would be to modify the fix to fix all future possible "odd outputs" from pacman. Would you mind changing the fix to:

- return LocalPackage(**package_info)
+ import dataclasses
+ return LocalPackage({field.name: package_info.get(field.name) for field in dataclasses.fields(LocalPackage)})

I'm no expert, but I think this would solve it. It would iterate the fields we've defined in LocalPackage and only populate those fields. Any additional fields (added inte future in the pacman project) would be ignored, including warning: or info: or whatever it outputs :)

I can manually change the patch but to keep the credit of finding and fixing the bug you're free to change if you want to :)

Hi there! Thanks for your feedback.

I think your suggestion to make the fix more future-proof is great. I will definitely keep that in mind for future contributions to open source.

I have just made the changes you suggested in a new commit. Thank you again for your help and for being such a supportive member of the open source community.

@Torxed Torxed merged commit 942112f into archlinux:master Mar 26, 2023
@Torxed
Copy link
Member

Torxed commented Mar 26, 2023

Thank you for this one! Merged! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants