Remove debconf apt progress #2962
Conversation
automated install/basic-install.sh
Outdated
| @@ -245,7 +245,7 @@ if is_command apt-get ; then | |||
| fi | |||
| # Since our install script is so large, we need several other programs to successfully get a machine provisioned | |||
| # These programs are stored in an array so they can be looped through later | |||
| INSTALLER_DEPS=(apt-utils dialog debconf dhcpcd5 git "${iproute_pkg}" whiptail) | |||
| INSTALLER_DEPS=(apt-utils debconf dhcpcd5 git "${iproute_pkg}" whiptail) | |||
There was a problem hiding this comment.
I was thinking the same but I did not want to remove the packages in the event they are required for a particular minimal Debian based distro.
The two packages are available by default in my Debian 10 test environment.
The following commands are provided by the packages:
bcambl@debian10:~$ dpkg-query -L debconf | grep bin
/usr/bin
/usr/bin/debconf
/usr/bin/debconf-apt-progress
/usr/bin/debconf-communicate
/usr/bin/debconf-copydb
/usr/bin/debconf-escape
/usr/bin/debconf-set-selections
/usr/bin/debconf-show
/usr/sbin
/usr/sbin/dpkg-preconfigure
/usr/sbin/dpkg-reconfigure
bcambl@debian10:~$ dpkg-query -L apt-utils | grep bin
/usr/bin
/usr/bin/apt-extracttemplates
/usr/bin/apt-ftparchive
/usr/bin/apt-sortpkgs
bcambl@debian10:~$
I would like to hear from the other team members before removing them as well.
Thanks for the review :)
|
This creates a lot of extra output, is that intentional? (ignore missing ticks, having terminal issues)
|
|
@PromoFaux Indeed it breaks the previous output style of the installer. On DietPi we separate it visually by setting and reverting the terminal colour to What would be IMO good, is some additional line before the actual apt-get call: The "please wait..." is especially helpful on slow devices (RPi1/Zero) with disabled APT cache, where the |
|
To my eye, it would be at least less jarring if it was possible to force the additional output to be offset from the screen like so: |
|
@PromoFaux |
|
@dschaper |
|
Just spit-balling... could we not pipe that output into a variable then offset it with a |
|
It would be a huge amount of text to buffer in memory... |
|
I guess when redirecting STDOUT and printing back, interactive input fields are overridden 🤔. If user then hits return, default choice is used instead of question + input field being printed again. |
|
Fair comments both. If the best we can do is a horizontal line, maybe with a different text colour then it may just look a bit better. Personal preference I guess.. I am just not a fan of a messy output, regardless of how many people will actually end up paying attention to it! |
=> due to possible delay until first output, as mentioned above? |
|
Ah yeah, for sure. We used to have a fancy spinner thing for those situtations, but it was buggy as all hell. I'd hate to dig that out again. |
|
Here is ours: https://github.com/MichaIng/DietPi/blob/dev/dietpi/func/dietpi-globals#L329-L379 Will do some tests with |
|
Would love to see this implemented soon. Did some more tests and sadly couldn't find a way to further reduce APT output while keeping the ability for interactive choices if e.g. config files are about to be updated: It seems that @bcambl @dschaper Removing debconf and apt-utils from dependencies can be done as a separate PR but since it somehow belongs together, see: #2962 (comment) |
|
@dschaper @PromoFaux have either of you put any additional thought into this? |
|
I think, as long as we can separate it somehow from the "pretty" formatted install output, then we should be good. I don't really have any suggestions beyond: |
|
Function beats form for me. I know running For now it may be ugly but it works and it solves a bug that is often hard to diagnose. We can pretty it up later, fix it in post. |
|
Definitely function > form. However to have outputs separated clearly, the following could be done here: https://github.com/pi-hole/pi-hole/blob/7052a7695e5b25e20d488bffd2fc94595e90a2e1/automated%20install/basic-install.sh#L1637 |
| @@ -1652,7 +1652,7 @@ install_dependent_packages() { | |||
| fi | |||
| done | |||
| if [[ "${#installArray[@]}" -gt 0 ]]; then | |||
| "${PKG_INSTALL[@]}" "${installArray[@]}" &> /dev/null | |||
There was a problem hiding this comment.
@bcambl
Not sure if this is wanted, since this is a yum install, not an apt-get install! Not sure if yum can hang for the same reason, when redirecting STDOUT, but in all other yum calls &> /dev/null is still in place, hence this makes it inconsistent. I suggest to revert it in this PR. What we've done for apt-get could be done for yum as well (if required for same reason), but should be a dedicated PR then, where I would change PKG_INSTALL to be a function to handle apt-get + yum/dnf all together in a consistent way.
There was a problem hiding this comment.
If I was to keep this /dev/null in place, there would be no output between the horizontal rule on yum/dnf distributions. As for the redirections in other locations...I think they are fine for now.
If we decide to remove them, those changes would belong in a different PR.
|
To push it a bid
|
Yep, If that's the best we can do to separate the output, I'm good with it. Other developer's opinions may vary |
|
Thanks all, I will update this PR soon. |
+ The installer uses `whiptail`, thus `dialog` is not required. Signed-off-by: MichaIng <micha@dietpi.com>
Signed-off-by: bcambl <blayne@blaynecampbell.com>
…es() Removes the need for conditional debconf-apt-progress dependency checking Signed-off-by: bcambl <blayne@blaynecampbell.com>
Signed-off-by: bcambl <blayne@blaynecampbell.com>
Signed-off-by: bcambl <blayne@blaynecampbell.com>
bcambl
force-pushed
the
remove_debconf-apt-progress
branch
from
7052a76
to
60c5188
Compare
|
I have rebased on |
There was a problem hiding this comment.
Confirmed works as described, and the horizontal rule goes some distance to separating the apt output from the installer output. Still a lot more verbose and jarring than I would like, but that's just my opinion, and I can see the need for doing it this way :)
|
Agreed with Promo about the verbosity but perfect is the enemy of good at this point. |
|
This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there: https://discourse.pi-hole.net/t/pihole-funktioniert-nicht-mehr-nach-update/32375/32 |
By submitting this pull request, I confirm the following:
What does this PR aim to accomplish?:
debconf-apt-progresscommand was unavailable.debconf-apt-progressOriginal issue report #1278 and fix was #1280
Additional discussion on #2892
Replaces my previous PR #2934
How does this PR accomplish the above?:
debconf-apt-progressdialogdependency for Debian (commit by @MichaIng)dialogdependency for FedoraWhat documentation changes (if any) are needed to support this PR?:
n/a