Merge ~sergiodj/ubuntu/+source/sssd:merge-2.4.1-2-impish into ubuntu/+source/sssd:debian/sid

Proposed by Sergio Durigan Junior
Status: Approved
Approved by: Sergio Durigan Junior
Approved revision: 0ad309fe1b1a981a05823ac2d405d8487aed3f20
Proposed branch: ~sergiodj/ubuntu/+source/sssd:merge-2.4.1-2-impish
Merge into: ubuntu/+source/sssd:debian/sid
Diff against target: 315 lines (+236/-2)
7 files modified
debian/apparmor-profile (+5/-0)
debian/changelog (+85/-0)
debian/control (+3/-2)
debian/patches/disable-fail_over-tests.patch (+56/-0)
debian/patches/fix-python-tests.patch (+83/-0)
debian/patches/series (+2/-0)
debian/rules (+2/-0)
Reviewer Review Type Date Requested Status
Bryce Harrington Approve
Canonical Server Core Reviewers Pending
Review via email: mp+402945@code.launchpad.net

Description of the change

This is the merge of sssd 2.4.1-2 from Debian.

First, the good news. Upstream and Debian have both adopted a change that I proposed during last cycle, which means that we're now able to drop some of our delta.

Here's a rundown of what we're dropping:

- d/p/condition-path-exists-sssd-conf.patch and d/p/0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch: Used to make sssd.service start only if there is a configuration file present. I submitted an upstream patch which was accepted.

- d/p/lp-1908065-01-syslog_identifier-format.patch and d/p/lp-1908065-02-remove-syslog_identifier.patch: Used to remove custom SYSLOG_IDENTIFIER from journald, which was cluttering the logs and making them hard to read. This was a backport from upstream, which is now not needed.

During the merge process I also found a problem with the Python tests which affects only Ubuntu. The fix can be considered a general improvement, so I submitted it upstream here:

https://github.com/SSSD/sssd/pull/5636

and I am also incorporating it into our delta.

