Merge ~utkarsh/ubuntu/+source/autofs:merge-1917423-hirsute into ubuntu/+source/autofs:debian/sid

Proposed by Utkarsh Gupta
Status: Merged
Merge reported by: Utkarsh Gupta
Merged at revision: 0de4df081bf44c6b74d48a48815004cd79223ee1
Proposed branch: ~utkarsh/ubuntu/+source/autofs:merge-1917423-hirsute
Merge into: ubuntu/+source/autofs:debian/sid
Diff against target: 81 lines (+40/-1)
2 files modified
debian/changelog (+38/-0)
debian/control (+2/-1)
Reviewer Review Type Date Requested Status
Bryce Harrington Approve
Robie Basak Pending
Canonical Server Team Pending
Canonical Server packageset reviewers Pending
Ubuntu Server Dev import team Pending
Review via email: mp+398955@code.launchpad.net

Description of the change

All changes for autofs are now upstream in 5.1.7, so this can be a sync.
Just filing MP as double-check for review.

Usual tags have been pushed.

PPA: https://launchpad.net/~utkarsh/+archive/ubuntu/autofs-merge-lp1917423

All tests are passing as well:
```
autopkgtest [15:40:22]: test nfs-mount: -----------------------]
autopkgtest [15:40:23]: test nfs-mount: - - - - - - - - - - results - - - - - - - - - -
nfs-mount PASS
autopkgtest [15:40:24]: @@@@@@@@@@@@@@@@@@@@ summary
smb-mount PASS
nfs-mount PASS
```

To post a comment you must log in.
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

5.1.7 upstream introduces a somewhat big change: a new shared library. Are we sure this doesn't need an FFe?

The changelog also has many other changes that would need to be analyzed, like:
- update ldap READMEs and schema definitions.
- remove intr hosts map mount option.
- add support for new sss autofs proto version call.
- (many refactors)
- add hashtable implementation.
- make autofs.a a shared library.

How debian dealt with the shread library:
  * debian/autofs.install:
    + Install libautofs.so into bin:pkg autofs (for now).
  * debian/autofs.lintian-overrides:
    + Ignore libautofs.so (share lib) for now.
    + Ignore lintian complain about autofs.8 man page.

