Merge ~bryce/ubuntu/+source/apache2:apache2-fix-lp1832182-hirsute into ubuntu/+source/apache2:ubuntu/devel
Status: | Merged |
---|---|
Approved by: | Bryce Harrington on 2020-11-19 |
Approved revision: | 2fea15554926ad8a677ad04b3720d73a7cca08d7 |
Merge reported by: | Christian Ehrhardt |
Merged at revision: | 2fea15554926ad8a677ad04b3720d73a7cca08d7 |
Proposed branch: | ~bryce/ubuntu/+source/apache2:apache2-fix-lp1832182-hirsute |
Merge into: | ubuntu/+source/apache2:ubuntu/devel |
Diff against target: |
98 lines (+40/-18) 2 files modified
debian/apache2ctl (+30/-18) debian/changelog (+10/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Bryce Harrington | Approve on 2020-11-19 | ||
Lucas Kanashiro | 2020-11-06 | Needs Fixing on 2020-11-12 | |
Canonical Server packageset reviewers | 2020-11-06 | Pending | |
Canonical Server Team | 2020-11-06 | Pending | |
Review via email:
|
Description of the change
This is a re-send of a server-next fix for Apache2, which supersedes an earlier MP targeting groovy: https:/
This fixes some errors in the earlier code and incorporates Christian's review comments. I've tested the code via the test case in the bug with some variations, and it seems good. Autopkgtest checks out too, although the test cases don't exercise the 'graceful' command.
The issue is applicable to all past supported releases, so once this is accepted for hirsute I plan to also apply it to groovy, focal, bionic, and xenial. Thus please also review the SRU text in the bug report itself.
Lucas Kanashiro (lucaskanashiro) wrote : | # |
Lucas Kanashiro (lucaskanashiro) wrote : | # |
The package changes and the SRU bug description LGMT in general, just some comments below:
- In the changelog your target is groovy but this MP is targeting hirsute.
- This is me being nitpick: I would change this commit message:
commit a4a20160eefd3cd
Author: Bryce Harrington <email address hidden>
Date: Mon Oct 5 16:03:00 2020 -0700
* d/apache2ctl: Also use systemd for graceful if it is in use. This extends an earlier fix for the start command to behave similarly for restart / graceful. Fixes service failures on unattended upgrade. (LP: #1832182)
to something like:
* d/apache2ctl: Also use systemd for graceful if it is in use (LP: #1832182).
This extends an earlier fix for the start command to behave similarly for
restart / graceful. Fixes service failures on unattended upgrade.
It is a better reading IMO.
- I see this same bug was reported to Debian here:
https:/
Could we forward these changes to Debian as well?
Apart from that the package built fine and I was able to confirm that the fix is working as expected (following the instructions in the SRU bug). I also confirmed that autopkgtest did not identify any regression (despite it does not exercise the path of proposed changes):
autopkgtest [10:51:10]: @@@@@@@
run-test-suite PASS
duplicate-
htcacheclean PASS
default-mods PASS
ssl-passphrase PASS
check-http2 PASS
chroot PASS
Bryce Harrington (bryce) wrote : | # |
Thanks for the review. I've incorporated the suggested changes.
I've forwarded the debdiff to debian: https:/
Branch and PPA are updated.
Bryce Harrington (bryce) wrote : | # |
Since I incorporated all the review feedback, and since Christian also reviewed and approved the same changes for the SRUs I've gone ahead and uploaded this one too.
Christian Ehrhardt (paelzer) wrote : | # |
This is fix released in hirsute -> merged
I am grabbing this for review.