Merge ~ahasenack/ubuntu/+source/base-files:bionic-motd-news-config-split into ubuntu/+source/base-files:ubuntu/bionic-devel

Proposed by Andreas Hasenack on 2020-08-07
Status: Merged
Approved by: Andreas Hasenack on 2020-08-17
Approved revision: 08744d99f2c353f8db78e777d5f79044c1a1a8c2
Merged at revision: 08744d99f2c353f8db78e777d5f79044c1a1a8c2
Proposed branch: ~ahasenack/ubuntu/+source/base-files:bionic-motd-news-config-split
Merge into: ubuntu/+source/base-files:ubuntu/bionic-devel
Diff against target: 254 lines (+137/-11)
8 files modified
debian/base-files.maintscript (+2/-0)
debian/changelog (+33/-0)
debian/control (+13/-1)
debian/motd-news-config.install (+1/-0)
debian/motd-news-config.postinst (+55/-0)
debian/postinst.in (+13/-0)
debian/rules (+0/-1)
update-motd.d/50-motd-news (+20/-9)
Reviewer Review Type Date Requested Status
Christian Ehrhardt  2020-08-07 Approve on 2020-08-17
Canonical Server Team 2020-08-07 Pending
Review via email: mp+388922@code.launchpad.net

Description of the change

Same as https://code.launchpad.net/~ahasenack/ubuntu/+source/base-files/+git/base-files/+merge/388835, but for bionic, with the following extra details:

- backport the switch to wget, and follow-up fixes that landed in groovy
- versions for breaks/replaces are the bionic ones:

ubuntu-server 1.417.5:
  Depends: motd-news-config

base-files 10.1ubuntu2.10:
  Breaks: ubuntu-server (<< 1.417.5)
  rm_conffile /etc/default/motd-news 10.1ubuntu2.10~ base-files

motd-news-config 10.1ubuntu2.10:
  Breaks/Replaces: base-files (<< 10.1ubuntu2.10)

PPA: https://launchpad.net/~ahasenack/+archive/ubuntu/zomg-5/

The linked bugs have the SRU template filled out.

To post a comment you must log in.
Andreas Hasenack (ahasenack) wrote :

This is ready for review

Andreas Hasenack (ahasenack) wrote :

I pushed a change I found out while testing the groovy package: motd-news-config has to have priority set to optional, otherwise it will inherit base-files' which is "required" and do-release-upgrade will install it regardless of dependencies.

Christian Ehrhardt  (paelzer) wrote :

Change matches what we had in Groovy and soon Focal.
Additional change for priority is good as well.
Versions are good as well for an SRU and the bug LGTM as well.

The breaks and rm_conffile versions are adapted correctly and the maintscript lines LGTM, but we really need to land the meta change at the same time to avoid some updaters to remove packages or even ubuntu-server.

+1 to this in general, but I found one triviality to fix.
The changelog entries got carried over without line-wrap. So you violate the 80 chars in two lines and tools like lintian will hate it.

Fix is trivial enough, you don't need a re-review for the CL changes - therefore approving now.

review: Approve
Andreas Hasenack (ahasenack) wrote :

I'm sorry, the longest d/changelog lines I see are at 75 chars:

  * motd/50-motd-news: use wget instead of curl, since wget is standard but

and

    - This changes the timeout behavior slightly because wget does not have

I ran lintian -I --pedantic and the only changelog issues it raiesd were on older entries:

P: base-files source: file-contains-trailing-whitespace debian/changelog (line 1678)
P: base-files source: file-contains-trailing-whitespace debian/changelog (line 915)

Andreas Hasenack (ahasenack) wrote :

Ah, it's in the xenial MP that the lines passed 80 columns.

Andreas Hasenack (ahasenack) wrote :

Tagging and uploading 08744d99f2c353f8db78e777d5f79044c1a1a8c2

$ git push pkg upload/10.1ubuntu2.10
Enumerating objects: 51, done.
Counting objects: 100% (51/51), done.
Delta compression using up to 4 threads
Compressing objects: 100% (38/38), done.
Writing objects: 100% (42/42), 6.63 KiB | 3.32 MiB/s, done.
Total 42 (delta 29), reused 5 (delta 3)
To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/base-files
 * [new tag] upload/10.1ubuntu2.10 -> upload/10.1ubuntu2.10

$ dput ubuntu ../base-files_10.1ubuntu2.10_source.changes
Checking signature on .changes
gpg: ../base-files_10.1ubuntu2.10_source.changes: Valid signature from AC983EB5BF6BCBA9
Checking signature on .dsc
gpg: ../base-files_10.1ubuntu2.10.dsc: Valid signature from AC983EB5BF6BCBA9
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading base-files_10.1ubuntu2.10.dsc: done.
  Uploading base-files_10.1ubuntu2.10.tar.xz: done.
  Uploading base-files_10.1ubuntu2.10_source.buildinfo: done.
  Uploading base-files_10.1ubuntu2.10_source.changes: done.
