Bug 34444 - missing error message in univention-upgrade
missing error message in univention-upgrade
Product: UCS
Classification: Unclassified
Component: Update - univention-updater
UCS 4.2
Other Linux
: P5 normal (vote)
: UCS 4.3-2-errata
Assigned To: Julia Bremer
Arvid Requate
: 22994 31808 42929 44060 (view as bug list)
Depends on:
  Show dependency treegraph
Reported: 2014-04-02 14:53 CEST by Charly Frahm
Modified: 2018-11-14 14:40 CET (History)
8 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 3: Simply Wrong: The implementation doesn't match the docu
Who will be affected by this bug?: 3: Will affect average number of installed domains
How will those affected feel about the bug?: 2: A Pain – users won’t like this once they notice it
User Pain: 0.103
Enterprise Customer affected?:
School Customer affected?:
ISV affected?: Yes
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2011052510001743
Bug group (optional):
Max CVSS v3 score:

univention-upgrade shows no updates available when they are (1.48 KB, text/plain)
2015-03-10 12:56 CET, Dmitry Galkin
qa-feedback.diff (1.75 KB, patch)
2018-10-18 21:50 CEST, Arvid Requate
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Charly Frahm univentionstaff 2014-04-02 14:53:25 CEST
While running univention-upgrade it occurs that univention-upgrade says that there are no new packages even if there are packages that to actualise. The problem seems to be that there are some broken packages on the system (found via apt-get update).
The univention-upgrade modul should print a useful error message like the one that is given by apt-get update (which roughly says "run dpkg --configure -a to fix this")
Comment 1 Dmitry Galkin univentionstaff 2015-03-10 12:56:14 CET
Still applicable to UCS4.0-1.

If there are package conflicts/unmet dependencies, the univention-upgrade simply shows that no updates are available...
Comment 2 Dmitry Galkin univentionstaff 2015-03-10 12:56:31 CET
Created attachment 6753 [details]
univention-upgrade shows no updates available when they are
Comment 3 Florian Best univentionstaff 2016-01-15 10:58:21 CET
*** Bug 22994 has been marked as a duplicate of this bug. ***
Comment 4 Florian Best univentionstaff 2016-01-15 10:59:24 CET
*** Bug 31808 has been marked as a duplicate of this bug. ***
Comment 5 Stefan Gohmann univentionstaff 2017-06-16 20:37:37 CEST
This issue has been filed against UCS 3. UCS 3 is out of the normal maintenance and many UCS components have vastly changed in UCS 4.

If this issue is still valid, please change the version to a newer UCS version otherwise this issue will be automatically closed in the next weeks.
Comment 6 Jürn Brodersen univentionstaff 2018-07-03 11:05:13 CEST
*** Bug 42929 has been marked as a duplicate of this bug. ***
Comment 7 Jürn Brodersen univentionstaff 2018-07-03 11:05:50 CEST
*** Bug 44060 has been marked as a duplicate of this bug. ***
Comment 8 Jürn Brodersen univentionstaff 2018-07-03 11:12:34 CEST
Some keywords for developers:
Checking for package updates says none
E: Unmet dependencies.
apt --fix-broken install

What happens:
component_update_get_packages() does not check the return code from "cmd_dist_upgrade_sim" neither is stderr checked -> there are no packages listed on stdout -> no error is shown, instead the updater just says there are no updates.

Possible fix:
check the return code and show the error.
Comment 9 Julia Bremer univentionstaff 2018-10-11 11:42:05 CEST
Package: univention-updater
Version: 13.0.1-60A~
Branch: ucs_4.3-0
Scope: jbremer
User: jbremer

67f0855b7b Bug #34444: univention-upgrade now shows error message if there are unmet dependencies
2be48e44be Bug #34444: univention-update now shows error message if unmet dependencies error appears
f6cf848365 Bug #34444: In case Error-Code 100 appears, univention-upgrade now prints the stderr - Changed spaces to tabs

The diff is messy because the original file was indented with spaces. I changed this to tabs.
The only lines I changed beside that are 1308,1309
Comment 10 Arvid Requate univentionstaff 2018-10-11 18:03:33 CEST
Ok, to prove that a code cleanup doesn't make any functional changes it is required by https://hutten.knut.univention.de/mediawiki/index.php/Priorisierung_in_der_Entwicklung#Code_Cleanups that cleanup commits have to be separate from commits that do functional changes.

> f6cf848365 Bug #34444: In case Error-Code 100 appears, univention-upgrade now prints the stderr - Changed spaces to tabs

Something went wrong in that commit because it accidentally re-introduces functions to tools.py that have been removed for Bug #36719 by commit f6cdfb9f6c

I guess you took a tools.py from a UCS 4.3-1 system prior to errata 210 and commited that to the 4.3-2 branch along with your changes.

I would suggest to continue with the following three steps:

1. apply the reverse of commit f6cf848365
2. introduce your minimal patch, that is required to solve this Bug
3. eventually commit a code cleanup

