-
-
Notifications
You must be signed in to change notification settings - Fork 6
[fix(builder): pre-import PKGBUILD validpgpkeys before build #362
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
Open
Aroy-Art
wants to merge
2
commits into
Lukas-Heiligenbrunner:master
Choose a base branch
from
Aroy-Art:fix/pgp-key-import
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. I think
source PKGBUILDis not a very good idea to run manually since it can be an arbitrary shell script.Doing
makepkg --printsrcinfoshould be a better option since its run in an controlled way.PR #360 moves from using paru to doing our own dependency resolution and added already something similar than you here:
https://github.com/Lukas-Heiligenbrunner/AURCache/pull/360/changes#diff-9824f463ed3db347394cc2d3267d13ed1349c9412eeb7de66503066486f4878bR5
We still need to review and pollish this pr until it gets to master.
Do you allign with the approach taken there?
Its not a very clean way to pass such a large shell script to the docker containers CMD tho.
Feel free to comment on PR #360 if you have improvement suggestions / input.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point — source PKGBUILD executing arbitrary shell is a valid concern. Agree that makepkg --printsrcinfo (or reading .SRCINFO if already present) is cleaner and safer.
The approach in #360 looks right: prefer .SRCINFO when it exists, fall back to makepkg --printsrcinfo, parse validpgpkeys lines with sed. Happy to update this PR to use that pattern instead of source PKGBUILD if it's useful as a quick fix on master while #360 is still being polished — otherwise happy to close this in favor of #360 since it handles the problem more comprehensively. Let me know which you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Aroy-Art I think it still makes sense to polish and merge this PR since #360 will probably take a while until RTM.
Maybe you can have a look at the relevant bits from #360 and port them over. When merged I can bump out a new release with this one and the new settings page too. :)