Successfully uploaded packages.

Andreas Hasenack (ahasenack) wrote :

Also uploading https://code.launchpad.net/~ahasenack/ubuntu/+source/ubuntu-meta/+git/ubuntu-meta/+ref/bionic-ubuntu-server-motd-news

The seed change will also happen, but since it involves a new package, the update script from ubuntu-meta won't pick it up, so I have to add the dep manually.

Tagging and uploading 28444d68eb81af94ee88e771755991866fddf638 of that branch:
$ git push pkg upload/1.417.5
Enumerating objects: 13, done.
Counting objects: 100% (13/13), done.
Delta compression using up to 4 threads
Compressing objects: 100% (9/9), done.
Writing objects: 100% (9/9), 1.13 KiB | 41.00 KiB/s, done.
Total 9 (delta 5), reused 0 (delta 0)
To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/ubuntu-meta
 * [new tag] upload/1.417.5 -> upload/1.417.5

$ dput ubuntu ../ubuntu-meta_1.417.5_source.changes
Checking signature on .changes
gpg: ../ubuntu-meta_1.417.5_source.changes: Valid signature from AC983EB5BF6BCBA9
Checking signature on .dsc
gpg: ../ubuntu-meta_1.417.5.dsc: Valid signature from AC983EB5BF6BCBA9
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading ubuntu-meta_1.417.5.dsc: done.
  Uploading ubuntu-meta_1.417.5.tar.xz: done.
  Uploading ubuntu-meta_1.417.5_source.buildinfo: done.
  Uploading ubuntu-meta_1.417.5_source.changes: done.
Successfully uploaded packages.

Andreas Hasenack (ahasenack) wrote :