The discussion about the, then upcoming, 5.1.7 release:
(https://www.spinics.net/lists/autofs/msg02341.html)

"""
I am going to include them in 5.1.7.

The the final rounds of testing are being done with the two patches
I mentioned.

The change is purely building my static library as a shared library,
nothing more, so it shouldn't impact existing code. In theory it
should be more economical from a RSS size and possibly WSS size too.
And being a change to the underlying autofs library it will be being
used during all the testing. I haven't seen any problems so far.

I'll announce the release and the 5.1.6 patches will be present on
kernel.org at that time, same as usual.

I'm not sure people will want to update straight away, there are
quite a few changes and a number of them aren't trivial, so people
may choose to select patches they need while waiting for the dust
to settle.
"""

Revision history for this message
Bryce Harrington (bryce) wrote :

Thanks Andreas. Indeed the release announcement (https://lwn.net/Articles/844100/) shows a fair bit of change; lots of bug fixes but also some refactoring, so even aside from the shared library change this is not a "pure bugfix" release. On the plus side, I see since 5.1.7 was released a month ago, there are no changes to the git tree, and no emails on upstream's mailing list regarding problems.

So, yeah I agree an FFe would be appropriate here. We're still early enough in the freeze that I don't see that as a showstopper, however I can see wanting to hold off on it to avoid taking any risks.

Revision history for this message
Bryce Harrington (bryce) wrote :

Utkarsh, the FFe process (at least, the required mechanics) is documented here:

  https://wiki.ubuntu.com/FreezeExceptionProcess

But, as Andreas alludes to, the spirit of the process is to ensure confidence that the update in question is safe to include. That involves making a technical decision as to the balance between the update's risk and reward, and thinking about if an issue does crop up will you be in a position to get it addressed quickly enough.

At the start it's a good idea to make a quick evaluation if you think the effort of getting to that level of confidence will be worth the time investment, particularly when the size of the upstream diff is large, as it is in this case. Look if there are bugs reported to ubuntu that would be fixed by this release. Look at upstream if there are post-release follow-on commits or discussions on their mailing list, that might indicate the release had problems. Review the release notes or changelog for any items that seem questionable (such as Andreas has flagged above), and for bug fixes that sound like they might be important for Ubuntu users. At the same time, take note if there are high-value fixes or features that users are going to really want. Consider also the nature of the software: if it's a central piece of IT infrastructure like a web server, or a library that lots of things depend on then it might pay more to be ultra-conservative than if it is developer tool.

Once you've taken all the above into account, decide if you feel it is worth proceeding with the FFe. If you do, then the next step would be to do a detailed analysis at least of the questions Andreas raised. If possible, while it's not strictly required by FFe, I like to do a detailed patch-by-patch review of all the changes. The FFe process does expect some rigorous testing, however, so for this you'd want to run any testsuites you can put your hands on, and set up a autofs installation that you can do some manual sanity checking and smoke testing on.

You can see examples of other FFe's currently in flight here, for reference:

  https://bugs.launchpad.net/~ubuntu-release/+subscribedbugs

Revision history for this message
Robie Basak (racb) wrote :

Is there a specific reason we need the latest autofs in Hirsute, apart from the version number being higher? If not, maybe we could focus on other bugs, and just defer this merge for the next cycle? We did merge autofs once this cycle already, so it's not like it's miles behind.

Revision history for this message
Utkarsh Gupta (utkarsh) wrote :

Hi Bryce and Robie,

I don't really find the need to get this version of autofs in Hirsute. And I agree that we should, indeed, defer this merge for the next cycle as this update might introduce some breaking change which could cause some unforeseeable problems and regressions.

The *only* reason to update and propose a MP here was because it included some bug fixes upstream, which would've been good to include in Hirsute but not at the cost of introducing other bugs and regression.

So let's not go via the FFe road but instead postpone this for the next cycle. Thanks for taking a look at this! :)

Revision history for this message
Bryce Harrington (bryce) wrote :

Hi Utkarsh,

I think that's a good decision in this case. The purpose of my previous post was for explaining the thinking process one would go through to reach that decision. Since you've joined right as FF started, the thought process seemed worth some explanation, particularly since you're going to be facing more of such decisions in the coming weeks with other packages.

What I would probably do in this case is rather than FFe the package, to focus more narrowly on the bugfixes themselves, and cherry-pick any that look likely to affect users. Debian flagged a couple of the upstream changes, so I'd start by examining these myself:

  * New upstream release.
    - autofs: Fix crash in sun_mount() (Closes: #892953).
    - Use PKG_CHECK_MODULES to detect the libxml2 library. (Closes: #949055).

The sun_mount crash is arch-specific so if we're not hitting it in our CI, that one isn't important. The libxml2 fix is going to be needed when libxml2 updates; maybe it's worth pulling proactively?

Since the upstream release includes some bug fixes, are there any worth cherrypicking? These kind of struck my eye:

- fix a regression with map instance lookup.
- fix quoted string length calc in expandsunent().
- mount_nfs.c fix local rdma share not mounting.
- fix ldap sasl reconnect problem.
- fix sss_master_map_wait timing.
- improve sss setautomntent() error handling.
- improve sss getautomntent() error handling.
- improve sss getautomntbyname() error handling.
- fix direct mount unlink_mount_tree() path.
- fix incorrect logical compare in unlink_mount_tree().
- fix remount expire.
- fix empty mounts list return from unlink_mount_tree().

Cherrypicking these fixes right now versus post-release is a bit easier since a release with only these changes could be released without FFe or SRU paperwork. The question would be if any of these are likely to actually affect end users. And in the case of autofs, as this is not a high profile package for the server team I'd probably be on the conservative side of what patches I'd bother with.

Revision history for this message
Utkarsh Gupta (utkarsh) wrote :

Hi Bryce,

Awesome, thanks for the explanation for the FFe part. Really appreciated!

As for cherrypicking fixes from upstream, et al, is concerned, we decided (or at least discussed) that it'd make sense to totally defer this to the next cycle. Since everybody seemed to agree on this, I'll take that road. So leaving this MP here, as is. I'll pick this up in the next cycle!

Thanks for your reviews and explanations! \o/

Revision history for this message
Utkarsh Gupta (utkarsh) wrote :

> [...] we decided [...] that it'd make sense to totally defer
> this to the next cycle. Since everybody seemed to agree on this,
> I'll take that road. So leaving this MP here, as is. I'll pick
> this up in the next cycle!

Now that it's "next" cycle time, I hereby re-propose this MP (now targeting impish). This can hopefully be a sync now, so requesting you to review and do a sync. TIA! \o/

Revision history for this message
Bryce Harrington (bryce) wrote :

Yep, LGTM.

Sponsored the sync.

Source autofs -> impish/Proposed: current version 5.1.6-4ubuntu1, new version 5.1.7-1
New changes:
autofs (5.1.7-1) unstable; urgency=medium

  * New upstream release.
    - autofs: Fix crash in sun_mount() (Closes: #892953).
    - Use PKG_CHECK_MODULES to detect the libxml2 library. (Closes: #949055).
  * debian/patches:
    + Drop fix-autofs-schema.patch and make-bind-mounts-propagation-slave-by-
      default.patch. Applied upstream.
    + Trivially rebase patches.
    + Add another typo fix to spelling-error-fixes.patch.
  * debian/autofs.install:
    + Install libautofs.so into bin:pkg autofs (for now).
  * debian/control:
    + Bump Standards-Version: to 4.5.1. No changes needed.
  * debian/autofs.lintian-overrides:
    + Ignore libautofs.so (share lib) for now.
    + Ignore lintian complain about autofs.8 man page.
  * debian/watch:
    + Update format version to 4.

 -- Mike Gabriel <email address hidden> Thu, 04 Feb 2021 13:36:20 +0100
Sponsoring this sync for Utkarsh Gupta (utkarsh)
Sync this package [y|N]? y
Request succeeded; you should get an e-mail once it is processed.
Launchpad bugs to be closed: 1917423
Please wait for the sync to be successful before closing bugs.
Close bugs [Y|n]? y
Closed bug https://bugs.launchpad.net/ubuntu/+source/autofs/+bug/1917423

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index f374146..441c765 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,14 @@
6+autofs (5.1.7-1ubuntu1) impish; urgency=medium
7+
8+ * Merge with Debian unstable (LP: #1917423).
9+ * Dropped Changes:
10+ - Allow administrative shares to work (LP 1680224):
11+ + d/p/lp1680224-fix-double-quoting-in-auto_smb.patch
12+ + d/p/lp1680224-fix-and-double-quoting-in-auto_smb.patch
13+ [Included in upstream 5.1.7-1]
14+
15+ -- Utkarsh Gupta <utkarsh.gupta@canonical.com> Wed, 19 May 2021 17:54:19 +0530
16+
17 autofs (5.1.7-1) unstable; urgency=medium
18
19 * New upstream release.
20@@ -20,6 +31,15 @@ autofs (5.1.7-1) unstable; urgency=medium
21
22 -- Mike Gabriel <sunweaver@debian.org> Thu, 04 Feb 2021 13:36:20 +0100
23
24+autofs (5.1.6-4ubuntu1) hirsute; urgency=medium
25+
26+ * Merge with Debian unstable. Remaining changes:
27+ - Allow administrative shares to work (LP 1680224):
28+ + d/p/lp1680224-fix-double-quoting-in-auto_smb.patch
29+ + d/p/lp1680224-fix-and-double-quoting-in-auto_smb.patch
30+
31+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Thu, 19 Nov 2020 16:05:17 +0100
32+
33 autofs (5.1.6-4) unstable; urgency=medium
34
35 * debian/patches:
36@@ -32,6 +52,15 @@ autofs (5.1.6-4) unstable; urgency=medium
37
38 -- Mike Gabriel <sunweaver@debian.org> Mon, 26 Oct 2020 21:46:29 +0100
39
40+autofs (5.1.6-3ubuntu1) groovy; urgency=medium
41+
42+ * Merge with Debian unstable. Remaining changes:
43+ - Allow administrative shares to work (LP #1680224):
44+ + d/p/lp1680224-fix-double-quoting-in-auto_smb.patch
45+ + d/p/lp1680224-fix-and-double-quoting-in-auto_smb.patch
46+
47+ -- Andreas Hasenack <andreas@canonical.com> Tue, 11 Aug 2020 17:57:17 -0300
48+
49 autofs (5.1.6-3) unstable; urgency=medium
50
51 * debian/patches:
52@@ -43,6 +72,15 @@ autofs (5.1.6-3) unstable; urgency=medium
53
54 -- Mike Gabriel <sunweaver@debian.org> Mon, 29 Jun 2020 13:21:47 +0200
55
56+autofs (5.1.6-2ubuntu1) groovy; urgency=medium
57+
58+ [ Andreas Hasenack ]
59+ * Allow administrative shares to work (LP: #1680224):
60+ - d/p/lp1680224-fix-double-quoting-in-auto_smb.patch
61+ - d/p/lp1680224-fix-and-double-quoting-in-auto_smb.patch
62+
63+ -- Rafael David Tinoco <rafaeldtinoco@ubuntu.com> Wed, 03 Jun 2020 00:30:58 +0000
64+
65 autofs (5.1.6-2) unstable; urgency=medium
66
67 [ Andreas Hasenack ]
68diff --git a/debian/control b/debian/control
69index 931261a..76b8613 100644
70--- a/debian/control
71+++ b/debian/control
72@@ -1,7 +1,8 @@
73 Source: autofs
74 Section: utils
75 Priority: optional
76-Maintainer: Mike Gabriel <sunweaver@debian.org>
77+Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
78+XSBC-Original-Maintainer: Mike Gabriel <sunweaver@debian.org>
79 Uploaders:
80 Debian Edu Packaging Team <debian-edu-pkg-team@lists.alioth.debian.org>,
81 Build-Depends:

Subscribers

People subscribed via source and target branches