Bug 169481 - [patch] sysutils/puppet fails to install packages
Summary: [patch] sysutils/puppet fails to install packages
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Steve Wills
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-27 10:40 UTC by Romain Tartière
Modified: 2012-09-06 07:20 UTC (History)
1 user (show)

See Also:


Attachments
file.diff (1.04 KB, patch)
2012-06-27 10:40 UTC, Romain Tartière
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Romain Tartière freebsd_committer freebsd_triage 2012-06-27 10:40:08 UTC
When using puppet with the default options, trying to install a package with a
manifest including for example the following lines will fail:

------ [beginning of manifest] ------
  [...]
  package { "sysutils/tmux":
    ensure => installed,
  }
  [...]
--------- [end of manifest] ---------


------ [beginning of failure details] ------
# puppet agent --test --debug
[...]
debug: Prefetching freebsd resources for package
debug: Puppet::Type::Package::ProviderFreebsd: Executing '/usr/sbin/pkg_info -aoQ'
debug: Puppet::Type::Package::ProviderFreebsd: Executing '/usr/sbin/pkg_info -aoQ'
debug: Package: sysutils/tmux: origin => {:port_category=>"sysutils", :port_name=>"tmux"}
debug: Package: sysutils/tmux: source => #<URI::HTTP:0x805cf55f0 URL:http://packages.blogreen.org/>
debug: Fetching INDEX: #<URI::HTTP:0x805cf4290 URL:http://packages.blogreen.org/INDEX.bz2>
err: /Stage[main]/XXXX/Package[sysutils/tmux]/ensure: change from absent to present failed: Could not set 'present on ensure: can't convert nil into String at /usr/local/etc/puppet/manifests/site.pp:27
--------- [end of failure details] ---------

The file /usr/local/lib/ruby/site_ruby/1.8/puppet/provider/package/freebsd.rb
is patched from the origninal one by the port, but the error is tied into it.
Relevant code:

------ [beginning of failing code] ------
 77         Puppet.debug "Fetching INDEX: #{uri.inspect}"
 78         begin
 79           open(uri, "r") do |f|
 80             Bzip2::Reader.open(f.path) do |f|
 81               while (line = f.gets)
 82                 fields = line.split("|")
 83                 pkg_info = self.class.parse_pkg_string(fields[0])
 84                 origin = self.class.parse_origin(fields[1])
 85                 @@ports_index[origin] = pkg_info
 86               end
 87             end
 88           end
 89         rescue IOError, OpenURI::HTTPError, Net::FTPError
 90           @@ports_index = nil
 91           raise Puppet::Error.new "Could not fetch ports INDEX: #{$!}"
 92         end
--------- [end of failing code] ---------

From IRB, the object provided by open at line 79 has no #path::

------ [beginning of failing code] ------
irb(main):001:0> require 'open-uri'
=> true
irb(main):002:0> open("http://packages.blogreen.org/INDEX-9.bz2", "r") do |f|
irb(main):003:1* f.path
irb(main):004:1> end
=> nil
--------- [end of failing code] ---------

Maybe this is a regression or something...  AFAICR, this code used to work a
while ago.

Fix: Bzip2::Reader.open seems to accept URIs, so we can just apply this change:

------ [beginning of fix] ------
How-To-Repeat: 
Try to install a package.
Comment 1 Edwin Groothuis freebsd_committer freebsd_triage 2012-06-27 10:40:17 UTC
Responsible Changed
From-To: freebsd-ports-bugs->swills

Over to maintainer (via the GNATS Auto Assign Tool)
Comment 2 Romain Tartière freebsd_committer freebsd_triage 2012-06-27 13:40:51 UTC
Ooops!  Wrong patch attached!

The one generated by diff follows:

--- files/optpatch-package_origin.orig  2012-06-27 14:32:02.673317242 +0200
+++ files/optpatch-package_origin       2012-06-27 14:32:23.221319587 +0200
@@ -1,6 +1,9 @@
---- lib/puppet/provider/package/freebsd.rb.orig        Thu Nov  3 10:58:56 2011
-+++ lib/puppet/provider/package/freebsd.rb     Thu Nov  3 10:59:02 2011
-@@ -1,37 +1,165 @@
+
+$FreeBSD$
+
+--- lib/puppet/provider/package/freebsd.rb.orig
++++ lib/puppet/provider/package/freebsd.rb
+@@ -1,37 +1,163 @@
 -Puppet::Type.type(:package).provide :freebsd, :parent => :openbsd do
 -  desc "The specific form of package management on FreeBSD.  This is an
 -    extremely quirky packaging system, in that it freely mixes between
@@ -16,14 +19,14 @@
 -    :pkgdelete => "/usr/sbin/pkg_delete"
 +Puppet::Type.type(:package).provide :freebsd, :parent => Puppet::Provider::Package do
 +  include Puppet::Util::Execution
- 
++
 +  desc "The specific form of package management on FreeBSD. Resource names must be
 +  specified as the port origin: <port_category>/<port_name>."
 +
 +  commands :pkginfo    => "/usr/sbin/pkg_info",
 +           :pkgadd     => "/usr/sbin/pkg_add",
 +           :pkgdelete  => "/usr/sbin/pkg_delete"
-+
+ 
    confine :operatingsystem => :freebsd
 +  defaultfor :operatingsystem => :freebsd
  
@@ -96,14 +99,12 @@
 +        uri = source.merge "INDEX.bz2"
 +        Puppet.debug "Fetching INDEX: #{uri.inspect}"
 +        begin
-+          open(uri, "r") do |f|
-+            Bzip2::Reader.open(f.path) do |f|
-+              while (line = f.gets)
-+                fields = line.split("|")
-+                pkg_info = self.class.parse_pkg_string(fields[0])
-+                origin = self.class.parse_origin(fields[1])
-+                @@ports_index[origin] = pkg_info
-+              end
++          Bzip2::Reader.open(uri) do |f|
++            while (line = f.gets)
++              fields = line.split("|")
++              pkg_info = self.class.parse_pkg_string(fields[0])
++              origin = self.class.parse_origin(fields[1])
++              @@ports_index[origin] = pkg_info
 +            end
 +          end
 +        rescue IOError, OpenURI::HTTPError, Net::FTPError
@@ -188,7 +189,7 @@
    end
  
    def query
-@@ -44,6 +172,7 @@
+@@ -44,6 +170,7 @@
    end
  
    def uninstall
Comment 3 Romain Tartière 2012-06-29 11:58:03 UTC
Hello again

I got the old (working) configuration and it still works with the
original code :-/

From IRB, I can see a difference when opening the working and non
working URIs:

irb(main):005:0> f = open("http://gnome3-packages.blogreen.org/INDEX-9.bz2")
=> #<File:/tmp/open-uri20120629-16697-f36den-0>
irb(main):006:0> f = open("http://packages.blogreen.org/INDEX-9.bz2")
=> #<StringIO:0x8036140a8>

Open does not return the same type of object for both URIs, both being
in theory the same kind of file: BZip2 data.

The server headers do not differ:

Working URI:
romain@marvin ~ % h http://gnome3-packages.blogreen.org/INDEX-9.bz2       12:47
HTTP/1.1 200 OK
Date: Fri, 29 Jun 2012 10:47:08 GMT
Server: Apache
Last-Modified: Fri, 29 Jun 2012 09:36:33 GMT
ETag: "19bd1e-507b-4c3992f034a40"
Accept-Ranges: bytes
Content-Length: 20603
Content-Type: application/x-bzip2

Failing URI:
romain@marvin ~ % h "http://packages.blogreen.org/INDEX.bz2"              12:46
HTTP/1.1 200 OK
Date: Fri, 29 Jun 2012 10:46:48 GMT
Server: Apache
Last-Modified: Fri, 22 Jun 2012 14:48:42 GMT
ETag: "b2357-d24-4c310ba758680"
Accept-Ranges: bytes
Content-Length: 3364
Content-Type: application/x-bzip2