Seeds changed for focal in 8b812c668c9cf4a6de14cbe9b38cc6c33a172735 after ok from steve.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/base-files.maintscript b/debian/base-files.maintscript
2index 0b59f8f..130d339 100644
3--- a/debian/base-files.maintscript
4+++ b/debian/base-files.maintscript
5@@ -3,3 +3,5 @@
6 # Remove the short-lived (within zesty dev cycle) /etc/update-motd.d/50-news file,
7 # which was renamed to /etc/update-motd.d/50-motd-news
8 rm_conffile /etc/update-motd.d/50-news 9.6ubuntu11~
9+
10+rm_conffile /etc/default/motd-news 10.1ubuntu2.10~ base-files
11diff --git a/debian/changelog b/debian/changelog
12index 3b75631..3087eb8 100644
13--- a/debian/changelog
14+++ b/debian/changelog
15@@ -1,3 +1,36 @@
16+base-files (10.1ubuntu2.10) bionic; urgency=medium
17+
18+ [ Andreas Hasenack ]
19+ * motd/50-motd-news: don't include uptime in the user-agent string
20+ (LP: #1886572)
21+ * Move the /etc/default/motd-news conffile to the motd-news-config
22+ package (LP: #1888575):
23+ - d/base-files.maintscript: remove /etc/default/motd-news config file
24+ on upgrade
25+ - d/control: break on ubuntu-server << 1.417.5 to force an upgrade if
26+ it is installed, which will pull motd-news-config and the conffile
27+ back in
28+ - d/motd-news-config.postinst:
29+ + handle the upgrade case where the motd-news config file was
30+ changed while it belonged to base-files
31+ + disable motd-news if the config file was removed by hand before
32+ the upgrade
33+ - d/postinst.in: signal the motd-news-config package if the
34+ motd-news config file was removed manually before the upgrade
35+ - d/control: new motd-news-config package, carrying the
36+ configuration file for the /etc/update-motd.d/50-motd-news script.
37+ - d/rules, d/motd-news-config.install: /e/d/motd-news is in the
38+ motd-news-config package now
39+
40+ [ Steve Langasek ]
41+ * motd/50-motd-news: use wget instead of curl, since wget is standard but
42+ curl is optional (LP: #1888572):
43+ - This changes the timeout behavior slightly because wget does not have
44+ an exact equivalent to curl's --max-time argument, we are using
45+ --timeout instead.
46+
47+ -- Andreas Hasenack <andreas@canonical.com> Thu, 13 Aug 2020 15:59:47 -0300
48+
49 base-files (10.1ubuntu2.9) bionic; urgency=medium
50
51 * /etc/issue, /etc/issue.net, /etc/lsb-release, /etc/os-release: Bump
52diff --git a/debian/control b/debian/control
53index a22308d..dcc292c 100644
54--- a/debian/control
55+++ b/debian/control
56@@ -14,7 +14,7 @@ Depends: ${misc:Depends}, ${shlibs:Depends}
57 Essential: yes
58 Priority: required
59 Replaces: base, miscutils, dpkg (<= 1.15.0)
60-Breaks: initscripts (<< 2.88dsf-13.3), sendfile (<< 2.1b.20080616-5.2~)
61+Breaks: initscripts (<< 2.88dsf-13.3), sendfile (<< 2.1b.20080616-5.2~), ubuntu-server (<< 1.417.5)
62 Multi-Arch: foreign
63 Description: Debian base system miscellaneous files
64 This package contains the basic filesystem hierarchy of a Debian system, and
65@@ -28,3 +28,15 @@ Section: debian-installer
66 Architecture: all
67 Priority: extra
68 Description: LSB release information
69+
70+Package: motd-news-config
71+Architecture: all
72+Priority: optional
73+Breaks: base-files (<< 10.1ubuntu2.10)
74+Replaces: base-files (<< 10.1ubuntu2.10)
75+Depends: ${shlibs:Depends}, ${misc:Depends}
76+Description: Configuration for motd-news shipped in base-files
77+ This package contains the configuration read by the motd-news script
78+ shipped in the base-files package.
79+ .
80+ Install this package if you want motd-news to be enabled.
81diff --git a/debian/motd-news-config.install b/debian/motd-news-config.install
82new file mode 100644
83index 0000000..876b3b8
84--- /dev/null
85+++ b/debian/motd-news-config.install
86@@ -0,0 +1 @@
87+debian/motd-news etc/default
88diff --git a/debian/motd-news-config.postinst b/debian/motd-news-config.postinst
89new file mode 100644
90index 0000000..a73427f
91--- /dev/null
92+++ b/debian/motd-news-config.postinst
93@@ -0,0 +1,55 @@
94+#!/bin/sh
95+# postinst script for motd-news-config
96+#
97+# see: dh_installdeb(1)
98+
99+set -e
100+
101+# summary of how this script can be called:
102+# * <postinst> `configure' <most-recently-configured-version>
103+# * <old-postinst> `abort-upgrade' <new version>
104+# * <conflictor's-postinst> `abort-remove' `in-favour' <package>
105+# <new-version>
106+# * <postinst> `abort-remove'
107+# * <deconfigured's-postinst> `abort-deconfigure' `in-favour'
108+# <failed-install-package> <version> `removing'
109+# <conflicting-package> <version>
110+# for details, see https://www.debian.org/doc/debian-policy/ or
111+# the debian-policy package
112+
113+
114+case "$1" in
115+ configure)
116+ # only run on new installs, $2 will be empty then
117+ if [ -z "$2" ]; then
118+ # /e/d/motd-news was moved from pkg:base-files to this package.
119+ # base-files runs rm_conffile which, if it was modified, leaves
120+ # /e/d/motd-news.dpkg-bak around. We want to preserve that *changed*
121+ # config file in this migration, and this is something that
122+ # rm_conffile is not handling. In that case, let's put the backup
123+ # file back in place
124+ if [ -e /etc/default/motd-news.dpkg-bak ]; then
125+ mv /etc/default/motd-news.dpkg-bak /etc/default/motd-news
126+ fi
127+ if [ -e /etc/default/motd-news.wasremoved ] && [ -e /etc/default/motd-news ]; then
128+ sed -i -e 's/^ENABLED=1/# Changed by motd-news-config.postinst:\n# config file was manually removed - disable the service\nENABLED=0/' /etc/default/motd-news
129+ rm /etc/default/motd-news.wasremoved
130+ fi
131+ fi
132+ ;;
133+
134+ abort-upgrade|abort-remove|abort-deconfigure)
135+ ;;
136+
137+ *)
138+ echo "postinst called with unknown argument \`$1'" >&2
139+ exit 1
140+ ;;
141+esac
142+
143+# dh_installdeb will replace this with shell code automatically
144+# generated by other debhelper scripts.
145+
146+#DEBHELPER#
147+
148+exit 0
149diff --git a/debian/postinst.in b/debian/postinst.in
150index 69435e3..c347a62 100644
151--- a/debian/postinst.in
152+++ b/debian/postinst.in
153@@ -125,4 +125,17 @@ if [ "$1" = "configure" ] && [ "$2" != "" ]; then
154 fi
155 fi
156
157+# special case of having /etc/default/motd-news removed by hand
158+# signal the motd-news-config package that this happened, so that
159+# it does not put back the file with default contents which would
160+# re-enable motd-news
161+motd_news_config="/etc/default/motd-news"
162+if [ ! -e ${motd_news_config} ]; then
163+ if [ ! -e ${motd_news_config}.dpkg-remove ]; then
164+ if [ ! -e ${motd_news_config}.dpkg-backup ]; then
165+ touch ${motd_news_config}.wasremoved
166+ fi
167+ fi
168+fi
169+
170 #DEBHELPER#
171diff --git a/debian/rules b/debian/rules
172index cb45e61..6c60a69 100755
173--- a/debian/rules
174+++ b/debian/rules
175@@ -24,7 +24,6 @@ override_dh_auto_build: locale-check
176
177 override_dh_auto_install:
178 install -p -m 644 etc/* $(DESTDIR)/etc
179- install -p -m 644 debian/motd-news $(DESTDIR)/etc/default
180 install -p -m 644 debian/motd-news.service $(DESTDIR)/lib/systemd/system/
181 install -p -m 644 debian/motd-news.timer $(DESTDIR)/lib/systemd/system/
182 install -p -m 644 licenses/* $(DESTDIR)/usr/share/common-licenses
183diff --git a/update-motd.d/50-motd-news b/update-motd.d/50-motd-news
184index 08d7f0d..c9d142f 100644
185--- a/update-motd.d/50-motd-news
186+++ b/update-motd.d/50-motd-news
187@@ -1,10 +1,11 @@
188 #!/bin/sh
189 #
190 # 50-motd-news - print the live news from the Ubuntu wire
191-# Copyright (C) 2016-2017 Canonical Ltd.
192+# Copyright (C) 2016-2020 Canonical Ltd.
193 # Copyright (C) 2016-2017 Dustin Kirkland
194 #
195 # Authors: Dustin Kirkland <kirkland@canonical.com>
196+# Steve Langasek <steve.langasek@canonical.com>
197 #
198 # This program is free software; you can redistribute it and/or modify
199 # it under the terms of the GNU General Public License as published by
200@@ -64,6 +65,9 @@ fi
201 # If we've made it here, we've been given the --force argument,
202 # probably from the systemd motd-news.service. Let's update...
203
204+# Abort early if wget is missing
205+[ -x /usr/bin/wget ] || exit 0
206+
207 # Generate our temp files, clean up when done
208 NEWS=$(mktemp) || exit 1
209 ERR=$(mktemp) || exit 1
210@@ -73,8 +77,8 @@ trap "rm -f $NEWS $ERR $CLOUD" HUP INT QUIT ILL TRAP KILL BUS TERM
211 # Construct a user agent, similar to Firefox/Chrome/Safari/IE to
212 # ensure a proper, tailored, accurate message of the day
213
214-# Curl browser version, for debug purposes
215-curl_ver="$(dpkg -l curl | awk '$1 == "ii" { print($3); exit(0); }')"
216+# wget browser version, for debug purposes
217+wget_ver="$(dpkg -l wget | awk '$1 == "ii" { print($3); exit(0); }')"
218
219 # Distribution version, for messages releated to this Ubuntu release
220 . /etc/lsb-release
221@@ -97,12 +101,8 @@ if [ -x /usr/bin/cloud-id ]; then
222 fi
223 fi
224
225-# Some messages may only be pertinent before or after some amount of uptime
226-read up idle < /proc/uptime
227-uptime="uptime/$up/$idle"
228-
229 # Piece together the user agent
230-USER_AGENT="curl/$curl_ver $lsb $platform $cpu $uptime cloud_id/$cloud_id"
231+USER_AGENT="wget/$wget_ver $lsb $platform $cpu cloud_id/$cloud_id"
232
233 # Loop over any configured URLs
234 for u in $URLS; do
235@@ -121,7 +121,18 @@ for u in $URLS; do
236 # If we're forced, set the wait to much higher (1 minute)
237 [ "$FORCED" = "1" ] && WAIT=60
238 # Fetch and print the news motd
239- if curl --connect-timeout "$WAIT" --max-time "$WAIT" -A "$USER_AGENT" -o- "$u" >"$NEWS" 2>"$ERR"; then
240+ result=0
241+ not_found_is_ok=0
242+ wget --timeout "$WAIT" -U "$USER_AGENT" -O- --content-on-error "$u" >"$NEWS" 2>"$ERR" || result=$?
243+ # from wget's manpage: 8 Server issued an error response.
244+ if [ $result -eq 8 ]; then
245+ if grep -q "ERROR 404" "$ERR"; then
246+ # The server's 404 document is the generic, non cloud-specific, motd-news
247+ # content present in the index.txt file
248+ not_found_is_ok=1
249+ fi
250+ fi
251+ if [ $result -eq 0 ] || [ $not_found_is_ok -eq 1 ]; then
252 echo
253 # At most, 10 lines of text, remove control characters, print at most 80 characters per line
254 safe_print "$NEWS"

Subscribers

People subscribed via source and target branches