Merge ~lucaskanashiro/ubuntu/+source/puppet:fix-links-issue into ubuntu/+source/puppet:ubuntu/devel

Proposed by Lucas Kanashiro
Status: Needs review
Proposed branch: ~lucaskanashiro/ubuntu/+source/puppet:fix-links-issue
Merge into: ubuntu/+source/puppet:ubuntu/devel
Diff against target: 60 lines (+38/-0)
3 files modified
debian/changelog (+6/-0)
debian/patches/0019-Set-file-links-attribute-to-manage-by-default.patch (+31/-0)
debian/patches/series (+1/-0)
Reviewer Review Type Date Requested Status
Athos Ribeiro (community) Approve
Canonical Server MOTU reviewers Pending
git-ubuntu import Pending
Review via email: mp+424107@code.launchpad.net

Description of the change

Fixes file's link attribute with ruby 3.0. This patch is not present in the upstream source tree nor in Debian, upstream does support ruby 3.0 properly but in the newer version (7.x) and Debian maintainers are still trying to move to that version (not willing to support 5.x). However, we are stuck to 5.x in Jammy and I believe kinetic will be released with the same version, so we need to make it work with ruby 3.0.

The patch I am proposing here is a workaround to this bug and it was tested by some users in LP #1969939. They do not find any issue with that but since this is a handcrafted patch there is always the risk to introduce unexpected behavior.

PPA with the proposed package:

https://launchpad.net/~lucaskanashiro/+archive/ubuntu/puppet/

autopkgtest summary:

autopkgtest [17:08:06]: @@@@@@@@@@@@@@@@@@@@ summary
command1 PASS
command2 PASS
command3 PASS
command4 PASS
command5 PASS

To post a comment you must log in.
Revision history for this message
Athos Ribeiro (athos-ribeiro) wrote :

Thanks, Lucas!

I looked for a change fixing the issue upstream. Then I realized this fixes a regression introduced by debian/patches/0017-PUP-10537-keyword-param-as-last-arg.patch. This patch was backported from upstream's 6.x and the APIs changed a lot since then.

I also screened the code for any bits that could possibly rely on the value of `links` not being set, and found no instances.

LGTM

review: Approve
Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote :

Thanks for the review Athos. Package uploaded:

Uploading puppet_5.5.22-4ubuntu2.dsc
Uploading puppet_5.5.22.orig.tar.gz
Uploading puppet_5.5.22.orig.tar.gz.asc
Uploading puppet_5.5.22-4ubuntu2.debian.tar.xz
Uploading puppet_5.5.22-4ubuntu2_source.buildinfo
Uploading puppet_5.5.22-4ubuntu2_source.changes

Revision history for this message
Lucas Kanashiro (lucaskanashiro) wrote :

I'll be preparing a MP with the same patch targeting Jammy.

Unmerged commits

c25202f... by Lucas Kanashiro

Update changelog

b35d7a0... by Lucas Kanashiro

Add patch to set file's link attribute to manage by default (LP: #1969939)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index 9fd37cc..01cd764 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
1puppet (5.5.22-4ubuntu2) kinetic; urgency=medium
2
3 * Add patch to set file's link attribute to manage by default (LP: #1969939)
4
5 -- Lucas Kanashiro <kanashiro@ubuntu.com> Tue, 31 May 2022 11:36:34 -0300
6
1puppet (5.5.22-4ubuntu1) kinetic; urgency=medium7puppet (5.5.22-4ubuntu1) kinetic; urgency=medium
28
3 * Backport upstream patches (LP: #1969939):9 * Backport upstream patches (LP: #1969939):
diff --git a/debian/patches/0019-Set-file-links-attribute-to-manage-by-default.patch b/debian/patches/0019-Set-file-links-attribute-to-manage-by-default.patch
4new file mode 10064410new file mode 100644
index 0000000..ddf8161
--- /dev/null
+++ b/debian/patches/0019-Set-file-links-attribute-to-manage-by-default.patch
@@ -0,0 +1,31 @@
1From: Lucas Kanashiro <lucas.kanashiro@canonical.com>
2Date: Tue, 31 May 2022 11:32:57 -0300
3Subject: Set file links attribute to manage by default
4
5This patch is not applied in the upstream source tree, this is a
6workaround written by me to attempt to fix the issue reported by users.
7
8Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/puppet/+bug/1969939
9---
10 lib/puppet/file_serving/metadata.rb | 8 +++++++-
11 1 file changed, 7 insertions(+), 1 deletion(-)
12
13diff --git a/lib/puppet/file_serving/metadata.rb b/lib/puppet/file_serving/metadata.rb
14index 08e2ca5..ec4d09d 100644
15--- a/lib/puppet/file_serving/metadata.rb
16+++ b/lib/puppet/file_serving/metadata.rb
17@@ -136,7 +136,13 @@ class Puppet::FileServing::Metadata < Puppet::FileServing::Base
18 @destination = data.delete('destination')
19 @source = data.delete('source')
20 @content_uri = data.delete('content_uri')
21- links = data.fetch('links', nil) || data.fetch(:links, nil)
22+ # This is a workaround to fix the issue Ubuntu users reported in LP #1969939.
23+ # Once links variable is nil we get a "undefined method `to_sym' for nil:NilClass"
24+ # in the links= method in file_serving/base.rb.
25+ # According to the upstream doc
26+ # (https://puppet.com/docs/puppet/5.5/types/file.html#file-attribute-links),
27+ # the default value is 'manage', therefore, setting the default to :manage.
28+ links = data.fetch('links', :manage) || data.fetch(:links, :manage)
29 relative_path = data.fetch('relative_path', nil) || data.fetch(:relative_path, nil)
30 source = @source || data.fetch(:source, nil)
31 super(path, links: links, relative_path: relative_path, source: source)
diff --git a/debian/patches/series b/debian/patches/series
index ff870c2..24e0778 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -16,3 +16,4 @@
160016-PUP-11046-Implement-RFC2396-style-escape-unescape.patch160016-PUP-11046-Implement-RFC2396-style-escape-unescape.patch
170017-PUP-10537-keyword-param-as-last-arg.patch170017-PUP-10537-keyword-param-as-last-arg.patch
180018-PUP-11045-ruby-openssl-now-sets-store_context.error.patch180018-PUP-11045-ruby-openssl-now-sets-store_context.error.patch
190019-Set-file-links-attribute-to-manage-by-default.patch

Subscribers

People subscribed via source and target branches