Further failsafe check for available APT packages #3537
Conversation
|
After quite some time of research and tests I found this to be the fastest 100% reliable solution, meaning as reliable as a dry-run install as it lets pass the same two cases. The chances that Also probably it is nicer to move this in a separate function like |
gives quite some output. How about storing this in an intermediate variable, like: and then do the individual greps (no need for a complicated regex here) on this reduced amount of data? And yes, I agree on factoring this out into two functions:
What do you think? |
|
Good idea to create the list once, to reduce overhead. It can be combined with the check function: The variable/function name is a bid long, but probably worth it to allow something similar for yum and dnf in the future? It could be merged into a single function for all package managers, but I personally don't see a benefit of it as it only requires more checks (about which package manager is used). |
|
@DL6ER |
automated install/basic-install.sh
Outdated
| @@ -179,6 +179,18 @@ is_command() { | |||
| command -v "${check_command}" >/dev/null 2>&1 | |||
| } | |||
|
|
|||
| apt_package_list= | |||
There was a problem hiding this comment.
If this is meant to be a global variable then it should be declare up with the rest of the variables, starting on line 28. And the name should be in all caps if it's intended to be global.
If not then it should be declared local inside the function.
There was a problem hiding this comment.
It's kind of an edge case. It is only used in the below function, but it needs to be global to survive for subsequent calls of this function.
I find it easier to read the code when in this case it is declared aside of the function, but I can move it to the other global functions and make it upper case, if you prefer.
There was a problem hiding this comment.
Yeah, I'd prefer all the variables to be in one place if they are not inside a function. Keeps from having to hunt down where they are. We already have the vars declared in one central place so this would become an outlier and I want to avoid things like that.
The function works from a quick test but we'd like to be more verbose with things. Use ifs and no || or &&s. It helps with our userbase to understand.
is_apt_package(){
# Checks whether a package, or one that provides it, is available in
# the installed APT repository lists.
local check_package=$1
# Obtain the list of available packages once
if [ -z "$apt_package_list" ]; then
apt_package_list=$(apt-cache dumpavail | grep -E '^P(ackage|rovides):')
fi
grep -qE " $check_package(,|$)" <<< "$apt_package_list"
}
- "apt-cache show package" succeeds as well if package is listed as (optional) dependency or conflict by another package, hence is not a 100% reliable measure. - There is no command which explicitly checks which package/name can be selected by apt-get for install. An install simulation/dry-run is possible as it was before Pi-hole v5.1, or the whole package cache can be scraped, which is still the less time consuming solution. - Allow to succeed if another package "provides" it, like "php7.3-apcu" provided by "php-apcu" or "awk" provided by "mawk" and "gawk", in which case the non-virtual package is selected automatically by apt-get. For reference: MichaIng/DietPi@066b89f Signed-off-by: MichaIng <micha@dietpi.com>
dschaper
added
Enhancement
PR: Approval Required
|
It is not urgent anyway, just makes that package detection a bit more robust, so take your time. Good luck with your moving, we just have that finished this spring 😛. |
DL6ER
added
PR: Approved
|
This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there: https://discourse.pi-hole.net/t/pi-hole-core-v5-2-3-web-v5-3-and-ftl-v5-4-released/43009/1 |
|
This may be causing problems for a user: https://www.reddit.com/r/pihole/comments/kyme7b/help_pihole_up_failed_iproute2_and_iproute/ |
When attempting to update to FTL 5.4 I receive the following error. |
|
One thing I will note, is that this change has realllly upped the verbosity of a |
|
I guess the whole APT_PACKAGE_LIST variable is dumped once when using @littleneutrino This is the output in my case, where the second-last does match the check, and this should be the case in all supported Debian and Ubuntu versions: Actually, considering the supported Debian and Ubuntu versions, |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
For the docker container, please see this issue thread: @littleneutrino, what is the contents of |
|
And try Edit: And it's converse |
|
pi@raspberrypi:~ $ apt-cache dumpavail | grep -E '^P(ackage|rovides):.* iproute2(,|$)' |
|
Strange, https://manpages.debian.org/buster/apt/apt-cache.8.en.html#OPTIONS
|
|
pi@raspberrypi:~ $ apt-cache dumpavail | grep -E '^P(ackage|rovides):.* iproute2(,|$)' |
|
Just for fun: |
|
pi@raspberrypi:~ $ |
|
Do you have
|
|
Try seeing if you can actually view that page: |
|
|
|
Something is intercepting your DNS lookups. |
|
|
You need to be really careful with your typing. |
|
Also, run |
|
Something is keeping you from seeing the remotes. A good |
|
pi@raspberrypi:~ $ ls -la /var/cache/apt/ |
|
Your Let's try removing it and doing a manual update to compare: Then try Lastly, post the results of |
I imagine I will be rebuilding this from scratch at some point but if you have more things you want ran feel free to ask. |
|
You didn't do what I asked you to do. (Or you didn't show the I don't think we're going to find what we need here. |
|
I did. I pasted in the commands MobaXterm just didnt keep the first line for whatever reason. pi@raspberrypi:/var/cache/apt/archives $ |
|
I still don't know why your |
|
The TL;DR on this is somehow /etc/apt/sources.list got blanked out hence apt update was only doing other repo's If you look at @eLKosmonaut apt update there seems to only be debian buster InRelease listed, but for me iproute2 was in the backports repo (see debugging notes) and I was missing it! The main bit that was telling to me was 'apt-cache showpkg' listing the "default" dpkg of /var/lib/dpkg (since apt has no idea about them), where as apt controlled files correctly show /var/lib/apt/lists/.... I guess the changes could be to check sources make sense, or at least perhaps use dpkg to check if the command exists ? I'm not sure all embedded systems will use apt (but again, I get this is straying from the default Pi installation base) My debugging notes: Working machine: This then leads to /var/lib/apt/lists not having the right .xz cache list for apt-chache dumpavail to search. "Broken" machine is missing main_binary (of any sorts, modern or backport!) dpkg however still works fine (on both machines) |
|
I should add that update worked fine after fixing my sources.list |
|
I think, to be honest, I'd like to go ahead with a revert on this PR (#3997) until we can nail down exactly what is going on, and how to mitigate it with (near) 100% success. Overall I like the idea of the |
|
I want to point out as well that reverting is only a band-aid fix and we should get this back in once we understand it better. At the same time, I agree we should maybe instead investigate further if we can remove some or maybe even all of the checks altogether (as suggested by @dschaper and @MichaIng) |
|
@eLKosmonaut
Wait, the RPi firmware list is the only one you use? Where is the Raspbian list http://raspbian.raspberrypi.org/raspbian/ ? The RPi list contains mostly kernel, firmware and special GPU-accelerated GUI software builds but none of the Debian base packages, it is no full repository 😉. EDIT: Ah would fit the case of Kyrth. Okay so then it seems the ckeck is doing and succeeding (compared to apt-get show) in what it aims: checking if a package is available in the installed APT lists/sources or not. I mean technically if the package is installed already, it wouldn't matter, but any other base package install would fail then. But indeed, from all I can see, the check is obsolete for currently supported distros. But the code should be kept at least commented, in case required in the future, with a link to this discussion 😉. |
|
@eLKosmonaut / @littleneutrino, you should be able to switch back to |
Just ran the update after going back to Core Master and its updated no errors. thanks! |
That was the case so that lead to the confusion.
Ran the checkout back to core master and the installer stepped through smoothly. Thanks again for the help! |
By submitting this pull request, I confirm the following:
git rebase)Signed-off-by: MichaIng micha@dietpi.com
What does this PR aim to accomplish?:
How does this PR accomplish the above?:
apt-cache dumpavailfor effectively available packages.For reference: MichaIng/DietPi@066b89f
What documentation changes (if any) are needed to support this PR?: