Merge ~ahasenack/ubuntu/+source/base-files:focal-handle-was-removed-1895302 into ubuntu/+source/base-files:ubuntu/devel

Proposed by Andreas Hasenack on 2020-09-15
Status: Superseded
Proposed branch: ~ahasenack/ubuntu/+source/base-files:focal-handle-was-removed-1895302
Merge into: ubuntu/+source/base-files:ubuntu/devel
Diff against target: 234 lines (+95/-0) (has conflicts)
9 files modified
debian/base-files.maintscript (+4/-0)
debian/changelog (+40/-0)
debian/control (+9/-0)
debian/motd-news-config.postinst (+8/-0)
debian/postinst.in (+12/-0)
etc/issue (+4/-0)
etc/issue.net (+4/-0)
etc/lsb-release (+6/-0)
etc/os-release (+8/-0)
Conflict in debian/base-files.maintscript
Conflict in debian/changelog
Conflict in debian/control
Conflict in debian/motd-news-config.postinst
Conflict in debian/postinst.in
Conflict in etc/issue
Conflict in etc/issue.net
Conflict in etc/lsb-release
Conflict in etc/os-release
Reviewer Review Type Date Requested Status
Canonical Server Core Reviewers 2020-09-15 Pending
Canonical Server Team 2020-09-15 Pending
Review via email: mp+390765@code.launchpad.net

This proposal has been superseded by a proposal from 2020-09-15.

Description of the change

Handle the creation and removal of the /etc/default/motd-news.wasremoved file in maintainer scripts.

I have two choices here:
a) handle the creation and removal of that file (this MP)
b) give up trying to handle that file, and just cleanup: https://git.launchpad.net/~ahasenack/ubuntu/+source/base-files/commit/?h=groovy-drop-wasremoved&id=1c8983d04fdadf831976cbe42259a4b9b5b79d0b

To recap, the idea behind /e/d/motd-news.wasremoved was to handle the case where:
- user had base-files and ubuntu-server from before the split, meaning /e/d/motd-news is a config file part of base-files
- motd-news is enabled (ENABLED=1 in /e/d/motd-news)
- in an erroneous attempt to disable the service, the user removes the /e/d/motd-news file. ENABLED doesn't get set, and the motd-news script just exits

In the above scenario, without the .wasremoved feature, if the user upgraded to the updated packages with the spit, then /e/d/motd-news would be reinstated because it's now part of the motd-news-config package. This would re-enable the motd-news feature, something we want to be careful about.

The implementation suffers from some problems, though:
- a fresh install of base-files, like done via debootstrap, will create the .wasremoved file. If you then install motd-news-config (via apt install ubuntu-server, for example), the motd-news-config postinst fill find the .wasremoved file, see it's a fresh install of motd-news-config, and disable motd-news. This is the current situation in the groovy lxd images, for example
- if you just have base-files installed, no motd-news-config, and then reinstall or upgrade base-files, the .wasremoved will be created

To address those, this MP (choice (a)) does:
- motd-news-config postinst: always remove the .wasremoved file in configure if found, regardless if /etc/default/motd-news was sed'ed or not, or if we are upgrading or on a first install
- base-files postinst: guard the creation of .wasremoved with:
  - Only during an upgrade
  - Only if ubuntu-server is installed (via a dpkg -l check)

If a user again decides to erroneously disable motd-news by removing /e/d/motd-news, then an upgrade of base-files, motd-news-config and ubuntu-server will go through the creation and removal of the .wasremoved file, but no new /e/d/motd-news config file will be created or changed, and motd-news will at least remain disabled. But it's extra unnecessary churn, as the corner case it handles only happens when upgrading from before the motd-news-config split.

The dpkg -l check for ubuntu-server relies on ubuntu-server depending (and not just recommending) on motd-news-config. I'm a bit afraid of the dpkg -l call in base-files' postinst:
- Locking issues?
- I'm checking for ^i, should I be looking for other cases? Other flags?
- At least dpkg is already installed, as I see other calls to it in the same postinst

If we find other issues in these maintainer scripts regarding the motd-news-config split, or introduce new ones with these changes, it will get harder and harder to fix each time. That's why I also have to consider simply giving up and living with the corner case we tried to handle in the beginning, and that is this diff: https://git.launchpad.net/~ahasenack/ubuntu/+source/base-files/commit/?h=groovy-drop-wasremoved&id=1c8983d04fdadf831976cbe42259a4b9b5b79d0b Just note that in this case, the corner case scenario will leave motd-news enabled when it was supposed to be disabled.

I have PPAs for both scenarios, with groovy and focal packages (just not yet bionic or xenial):

Give up handling of .wasremoved: https://launchpad.net/~ahasenack/+archive/ubuntu/motd-news-drop-wasremoved/

Try to fix handling of .wasremoved: https://launchpad.net/~ahasenack/+archive/ubuntu/motd-news-wasremoved-handling/

May the best one win :)

To post a comment you must log in.

Unmerged commits

8a66e5c... by Andreas Hasenack on 2020-09-14

changelog

6fcd366... by Andreas Hasenack on 2020-09-11

  * d/motd-news-config.postinst: always remove /e/d/motd-news.wasremoved
    if present

9b68a73... by Andreas Hasenack on 2020-09-11

  * d/postinst.in: re-arrange the cascading conditionals around the
    creation of the .wasremoved file for better clarity

03931a2... by Andreas Hasenack on 2020-09-11

  * d/postinst.in: check if ubuntu-server is installed before creating
    the .wasremoved file

6f3fbdb... by Andreas Hasenack on 2020-08-14

  * d/control: set priority to optional for motd-news-config to avoid
    having it pulled in by the release upgrader

1c5b4b9... by Andreas Hasenack on 2020-08-17

changelog

d06cb18... by Steve Langasek on 2020-07-10

  [ Steve Langasek ]
  * motd/50-motd-news: use wget instead of curl, since wget is standard but
    curl is optional (LP: #1888572):
    - This changes the timeout behavior slightly because wget does not have
      an exact equivalent to curl's --max-time argument, we are using
      --timeout instead.

c4e4748... by Andreas Hasenack on 2020-08-06

    - d/rules, d/motd-news-config.install: /e/d/motd-news is in the
      motd-news-config package now

d217183... by Andreas Hasenack on 2020-08-06

    - d/control: new motd-news-config package, carrying the
      configuration file for the /etc/update-motd.d/50-motd-news script.

78f2dc7... by Andreas Hasenack on 2020-08-10

    - d/postinst.in: signal the motd-news-config package if the
      motd-news config file was removed manually before the upgrade

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 9972f51..b1e3c88 100644
3--- a/debian/base-files.maintscript
4+++ b/debian/base-files.maintscript
5@@ -1 +1,5 @@
6+<<<<<<< debian/base-files.maintscript
7 rm_conffile /etc/default/motd-news 11ubuntu11~ base-files
8+=======
9+rm_conffile /etc/default/motd-news 11ubuntu5.2~ base-files
10+>>>>>>> debian/base-files.maintscript
11diff --git a/debian/changelog b/debian/changelog
12index eb44a50..5146861 100644
13--- a/debian/changelog
14+++ b/debian/changelog
15@@ -1,3 +1,4 @@
16+<<<<<<< debian/changelog
17 base-files (11ubuntu12) groovy; urgency=medium
18
19 * d/control: set priority to optional for motd-news-config to avoid
20@@ -7,11 +8,33 @@ base-files (11ubuntu12) groovy; urgency=medium
21
22 base-files (11ubuntu11) groovy; urgency=medium
23
24+=======
25+base-files (11ubuntu5.3) focal; urgency=medium
26+
27+ * d/postinst.in: check if ubuntu-server is installed before creating
28+ the .wasremoved file
29+ * d/postinst.in: re-arrange the cascading conditionals around the
30+ creation of the .wasremoved file for better clarity
31+ * d/motd-news-config.postinst: always remove /e/d/motd-news.wasremoved
32+ if present
33+
34+ -- Andreas Hasenack <andreas@canonical.com> Mon, 14 Sep 2020 18:06:37 -0300
35+
36+base-files (11ubuntu5.2) focal; urgency=medium
37+
38+ [ Andreas Hasenack ]
39+ * motd/50-motd-news: don't include uptime in the user-agent string
40+ (LP: #1886572)
41+>>>>>>> debian/changelog
42 * Move the /etc/default/motd-news conffile to the motd-news-config
43 package (LP: #1888575):
44 - d/base-files.maintscript: remove /etc/default/motd-news config file
45 on upgrade
46+<<<<<<< debian/changelog
47 - d/control: break on ubuntu-server << 1.453 to force an upgrade if
48+=======
49+ - d/control: break on ubuntu-server << 1.450.2 to force an upgrade if
50+>>>>>>> debian/changelog
51 it is installed, which will pull motd-news-config and the conffile
52 back in
53 - d/motd-news-config.postinst:
54@@ -26,6 +49,7 @@ base-files (11ubuntu11) groovy; urgency=medium
55 - d/rules, d/motd-news-config.install: /e/d/motd-news is in the
56 motd-news-config package now
57
58+<<<<<<< debian/changelog
59 -- Andreas Hasenack <andreas@canonical.com> Mon, 10 Aug 2020 21:02:37 +0000
60
61 base-files (11ubuntu10) groovy; urgency=medium
62@@ -41,10 +65,16 @@ base-files (11ubuntu9) groovy; urgency=medium
63
64 * motd/50-motd-news: use wget instead of curl, since wget is standard but
65 curl is optional.
66+=======
67+ [ Steve Langasek ]
68+ * motd/50-motd-news: use wget instead of curl, since wget is standard but
69+ curl is optional (LP: #1888572):
70+>>>>>>> debian/changelog
71 - This changes the timeout behavior slightly because wget does not have
72 an exact equivalent to curl's --max-time argument, we are using
73 --timeout instead.
74
75+<<<<<<< debian/changelog
76 -- Steve Langasek <steve.langasek@ubuntu.com> Fri, 10 Jul 2020 09:58:46 -0700
77
78 base-files (11ubuntu8) groovy; urgency=medium
79@@ -66,6 +96,16 @@ base-files (11ubuntu6) groovy; urgency=medium
80 * /etc/issue{,.net}, /etc/{lsb,os}-release: Welcome to Groovy Gorilla!
81
82 -- Matthias Klose <doko@ubuntu.com> Fri, 24 Apr 2020 17:11:27 +0200
83+=======
84+ -- Andreas Hasenack <andreas@canonical.com> Mon, 17 Aug 2020 10:31:58 -0300
85+
86+base-files (11ubuntu5.1) focal; urgency=medium
87+
88+ * /etc/issue, /etc/issue.net, /etc/lsb-release, /etc/os-release: Bump
89+ version number to 20.04.1 in preparation of the next point release.
90+
91+ -- Łukasz 'sil2100' Zemczak <lukasz.zemczak@ubuntu.com> Wed, 15 Jul 2020 10:59:48 +0200
92+>>>>>>> debian/changelog
93
94 base-files (11ubuntu5) focal; urgency=medium
95
96diff --git a/debian/control b/debian/control
97index 1dbf9b9..d87d63a 100644
98--- a/debian/control
99+++ b/debian/control
100@@ -15,7 +15,11 @@ Essential: yes
101 Priority: required
102 Replaces: base, miscutils, dpkg (<= 1.15.0)
103 Breaks: debian-security-support (<< 2019.04.25), initscripts (<< 2.88dsf-13.3), sendfile (<< 2.1b.20080616-5.2~),
104+<<<<<<< debian/control
105 ubuntu-server (<< 1.453)
106+=======
107+ ubuntu-server (<< 1.450.2)
108+>>>>>>> debian/control
109 Multi-Arch: foreign
110 Description: Debian base system miscellaneous files
111 This package contains the basic filesystem hierarchy of a Debian system, and
112@@ -33,8 +37,13 @@ Description: LSB release information
113 Package: motd-news-config
114 Architecture: all
115 Priority: optional
116+<<<<<<< debian/control
117 Breaks: base-files (<< 11ubuntu11)
118 Replaces: base-files (<< 11ubuntu11)
119+=======
120+Breaks: base-files (<< 11ubuntu5.2)
121+Replaces: base-files (<< 11ubuntu5.2)
122+>>>>>>> debian/control
123 Depends: ${shlibs:Depends}, ${misc:Depends}
124 Description: Configuration for motd-news shipped in base-files
125 This package contains the configuration read by the motd-news script
126diff --git a/debian/motd-news-config.postinst b/debian/motd-news-config.postinst
127index a73427f..2b4fafd 100644
128--- a/debian/motd-news-config.postinst
129+++ b/debian/motd-news-config.postinst
130@@ -33,9 +33,17 @@ case "$1" in
131 fi
132 if [ -e /etc/default/motd-news.wasremoved ] && [ -e /etc/default/motd-news ]; then
133 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
134+<<<<<<< debian/motd-news-config.postinst
135 rm /etc/default/motd-news.wasremoved
136 fi
137 fi
138+=======
139+ fi
140+ fi
141+ if [ -e /etc/default/motd-news.wasremoved ]; then
142+ rm /etc/default/motd-news.wasremoved
143+ fi
144+>>>>>>> debian/motd-news-config.postinst
145 ;;
146
147 abort-upgrade|abort-remove|abort-deconfigure)
148diff --git a/debian/postinst.in b/debian/postinst.in
149index c347a62..c81e89a 100644
150--- a/debian/postinst.in
151+++ b/debian/postinst.in
152@@ -130,12 +130,24 @@ fi
153 # it does not put back the file with default contents which would
154 # re-enable motd-news
155 motd_news_config="/etc/default/motd-news"
156+<<<<<<< debian/postinst.in
157 if [ ! -e ${motd_news_config} ]; then
158 if [ ! -e ${motd_news_config}.dpkg-remove ]; then
159 if [ ! -e ${motd_news_config}.dpkg-backup ]; then
160 touch ${motd_news_config}.wasremoved
161 fi
162 fi
163+=======
164+if [ "$2" != "" ] && \
165+ [ ! -e ${motd_news_config} ] && \
166+ [ ! -e ${motd_news_config}.dpkg-remove ] && \
167+ [ ! -e ${motd_news_config}.dpkg-backup ]; then
168+ # The .wasremoved file only matters if ubuntu-server is installed,
169+ # because that's what will pull in motd-news-config
170+ if dpkg -l ubuntu-server 2>/dev/null | grep -q ^i; then
171+ touch ${motd_news_config}.wasremoved
172+ fi
173+>>>>>>> debian/postinst.in
174 fi
175
176 #DEBHELPER#
177diff --git a/etc/issue b/etc/issue
178index 62288be..eb7b599 100644
179--- a/etc/issue
180+++ b/etc/issue
181@@ -1,2 +1,6 @@
182+<<<<<<< etc/issue
183 Ubuntu Groovy Gorilla (development branch) \n \l
184+=======
185+Ubuntu 20.04.1 LTS \n \l
186+>>>>>>> etc/issue
187
188diff --git a/etc/issue.net b/etc/issue.net
189index e86fc46..4b1d230 100644
190--- a/etc/issue.net
191+++ b/etc/issue.net
192@@ -1 +1,5 @@
193+<<<<<<< etc/issue.net
194 Ubuntu Groovy Gorilla (development branch)
195+=======
196+Ubuntu 20.04.1 LTS
197+>>>>>>> etc/issue.net
198diff --git a/etc/lsb-release b/etc/lsb-release
199index b08a2f9..83ad511 100644
200--- a/etc/lsb-release
201+++ b/etc/lsb-release
202@@ -1,4 +1,10 @@
203 DISTRIB_ID=Ubuntu
204+<<<<<<< etc/lsb-release
205 DISTRIB_RELEASE=20.10
206 DISTRIB_CODENAME=groovy
207 DISTRIB_DESCRIPTION="Ubuntu Groovy Gorilla (development branch)"
208+=======
209+DISTRIB_RELEASE=20.04
210+DISTRIB_CODENAME=focal
211+DISTRIB_DESCRIPTION="Ubuntu 20.04.1 LTS"
212+>>>>>>> etc/lsb-release
213diff --git a/etc/os-release b/etc/os-release
214index 3ccc041..aac715d 100644
215--- a/etc/os-release
216+++ b/etc/os-release
217@@ -1,9 +1,17 @@
218 NAME="Ubuntu"
219+<<<<<<< etc/os-release
220 VERSION="20.10 (Groovy Gorilla)"
221 ID=ubuntu
222 ID_LIKE=debian
223 PRETTY_NAME="Ubuntu Groovy Gorilla (development branch)"
224 VERSION_ID="20.10"
225+=======
226+VERSION="20.04.1 LTS (Focal Fossa)"
227+ID=ubuntu
228+ID_LIKE=debian
229+PRETTY_NAME="Ubuntu 20.04.1 LTS"
230+VERSION_ID="20.04"
231+>>>>>>> etc/os-release
232 HOME_URL="https://www.ubuntu.com/"
233 SUPPORT_URL="https://help.ubuntu.com/"
234 BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"

Subscribers

People subscribed via source and target branches