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

feat: implement already installed query for provider yay #140

Merged
merged 1 commit into from
Mar 4, 2022

Conversation

mriehl
Copy link
Contributor

@mriehl mriehl commented Mar 4, 2022

Resolves #136

-Qq with package list is chosen for efficiency (no need to query each
package individually)

There is a bit sad edge case where you can request installation of
something like pkg-config but the package is actually called
pkgconf where this will fail to detect already installed. But the
"fix" is simply to add the proper package name :)
I don't see a way of solving this without shelling out for each
package individually or saving all the package aliases.

@mriehl
Copy link
Contributor Author

mriehl commented Mar 4, 2022

Maybe you have an idea to test this @rawkode ... I can probably mock the Process API and assert on the Steps that are returned from calling install?

For now it's just tested locally on arch.

mriehl added a commit to mriehl/dotfiles that referenced this pull request Mar 4, 2022
with comtrya/comtrya#140 everyting just works without
any shenanigans
mriehl added a commit to mriehl/dotfiles that referenced this pull request Mar 4, 2022
with comtrya/comtrya#140 everything just works without
any shenanigans
@rawkode
Copy link
Member

rawkode commented Mar 4, 2022

@mriehl It's really painful to test an Action at the moment because they require a Manifest and Contexts - I actually just did this myself on PR #139 and I hated everything about it.

I'll look at providing some helpers for this, but for now it's OK.

I see that you've tested this with your own dotfiles, soo you're confident and happy it works?

@rawkode
Copy link
Member

rawkode commented Mar 4, 2022

@mriehl Looks like you need to run cargo fmt --all -- --check and push the formatting fixes 👍🏻

Resolves comtrya#136

-Qq with package list is chosen for efficiency (no need to query each
package individually)

There is a bit sad edge case where you can request installation of
something like `pkg-config` but the package is actually called
`pkgconf` where this will fail to detect already installed. But the
"fix" is simply to add the proper package name :)
I don't see a way of solving this without shelling out for each
package individually or saving all the package aliases.
@mriehl
Copy link
Contributor Author

mriehl commented Mar 4, 2022

@mriehl Looks like you need to run cargo fmt --all -- --check and push the formatting fixes 👍🏻

Oof. I actually ran cargo fmt but it changed every file, so I thought there was no formatting. Turns out I still had a rustfmt.toml file lying around in my home which explains why it changed everything. I rebased the changes and it should be good now!

I see that you've tested this with your own dotfiles, soo you're confident and happy it works?

Yup, works perfectly for me, both for AUR and non-AUR packages. I did find out about the issue where it was still installing pkconf but this turned out to be due to the incorrect package name I was using.

@rawkode
Copy link
Member

rawkode commented Mar 4, 2022

Nevermind, I think it's OK

@rawkode rawkode merged commit 9e83a7f into comtrya:main Mar 4, 2022
@rawkode
Copy link
Member

rawkode commented Mar 4, 2022

Thanks @mriehl !

@mriehl mriehl deleted the yay-installed-filter branch March 4, 2022 17:14
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.

yay/pacman provider - should be able to filter packages with query
2 participants