So the problem seems to be deeper in Ruby.  The proposed 'fix' then
becomes a 'workaround', and works with both bz2 files.

-- 
Romain Tartière <romain@blogreen.org>        http://romain.blogreen.org/
pgp: 8234 9A78 E7C0 B807 0B59  80FF BA4D 1D95 5112 336F (ID: 0x5112336F)
(plain text =non-HTML= PGP/GPG encrypted/signed e-mail much appreciated)
Comment 4 Steve Wills freebsd_committer freebsd_triage 2012-07-25 02:56:23 UTC
Hi,

Did you ever find the source of this problem? I'm not sure the
workaround is appropriate to commit unless we can find the source of the
problem.

Steve
Comment 5 Steve Wills freebsd_committer freebsd_triage 2012-07-25 03:07:07 UTC
State Changed
From-To: open->feedback

- Waiting on submitter feedback
Comment 6 Romain Tartière freebsd_committer freebsd_triage 2012-07-25 05:58:49 UTC
A quick look at open-uri.rb and searching for StringIO leads to this
portion of code:

366   class Buffer # :nodoc:
367     def initialize
368       @io = StringIO.new
369       @size = 0
370     end
371     attr_reader :size
372 
373     StringMax = 10240
        ^^^^^^^^^^^^^^^^^
374     def <<(str)
375       @io << str
376       @size += str.length
377       if StringIO === @io && StringMax < @size
                                 ^^^^^^^^^^^^^^^^^
378         require 'tempfile'
379         io = Tempfile.new('open-uri')
                 ^^^^^^^^^^^^^^^^^^^^^^^^
380         io.binmode
381         Meta.init io, @io if Meta === @io
382         io << @io.string
383         @io = io
384       end
385     end

According to the preceding messages, the working URI file size is 20603,
the failing one is 3364.  The examples in the documentation of open-uri
never use the #path of the object returned by #open.  We can't assume
that open will return a (temporary) File and not a simple StringIO.

I can't tell if it some kind of bug that should be fixed in Ruby (I
guess that this code logic is here for some reason, so I would tell it's
not), but as-it, the usage of open-uri here is ``non-safe'' and should
be changed to cope with files smaller than 10k.

Regards,
Romain
Comment 7 Romain Tartière freebsd_committer freebsd_triage 2012-07-25 06:44:57 UTC
State Changed
From-To: feedback->open

Feedback provided.
Comment 8 dfilter service freebsd_committer freebsd_triage 2012-09-06 07:15:43 UTC
Author: swills
Date: Thu Sep  6 06:15:28 2012
New Revision: 303746
URL: http://svn.freebsd.org/changeset/ports/303746

Log:
  - Fix issue with installing packages [1]
  - Fix shell config issue [2]
  
  PR:		ports/169481 [1]
  PR:		ports/171188 [2]
  Submitted by:	romain [1]
  Submitted by:	Christopher McCoy <syseng@wayfair.com> [2]

Added:
  head/sysutils/puppet/files/patch-exec.rb   (contents, props changed)
Modified:
  head/sysutils/puppet/Makefile   (contents, props changed)
  head/sysutils/puppet/files/optpatch-package_origin   (contents, props changed)

Modified: head/sysutils/puppet/Makefile
==============================================================================
--- head/sysutils/puppet/Makefile	Thu Sep  6 06:12:30 2012	(r303745)
+++ head/sysutils/puppet/Makefile	Thu Sep  6 06:15:28 2012	(r303746)
@@ -7,6 +7,7 @@
 
 PORTNAME=	puppet
 PORTVERSION=	2.7.19
+PORTREVISION=	1
 CATEGORIES=	sysutils
 MASTER_SITES=	http://downloads.puppetlabs.com/puppet/
 