Regarding the building of packages: if you commit or merge into the 4.3-2 git branch, then the package should be built in the errata4.3-2 scope instead of your private scope and you need to create/update an advisory file for your package (in ~/git/ucs/doc/errata/staging/univention-updater.yaml in this case), so others know that there are changes that are still in QA. Otherwise it may happen that a co-worker also works on the package, builds in in errata4.3-2 and is unaware that your changes are in there too. That way they may even get released without QA. Sorry, I should have explained this better I guess.
Comment 11 Julia Bremer univentionstaff 2018-10-12 09:36:17 CEST
Successful build
Package: univention-updater
Version: 13.0.1-61A~
Branch: ucs_4.3-0
Scope: errata4.3-2
User: jbremer

d406fac4f2 Bug #34444: Fixed missing error message in univention updater.
f7c4271130 Bug #34444: Advisory
b02c4e9d17 Bug #34444: Fixed error message in univention-updater. Changelog update
8169296e61 Bug #34444: Univention upgrader now shows error message
806b54a676 Revert "Bug #34444: In case Error-Code 100 appears, univention-upgrade now prints the stderr - Changed spaces to tabs"

Sorry for that mistake, I reverted the commit and made the right changes now.
Comment 12 Arvid Requate univentionstaff 2018-10-12 16:59:47 CEST
Ok, the code is fixed, but the advisory name doesn't match the source package:
Please rename doc/errata/staging/univention-upgrader.yaml and then mark this bug as fixed.
Comment 13 Arvid Requate univentionstaff 2018-10-12 17:01:52 CEST
Please remember to fix the package name in the "src:" field of the yaml too.
Comment 14 Julia Bremer univentionstaff 2018-10-17 16:38:31 CEST
Comment 15 Arvid Requate univentionstaff 2018-10-18 21:50:40 CEST
Created attachment 9709 [details]


* manpage says "apt-get returns zero on normal operation, decimal 100 on error."
* Code checks for return code 100

Suggestion for improvement:

The adjusted class method now uses "print" to output the error message,
which has some drawbacks: The class method may be used in contexts where
standard output will not end up visible to the user. For example it is
called by the method component_update_available just a couple of lines before,
and that method is called by a UMC module, which may expect a different way
of message passing. I experimented a bit on a test VM and replaced apt-get by a wrapper script to simulate the error code:

root@master10:~# mv /usr/bin/apt-get /usr/bin/apt-get.real

root@master10:~# cat > /usr/bin/apt-get <<-%EOF
echo "TEST: Run apt-get $*" >&2 ## Some test message
/usr/bin/apt-get.real "$@"      ## run the real apt-get binary
exit 100                        ## Fake the error condition

root@master10:~# chmod +x /usr/bin/apt-get

With the "print" out in tools.py, the messages are printed to the screen but the univention-upgrade script doesn't report an error properly and continues as if nothing is wrong:
root@master10:~# univention-upgrade 

Starting univention-upgrade. Current UCS version is 4.3-2 errata285

Checking for local repository:                          none
Checking for package updates:                           W: The repository ' ucs_4.3-0-errata4.3-2/all/ Release' does not have a Release file.
W: The repository ' ucs_4.3-0-errata4.3-2/amd64/ Release' does not have a Release file.
W: The repository ' ucs_4.3-0-errata4.3-2/source/ Release' does not have a Release file.

Checking for app updates:                               none
Checking for release updates:                           none

The I've experimented a bit with the output of stderr (e.g. to sys.stderr) and had a look at the code of /usr/sbin/univention-upgrade and the error handling done in other corners of tools.py. Since the script (univention-upgrade) does some output handling (catching exceptions in the do_update function, redirecting error message to a log file, reporting the error and the log file location), we should use that by just throwing an exception. There are a couple defined in errors.py and I was simply lazy and used the base class "UpdaterException" instead of explicitly defining a new one. Throwing an exception at that point also has the benefit that it stops the script from continuing even though a severe problem has been detected. The attached patch shows the idea.
Comment 16 Julia Bremer univentionstaff 2018-10-29 10:25:02 CET
Successful build
Package: univention-updater
Version: 13.0.1-62A~
Branch: ucs_4.3-0
Scope: errata4.3-2
User: jbremer

f04b19640e Bug #34444: Advisory
a4ef1b5194 Bug #34444: Added Exception for missing package dependencies in univention-updater
a45475cf4f Bug #34444: Version bump
358d85fa7f Bug #34444: Added an updater exception

Added an Exception that gets raised, if return-code = 100
Comment 17 Arvid Requate univentionstaff 2018-11-12 21:05:40 CET
Ok, that looks good now. Commit f04b19640e wrote the package version into the "scope:" filed of the advisory, Philipp fixed that and I've adjusted the wording a bit, so it makes more sense for non-developers (commit d6dd035470).
Comment 18 Arvid Requate univentionstaff 2018-11-14 14:40:24 CET