Univention Bugzilla – Full Text Bug Listing |
Summary: | missing error message in univention-upgrade | ||
---|---|---|---|
Product: | UCS | Reporter: | Charly Frahm <frahm> |
Component: | Update - univention-updater | Assignee: | Julia Bremer <bremer> |
Status: | CLOSED FIXED | QA Contact: | Arvid Requate <requate> |
Severity: | normal | ||
Priority: | P5 | CC: | brodersen, damrose, galkin, gohmann, meybohm, requate, troeder, zvn |
Version: | UCS 4.2 | ||
Target Milestone: | UCS 4.3-2-errata | ||
Hardware: | Other | ||
OS: | Linux | ||
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: | |||
Attachments: |
univention-upgrade shows no updates available when they are
qa-feedback.diff |
Description
Charly Frahm
2014-04-02 14:53:25 CEST
Still applicable to UCS4.0-1. If there are package conflicts/unmet dependencies, the univention-upgrade simply shows that no updates are available... Created attachment 6753 [details]
univention-upgrade shows no updates available when they are
*** Bug 22994 has been marked as a duplicate of this bug. *** *** Bug 31808 has been marked as a duplicate of this bug. *** 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. *** Bug 42929 has been marked as a duplicate of this bug. *** *** Bug 44060 has been marked as a duplicate of this bug. *** Some keywords for developers: Checking for package updates says none E: Unmet dependencies. apt --fix-broken install component_update_get_packages What happens: modules/univention/updater/tools.py: 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. Package: univention-updater Version: 13.0.1-60A~4.3.0.201810111136 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 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. Successful build Package: univention-updater Version: 13.0.1-61A~4.3.0.201810120915 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. 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. Please remember to fix the package name in the "src:" field of the yaml too. done Created attachment 9709 [details] qa-feedback.diff Ok: * 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 #!/bin/bash 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 %EOF 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 'http://192.168.0.10/build2 ucs_4.3-0-errata4.3-2/all/ Release' does not have a Release file. W: The repository 'http://192.168.0.10/build2 ucs_4.3-0-errata4.3-2/amd64/ Release' does not have a Release file. W: The repository 'http://192.168.0.10/build2 ucs_4.3-0-errata4.3-2/source/ Release' does not have a Release file. none 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. Successful build Package: univention-updater Version: 13.0.1-62A~4.3.0.201810291008 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 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). |