Modified: head/sysutils/puppet/files/optpatch-package_origin
==============================================================================
--- head/sysutils/puppet/files/optpatch-package_origin	Thu Sep  6 06:12:30 2012	(r303745)
+++ head/sysutils/puppet/files/optpatch-package_origin	Thu Sep  6 06:15:28 2012	(r303746)
@@ -1,6 +1,9 @@
---- lib/puppet/provider/package/freebsd.rb.orig	Thu Nov  3 10:58:56 2011
-+++ lib/puppet/provider/package/freebsd.rb	Thu Nov  3 10:59:02 2011
-@@ -1,37 +1,165 @@
+
+$FreeBSD$
+
+--- lib/puppet/provider/package/freebsd.rb.orig
++++ lib/puppet/provider/package/freebsd.rb
+@@ -1,37 +1,163 @@
 -Puppet::Type.type(:package).provide :freebsd, :parent => :openbsd do
 -  desc "The specific form of package management on FreeBSD.  This is an
 -    extremely quirky packaging system, in that it freely mixes between
@@ -16,14 +19,14 @@
 -    :pkgdelete => "/usr/sbin/pkg_delete"
 +Puppet::Type.type(:package).provide :freebsd, :parent => Puppet::Provider::Package do
 +  include Puppet::Util::Execution
- 
++
 +  desc "The specific form of package management on FreeBSD. Resource names must be
 +  specified as the port origin: <port_category>/<port_name>."
 +
 +  commands :pkginfo    => "/usr/sbin/pkg_info",
 +           :pkgadd     => "/usr/sbin/pkg_add",
 +           :pkgdelete  => "/usr/sbin/pkg_delete"
-+
+ 
    confine :operatingsystem => :freebsd
 +  defaultfor :operatingsystem => :freebsd
  
@@ -96,14 +99,12 @@
 +        uri = source.merge "INDEX.bz2"
 +        Puppet.debug "Fetching INDEX: #{uri.inspect}"
 +        begin
-+          open(uri, "r") do |f|
-+            Bzip2::Reader.open(f.path) do |f|
-+              while (line = f.gets)
-+                fields = line.split("|")
-+                pkg_info = self.class.parse_pkg_string(fields[0])
-+                origin = self.class.parse_origin(fields[1])
-+                @@ports_index[origin] = pkg_info
-+              end
++          Bzip2::Reader.open(uri) do |f|
++            while (line = f.gets)
++              fields = line.split("|")
++              pkg_info = self.class.parse_pkg_string(fields[0])
++              origin = self.class.parse_origin(fields[1])
++              @@ports_index[origin] = pkg_info
 +            end
 +          end
 +        rescue IOError, OpenURI::HTTPError, Net::FTPError
@@ -188,7 +189,7 @@
    end
  
    def query
-@@ -44,6 +172,7 @@
+@@ -44,6 +170,7 @@
    end
  
    def uninstall

Added: head/sysutils/puppet/files/patch-exec.rb
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/sysutils/puppet/files/patch-exec.rb	Thu Sep  6 06:15:28 2012	(r303746)
@@ -0,0 +1,17 @@
+--- lib/puppet/provider/exec.rb.orig    2012-08-21 17:41:17.000000000 -0400
++++ lib/puppet/provider/exec.rb 2012-08-30 12:31:32.000000000 -0400
+@@ -66,11 +66,9 @@
+   end
+
+   def extractexe(command)
+-    if command.is_a? Array
+-      command.first
+-    elsif match = /^"([^"]+)"|^'([^']+)'/.match(command)
+-      # extract whichever of the two sides matched the content.
+-      match[1] or match[2]
++    # easy case: command was quoted
++    if command =~ /^"([^"]+)"/
++      $1
+     else
+       command.split(/ /)[0]
+     end
_______________________________________________
svn-ports-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-ports-all
To unsubscribe, send any mail to "svn-ports-all-unsubscribe@freebsd.org"
Comment 9 Steve Wills freebsd_committer freebsd_triage 2012-09-06 07:16:10 UTC
State Changed
From-To: open->closed

Ok, makes sense. Change committed. Thanks!