Now, the not-so-good news. There is another test failing: fail_over-tests. I spent some time debugging it and trying to understand what exactly is happening, but was unable to get to the bottom of the problem (I didn't want to spend too much time on it, since I have other things on my plate). I was able to determine that the problem seems to be related to libtevent, because the failure happens when "tevent_loop_once" is invoked. What's curious is that this failure only happens inside a chroot (when using sbuild, or when building the package on Launchpad, for example), and doesn't manifest on Debian. In all other scenarios the test passes, which tells me that this is probably sbuild/chroot-related and shouldn't impact regular users. For this reason, I am proposing that we temporarily disable this test. By the way, I got in touch with Timo (Debian's sssd maintainer) and he confirmed that he is aware of this issue but doesn't know why it happens.

As a side note, sssd has been recently re-enabled on i386 and you can check that it is currently not building there because of a missing dependency. I am already in contact with vorlon in order to get this sorted out; by the looks of it, it shouldn't be too much of a work.

There's a proposed PPA here:

https://launchpad.net/~sergiodj/+archive/ubuntu/sssd-merge/+packages

autopkgtest is still happy:

autopkgtest [18:32:44]: @@@@@@@@@@@@@@@@@@@@ summary
ldap-user-group-ldap-auth PASS
ldap-user-group-krb5-auth PASS

To post a comment you must log in.
Revision history for this message
Bryce Harrington (bryce) wrote :

* Changelog:
  - [√] old content and logical tag match as expected
  - [√] changelog entry correct version and targeted codename
  - [√] changelog entries correct
  - [√] update-maintainer has been run

* Actual changes:
  - [√] no upstream changes to consider
  - [√] no further upstream version to consider
  - [√] debian changes look safe

* Old Delta:
  - [√] dropped changes are ok to be dropped
  - [√] nothing else to drop
  - [√] changes forwarded upstream/debian (if appropriate)

* New Delta:
  - [-] no new patches added
  - [√] patches match what was proposed upstream
  - [√] patches correctly included in debian/patches/series
  - [√] patches have correct DEP3 metadata

* Build/Test:
  - [√] build is ok (modulo i386, as already addressed in mp desc)
  - [√] verified PPA package installs/uninstalls
  - [√] autopkgtest against the PPA package passes
  - [√] sanity checks test fine

Thorough analysis on remaining issues, and otherwise LGTM, +1.

review: Approve
Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

Thank you for the review, Bryce.

Uploaded & pushed:

$ git push pkg upload/2.4.1-2ubuntu1
Enumerating objects: 48, done.
Counting objects: 100% (48/48), done.
Delta compression using up to 8 threads
Compressing objects: 100% (40/40), done.
Writing objects: 100% (40/40), 7.60 KiB | 370.00 KiB/s, done.
Total 40 (delta 26), reused 0 (delta 0)
To ssh://git.launchpad.net/ubuntu/+source/sssd
 * [new tag] upload/2.4.1-2ubuntu1 -> upload/2.4.1-2ubuntu1

$ dput sssd_2.4.1-2ubuntu1_source.changes
Trying to upload package to ubuntu
Checking signature on .changes
gpg: /home/sergio/work/sssd/sssd_2.4.1-2ubuntu1_source.changes: Valid signature from 106DA1C8C3CBBF14
Checking signature on .dsc
gpg: /home/sergio/work/sssd/sssd_2.4.1-2ubuntu1.dsc: Valid signature from 106DA1C8C3CBBF14
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading sssd_2.4.1-2ubuntu1.dsc: done.
  Uploading sssd_2.4.1-2ubuntu1.debian.tar.xz: done.
  Uploading sssd_2.4.1-2ubuntu1_source.buildinfo: done.
  Uploading sssd_2.4.1-2ubuntu1_source.changes: done.
Successfully uploaded packages.

Unmerged commits

0ad309f... by Sergio Durigan Junior

update-maintainer

645b779... by Sergio Durigan Junior

reconstruct-changelog

11911f3... by Sergio Durigan Junior

merge-changelogs

48aab6b... by Sergio Durigan Junior

    - d/p/disable-fail_over-tests.patch: Disable fail_over-tests,
      which is failing when running inside sbuild.

08fa616... by Sergio Durigan Junior

  * Added changes:
    - d/p/fix-python-tests.patch: Fix Python tests by making them
      assert Python module paths by using full pathnames.

6af950f... by Sergio Durigan Junior

    - Avoid sending malformed SYSLOG_IDENTIFIER to journald (LP: #1908065):
      + d/p/lp-1908065-01-syslog_identifier-format.patch:
        Upstream patch to include "sssd[]" identifier in program names.
      + d/p/lp-1908065-02-remove-syslog_identifier.patch:
        Upstream patch to remove custom SYSLOG_IDENTIFIER from Journald.
      [ Included in 2.4.1-2 ]

726a168... by Sergio Durigan Junior

    - d/p/0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch:
      Upstream patch to make sssd.service only able to start when there
      is a configuration file present. (LP: #1900642)
    - d/p/condition-path-exists-sssd-conf.patch: Remove.
      [ Included in 2.4.1-2 ]

7933299... by Sergio Durigan Junior

  * Dropped changes:
    - d/p/condition-path-exists-sssd-conf.patch: Only start
      sssd.service if there is a configuration file present.
      (LP: #1900642)
      [ Included in 2.4.1-2 ]

0ce0376... by Sergio Durigan Junior

    - d/control: Drop libgdm-dev Build-Depend on i386.

2f5382f... by Sergio Durigan Junior

    - Disable lto, not ready upstream.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/apparmor-profile b/debian/apparmor-profile
2index c5f3658..ecf5f7d 100644
3--- a/debian/apparmor-profile
4+++ b/debian/apparmor-profile
5@@ -25,10 +25,15 @@
6 /etc/localtime r,
7 /etc/shells r,
8 /etc/sssd/sssd.conf r,
9+ /etc/sssd/conf.d/ r,
10+ /etc/sssd/conf.d/** r,
11+ /etc/gss/mech.d/ r,
12+ /etc/gss/mech.d/** r,
13
14 /usr/lib/@{multiarch}/ldb/modules/ldb/* m,
15 /usr/lib/@{multiarch}/samba/ldb/* m,
16 /usr/lib/@{multiarch}/sssd/* rix,
17+ /usr/libexec/sssd/* rmix,
18 /usr/sbin/sssd rmix,
19
20 /tmp/{,.}krb5cc_* rwk,
21diff --git a/debian/changelog b/debian/changelog
22index 2e1562c..b7532c3 100644
23--- a/debian/changelog
24+++ b/debian/changelog
25@@ -1,3 +1,35 @@
26+sssd (2.4.1-2ubuntu1) impish; urgency=medium
27+
28+ * Merge with Debian unstable. Remaining changes:
29+ - d/apparmor-profile: Update profile. (LP #1910611)
30+ + Extend read permissions to /etc/sssd/** and /etc/gss/**.
31+ + Add read/execute permission to /usr/libexec/sssd/*.
32+ - Disable lto, not ready upstream.
33+ - d/control: Drop libgdm-dev Build-Depend on i386.
34+ * Dropped changes:
35+ - d/p/condition-path-exists-sssd-conf.patch: Only start
36+ sssd.service if there is a configuration file present.
37+ (LP: #1900642)
38+ [ Included in 2.4.1-2 ]
39+ - d/p/0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch:
40+ Upstream patch to make sssd.service only able to start when there
41+ is a configuration file present. (LP #1900642)
42+ - d/p/condition-path-exists-sssd-conf.patch: Remove.
43+ [ Included in 2.4.1-2 ]
44+ - Avoid sending malformed SYSLOG_IDENTIFIER to journald (LP #1908065):
45+ + d/p/lp-1908065-01-syslog_identifier-format.patch:
46+ Upstream patch to include "sssd[]" identifier in program names.
47+ + d/p/lp-1908065-02-remove-syslog_identifier.patch:
48+ Upstream patch to remove custom SYSLOG_IDENTIFIER from Journald.
49+ [ Included in 2.4.1-2 ]
50+ * Added changes:
51+ - d/p/fix-python-tests.patch: Fix Python tests by making them
52+ assert Python module paths by using full pathnames.
53+ - d/p/disable-fail_over-tests.patch: Disable fail_over-tests,
54+ which is failing when running inside sbuild.
55+
56+ -- Sergio Durigan Junior <sergio.durigan@canonical.com> Tue, 18 May 2021 17:29:58 -0400
57+
58 sssd (2.4.1-2) unstable; urgency=medium
59
60 [ Marco Trevisan (Treviño) ]
61@@ -23,6 +55,59 @@ sssd (2.4.1-1) unstable; urgency=medium
62
63 -- Timo Aaltonen <tjaalton@debian.org> Wed, 10 Feb 2021 11:32:35 +0200
64
65+sssd (2.4.0-1ubuntu7) impish; urgency=medium
66+
67+ * d/control: Drop libgdm-dev Build-Depend on i386.
68+
69+ -- Sergio Durigan Junior <sergio.durigan@canonical.com> Tue, 11 May 2021 16:22:31 -0400
70+
71+sssd (2.4.0-1ubuntu6) hirsute; urgency=medium
72+
73+ * Disable lto, not ready upstream.
74+
75+ -- Matthias Klose <doko@ubuntu.com> Tue, 23 Mar 2021 13:18:53 +0100
76+
77+sssd (2.4.0-1ubuntu5) hirsute; urgency=medium
78+
79+ * No change rebuild with fixed ownership.
80+
81+ -- Dimitri John Ledkov <xnox@ubuntu.com> Tue, 16 Feb 2021 15:22:14 +0000
82+
83+sssd (2.4.0-1ubuntu4) hirsute; urgency=medium
84+
85+ * Avoid sending malformed SYSLOG_IDENTIFIER to journald (LP: #1908065):
86+ - d/p/lp-1908065-01-syslog_identifier-format.patch:
87+ Upstream patch to include "sssd[]" identifier in program names.
88+ - d/p/lp-1908065-02-remove-syslog_identifier.patch:
89+ Upstream patch to remove custom SYSLOG_IDENTIFIER from Journald.
90+
91+ -- Valters Jansons <valter.jansons@gmail.com> Fri, 05 Feb 2021 20:51:32 +0000
92+
93+sssd (2.4.0-1ubuntu3) hirsute; urgency=medium
94+
95+ * d/apparmor-profile: Update profile. (LP: #1910611)
96+ - Extend read permissions to /etc/sssd/conf.d/* and /etc/gss/mech.d/*.
97+ - Add read/execute permission to /usr/libexec/sssd/*.
98+
99+ -- Sergio Durigan Junior <sergio.durigan@canonical.com> Mon, 18 Jan 2021 16:57:21 -0500
100+
101+sssd (2.4.0-1ubuntu2) hirsute; urgency=medium
102+
103+ * d/p/0003-Only-start-sssd.service-if-there-s-a-configuration-f.patch:
104+ Upstream patch to make sssd.service only able to start when there
105+ is a configuration file present. (LP: #1900642)
106+ * d/p/condition-path-exists-sssd-conf.patch: Remove.
107+
108+ -- Sergio Durigan Junior <sergio.durigan@canonical.com> Tue, 12 Jan 2021 16:17:38 -0500
109+
110+sssd (2.4.0-1ubuntu1) hirsute; urgency=medium
111+
112+ * d/p/condition-path-exists-sssd-conf.patch: Only start
113+ sssd.service if there is a configuration file present.
114+ (LP: #1900642)
115+
116+ -- Sergio Durigan Junior <sergio.durigan@canonical.com> Thu, 10 Dec 2020 14:20:24 -0500
117+
118 sssd (2.4.0-1) unstable; urgency=medium
119
120 * New upstream release.
121diff --git a/debian/control b/debian/control
122index 6389b33..49e4ee0 100644
123--- a/debian/control
124+++ b/debian/control
125@@ -1,7 +1,8 @@
126 Source: sssd
127 Section: utils
128 Priority: optional
129-Maintainer: Debian SSSD Team <pkg-sssd-devel@alioth-lists.debian.net>
130+Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
131+XSBC-Original-Maintainer: Debian SSSD Team <pkg-sssd-devel@alioth-lists.debian.net>
132 Uploaders: Timo Aaltonen <tjaalton@debian.org>,
133 Dominik George <natureshadow@debian.org>
134 Build-Depends:
135@@ -25,7 +26,7 @@ Build-Depends:
136 libcollection-dev,
137 libdbus-1-dev,
138 libdhash-dev,
139- libgdm-dev [!s390x !kfreebsd-any !hurd-any],
140+ libgdm-dev [!s390x !kfreebsd-any !hurd-any !i386],
141 libglib2.0-dev,
142 libini-config-dev,
143 libjansson-dev,
144diff --git a/debian/patches/disable-fail_over-tests.patch b/debian/patches/disable-fail_over-tests.patch
145new file mode 100644
146index 0000000..7baff04
147--- /dev/null
148+++ b/debian/patches/disable-fail_over-tests.patch
149@@ -0,0 +1,56 @@
150+From: Sergio Durigan Junior <sergio.durigan@canonical.com>
151+Date: Tue, 18 May 2021 17:05:28 -0400
152+Subject: Disable fail_over-tests
153+
154+fail_over-tests is failing when running inside an impish
155+chroot (sbuild) on Ubuntu. This is a strange failure; it doesn't
156+happen on Debian, for example.
157+
158+I was not able to determine the root cause of it, but I was getting
159+close: it seems the problem happens when the "test_loop (test_ctx);"
160+function is invoked inside "test_resolve_service_callback". This
161+tells me that the problem is likely happening inside libtevent.
162+
163+For now, it is safe to just disable this test from running because I
164+was able to verify that it normally passes when one builds and tests
165+sssd inside a container/VM with dpkg-buildpackage, for example.
166+
167+Forwarded: not-needed
168+Last-Updated: 2021-05-18
169+---
170+ Makefile.am | 15 ---------------
171+ 1 file changed, 15 deletions(-)
172+
173+diff --git a/Makefile.am b/Makefile.am
174+index f0083ff..e36e569 100644
175+--- a/Makefile.am
176++++ b/Makefile.am
177+@@ -225,7 +225,6 @@ if HAVE_CHECK
178+ check_and_open-tests \
179+ files-tests \
180+ refcount-tests \
181+- fail_over-tests \
182+ find_uid-tests \
183+ auth-tests \
184+ ipa_ldap_opt-tests \
185+@@ -2387,20 +2386,6 @@ refcount_tests_LDADD = \
186+ $(SSSD_INTERNAL_LTLIBS) \
187+ libsss_test_common.la
188+
189+-fail_over_tests_SOURCES = \
190+- src/tests/fail_over-tests.c \
191+- $(SSSD_FAILOVER_OBJ) \
192+- $(NULL)
193+-fail_over_tests_CFLAGS = \
194+- $(AM_CFLAGS) \
195+- $(CHECK_CFLAGS)
196+-fail_over_tests_LDADD = \
197+- $(SSSD_LIBS) \
198+- $(CHECK_LIBS) \
199+- $(CARES_LIBS) \
200+- $(SSSD_INTERNAL_LTLIBS) \
201+- libsss_test_common.la
202+-
203+ find_uid_tests_SOURCES = \
204+ src/tests/find_uid-tests.c \
205+ src/util/find_uid.c \
206diff --git a/debian/patches/fix-python-tests.patch b/debian/patches/fix-python-tests.patch
207new file mode 100644
208index 0000000..5053f8e
209--- /dev/null
210+++ b/debian/patches/fix-python-tests.patch
211@@ -0,0 +1,83 @@
212+From: Sergio Durigan Junior <sergio.durigan@canonical.com>
213+Date: Mon, 17 May 2021 19:09:14 -0400
214+Subject: Improve assertion when verifying paths for Python modules
215+
216+In Ubuntu we're facing a problem where the 3 Python tests under
217+src/tests/*-test.py are failing due to cosmetical differences between
218+what the '.__file__' method returns and what 'MODPATH' ends up being.
219+
220+I have not been able to pinpoint exactly what is causing this issue;
221+it only happens when SSSD is built inside a chroot environment (with
222+sbuild, for example). The logs look like this:
223+
224+F
225+======================================================================
226+FAIL: testImport (__main__.PyHbacImport)
227+Import the module and assert it comes from tree
228+----------------------------------------------------------------------
229+Traceback (most recent call last):
230+ File "/<<PKGBUILDDIR>>/src/tests/pyhbac-test.py", line 91, in testImport
231+ self.assertEqual(pyhbac.__file__, MODPATH + "/pyhbac.so")
232+AssertionError: '/<<PKGBUILDDIR>>/build/./tp_pyhbac_xw2omut2/pyhbac.so' != './tp_pyhbac_xw2omut2/pyhbac.so'
233+- /<<PKGBUILDDIR>>/build/./tp_pyhbac_xw2omut2/pyhbac.so
234++ ./tp_pyhbac_xw2omut2/pyhbac.so
235+
236+Given that the intention of the test is to verify that the two paths
237+are equal, I suggest that we do this slight improvement and call
238+'os.path.realpath' before comparing both paths. This way we guarantee
239+that they're both properly canonicalized.
240+
241+I have verified that the tests still pass with this change.
242+
243+Signed-off-by: Sergio Durigan Junior <sergio.durigan@canonical.com>
244+
245+Forwarded: yes, https://github.com/SSSD/sssd/pull/5636
246+Last-Updated: 2021-05-18
247+---
248+ src/tests/pyhbac-test.py | 3 ++-
249+ src/tests/pysss-test.py | 3 ++-
250+ src/tests/pysss_murmur-test.py | 3 ++-
251+ 3 files changed, 6 insertions(+), 3 deletions(-)
252+
253+diff --git a/src/tests/pyhbac-test.py b/src/tests/pyhbac-test.py
254+index 06163af..c8ce47f 100755
255+--- a/src/tests/pyhbac-test.py
256++++ b/src/tests/pyhbac-test.py
257+@@ -88,7 +88,8 @@ class PyHbacImport(unittest.TestCase):
258+ print("Could not load the pyhbac module. Please check if it is "
259+ "compiled", file=sys.stderr)
260+ raise e
261+- self.assertEqual(pyhbac.__file__, MODPATH + "/pyhbac.so")
262++ self.assertEqual(os.path.realpath(pyhbac.__file__),
263++ os.path.realpath(MODPATH + "/pyhbac.so"))
264+
265+
266+ class PyHbacRuleElementTest(unittest.TestCase):
267+diff --git a/src/tests/pysss-test.py b/src/tests/pysss-test.py
268+index 30bc074..20ef0ab 100755
269+--- a/src/tests/pysss-test.py
270++++ b/src/tests/pysss-test.py
271+@@ -58,7 +58,8 @@ class PysssImport(unittest.TestCase):
272+ print("Could not load the pysss module. Please check if it is "
273+ "compiled", file=sys.stderr)
274+ raise ex
275+- self.assertEqual(pysss.__file__, MODPATH + "/pysss.so")
276++ self.assertEqual(os.path.realpath(pysss.__file__),
277++ os.path.realpath(MODPATH + "/pysss.so"))
278+
279+
280+ class PysssEncryptTest(unittest.TestCase):
281+diff --git a/src/tests/pysss_murmur-test.py b/src/tests/pysss_murmur-test.py
282+index 531f8b5..75b4651 100755
283+--- a/src/tests/pysss_murmur-test.py
284++++ b/src/tests/pysss_murmur-test.py
285+@@ -59,7 +59,8 @@ class PySssMurmurImport(unittest.TestCase):
286+ print("Could not load the pysss_murmur module. "
287+ "Please check if it is compiled", file=sys.stderr)
288+ raise e
289+- self.assertEqual(pysss_murmur.__file__, MODPATH + "/pysss_murmur.so")
290++ self.assertEqual(os.path.realpath(pysss_murmur.__file__),
291++ os.path.realpath(MODPATH + "/pysss_murmur.so"))
292+
293+
294+ class PySssMurmurTestNeg(unittest.TestCase):
295diff --git a/debian/patches/series b/debian/patches/series
296index d83fab8..1898b49 100644
297--- a/debian/patches/series
298+++ b/debian/patches/series
299@@ -1,2 +1,4 @@
300 fix-whitespace-test.diff
301 default-to-socket-activated-services.diff
302+fix-python-tests.patch
303+disable-fail_over-tests.patch
304diff --git a/debian/rules b/debian/rules
305index f4d3bbd..d6487c4 100755
306--- a/debian/rules
307+++ b/debian/rules
308@@ -3,6 +3,8 @@
309 dh $@ --with python3 \
310 --builddirectory=build
311
312+export DEB_BUILD_MAINT_OPTIONS = optimize=-lto
313+
314 DPKG_EXPORT_BUILDFLAGS = 1
315 include /usr/share/dpkg/buildflags.mk
316

Subscribers

People subscribed via source and target branches