-
Notifications
You must be signed in to change notification settings - Fork 30
feat: add ability to downgrade aur package using git #260
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
Conversation
|
Added some tests, however i don't understand yet how syntax works, so will edit them when I figure out. |
pbrisbin
left a comment
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.
This is looking good. Sorry, I should've warned you not to worry about the existing shfmt / shellcheck stuff.
|
I realized that see Here's simple command how to try build something from aur |
pbrisbin
left a comment
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.
Left some thoughts, but nothing blocking. Let me know if you'd like to address them before merging.
|
I'm going to push a commit that improves the "No fixture found" error message, and then does the thing it suggests to fix those. The remaining failures seem like they may just be actual bugs. |
|
Looking at the test failures, I see a few examples like: This test is really about Is it possible your regex changes are causing the operators filtering to no longer work? |
|
I fixed regex already, however I don't understand why search_git is not outputting anything. It should, since we clone repo and set correct path to CacheDir. Once I figured that out I push fixes. |
|
I'm idiot I forgot to read config values now it should be without error. |
|
hopefully i rebased correctly, first time doing this :D |
|
dont run workflows yet, I just discovered |
|
So this part should match multiple lines so when you update we don't have to rewrite test to add new version however it matches only first match. So i assume cram does not support multiline regex match. Any suggestion? |
|
Ok so come up with workaround to use |
That's correct. Cram is basically golden testing, so it should be written to match all the expected output. The glob and regex form can be useful, but should be used sparingly. I would probably use Does that work?
I'm personally OK if we just don't test that function, it seems hard to get any useful test coverage there. It's an add-on / special case thing that, if it breaks, |
|
did i mess up something? |
|
just test passing on my side don't understand why did this happen |
|
OK, I see what's happening. If you are happy with it now, I can take it from here and push some fixes before merging. |
On GHA, tests that run in containers must use `root`[^1]. Otherwise, you don't have access to write in the workspace. For example, our tests failing couldn't write the `.t.err` files. Therefore, to use the `archlinux` container, we have to remove this test that was asserting errors when running as non-`root`. In testing that this would work, I used `sudo just test` locally, which ran into an unrelated error being unable to write in `/tmp`. This is probably solvable, but we never used that file, so I replaced it with `/dev/null`. Lastly, I installed `git` since I know we'll need that. [^1]: https://docs.github.com/en/actions/reference/workflows-and-actions/dockerfile-support#user
This is fully opt-in by the `--git` option (or equivalent env var). If enabled, we check to see if the configured cache directory represents the git repository. This would be the case if the user is downgrading an AUR package installed via `yay`. In such a repository, we can hunt for commits that represent the older versions of the package and present them as options to downgrade to. If selected, we can check out that commit and build the package at that version (if necessary) and install it. The logic is isolated to a `search_git` function, for handling locating commits that represent versions and output potential package names that include "gitpkg" in them, and a `build_pkg` function that looks for such package names and builds them. We have to be careful to use `su` so that `makepkg` is not run as root. The only changes to `downgrade` outside of these new functions is to regexs used to locate and parse package names, but this is well covered by tests.
|
OK, I'm done and ready to merge. I will let this sit for a few hours in case you want to review my changes, commit messages, etc. Thanks for the contribution, I hope it works well for you! |
|
lgtm |
|
there is issue restyle fucked up something probably, quickfix: |
|
for some reason missing quoted didn't saw it in head diff when was merging |
|
how did tests pass tho? |
This was what Restyled did, it was only whitespace. diff --git a/src/downgrade b/src/downgrade
index 84e9bc1..24a3204 100755
--- a/src/downgrade
+++ b/src/downgrade
@@ -211,8 +211,8 @@ search_git() {
path="$cache/$name" # directory name
if ! [[ -d "$path/.git" ]]; then
{
- gettext "\`$path\" not a git directory, skipping..."
- echo
+ gettext "\`$path\" not a git directory, skipping..."
+ echo
} >&2
continue
fi
@@ -586,12 +586,11 @@ build_pkg() {
exit 1
fi
-
item="$(find "$path" -maxdepth 1 -type f -name "$(sed -nr '/.*Finished making: ([^ ]+) ([^ ]+) .*/{s//\1-\2/;p}' makepkg.log)*" -print -quit)"
rm makepkg.log
popd >/dev/null
fi
- printf "%s\0" "$item" >> new_to_install.downgrade
+ printf "%s\0" "$item" >>new_to_install.downgrade
}
cli() {
@@ -611,7 +610,7 @@ cli() {
for item in "${to_install[@]}"; do
build_pkg "$item"
done
- mapfile -d '' to_install < new_to_install.downgrade
+ mapfile -d '' to_install <new_to_install.downgrade
rm new_to_install.downgrade
fi
--
2.52.0
🤷 |
|
5f75463 this was commit that changed something in downgrade (and is correct) so maybe build? |
|
to be clear: |
You are right. The I can try to update the tests so it builds before testing to catch these. |
|
I'm going to revert and release since this breaks everything. Will bring it back after addressing the CI gap. |
|
FYI the feature was reverted in 11.7 and brought back in 11.8. |
Checklist
dist/completion/*doc/*.ronnlocale/*.poDescription
If you don't store all tarballs in your ~/.cache/yay (like me, store only last 2) or if you use other aur-manager you could instead use vcs to switch to desired branch and rebuild chosen version. Hopefully everything works as should, tested only for yay.