Hi,
On Tuesday, April 09, 2024 22:43 CEST, Giovanni Bechis <giova...@paclan.it> 
wrote:

> On Tue, Apr 09, 2024 at 10:17:30PM +0200, Sebastian Reitenbach wrote:
> > Since we now have a recent Puppet in ports, I started looking at how 
> > packages are handled with Puppet.
> > My current trouble is that it wasn't really possible to install banches 
> > properly: i.e. can't properly install gimp, 
> > or auto* based on branch. Or esp, when want to install multiple of them, it 
> > was just not possible.
> > For ports where branches conflict, i.e. postfix, this was working, but had 
> > to specify exact version, and on every
> > upgrade bump the version .... _very_ annoying.
> > 
> > Currently Puppet allows to install packages of a given version (ensure => 
> > "X.Y.Z"), or to follow updates (ensure => "latest").
> > 
> [...]
> > This is just for Puppet 8. Anyone still on Puppet 7? It should be easily 
> > ported to Puppet 7 as well. 
> > 
> I am on Puppet 7 and I do not have time to upgrade to Puppet 8 soon; I

My upgrade from Puppet 7 to 8 took quite a long time. They removed a lot of 
backward compat
shim, and if you have many old and partially hand made or patched modules like 
I do, it took
quite a while to update all of them....

Attached an untested version of the same changes for the Puppet 7 port.

> am also using "ensure => latest" syntax but I can change my code to get
> rid of it OpenBSD will no more support it.

What's your use-case to using "ensure => latest"? If there's really a "good 
one",
I can look into re-adding the feature. 
My use-case up to now for sparingly using "latest" was to force using 
"snapshot" 
branch of some packages.

> 
>  Cheers
>   Giovanni
> 
> 
> > cheers,
> > Sebastian
> > 
> > 
> > Index: Makefile
> > ===================================================================
> > RCS file: /cvs/ports/sysutils/ruby-puppet/8/Makefile,v
> > diff -u -r1.2 Makefile
> > --- Makefile        20 Mar 2024 21:21:14 -0000      1.2
> > +++ Makefile        9 Apr 2024 19:51:40 -0000
> > @@ -1,6 +1,7 @@
> >  PORTROACH=         limit:^7
> >  
> >  VERSION=           8.5.1
> > +REVISION=          0
> >  
> >  RUN_DEPENDS+=              
> > converters/ruby-multi_json,${MODRUBY_FLAVOR}>=1.13,<2 \
> >                     devel/ruby-concurrent-ruby,${MODRUBY_FLAVOR}>=1,<2 \
> > Index: patches/patch-lib_puppet_provider_package_openbsd_rb
> > ===================================================================
> > RCS file: 
> > /cvs/ports/sysutils/ruby-puppet/8/patches/patch-lib_puppet_provider_package_openbsd_rb,v
> > diff -u -r1.2 patch-lib_puppet_provider_package_openbsd_rb
> > --- patches/patch-lib_puppet_provider_package_openbsd_rb    20 Mar 2024 
> > 21:21:14 -0000      1.2
> > +++ patches/patch-lib_puppet_provider_package_openbsd_rb    9 Apr 2024 
> > 19:51:40 -0000
> > @@ -1,42 +1,89 @@
> > -- Handle errors from pkg_add
> > -- Handle uninstall_options being 'nil' by default
> > -- If no flavor speficied, force the empty flavor with '--'
> > -  but skipping the % un-ambiguity pkg names
> > -- Bail out on shortform PKG_PATH (i.e. 'ftp.openbsd.org')
> > -- pkg.conf is gone
> > -- properly handle packages with multiple versions and flavors,
> > -  i.e. postfix-XXX-flavor
> > -
> > +- get rid of versionable (no ensure => "version X.X.X")
> > +- get rid of upgradeable (ensure => latest)
> > +- properly support branches
> >  
> >  Index: lib/puppet/provider/package/openbsd.rb
> >  --- lib/puppet/provider/package/openbsd.rb.orig
> >  +++ lib/puppet/provider/package/openbsd.rb
> > -@@ -24,6 +24,8 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
> > -   has_feature :upgradeable
> > +@@ -6,10 +6,14 @@ require_relative '../../../puppet/provider/package'
> > + Puppet::Type.type(:package).provide :openbsd, :parent => 
> > Puppet::Provider::Package do
> > +   desc "OpenBSD's form of `pkg_add` support.
> > + 
> > ++    OpenBSD has the concept of package branches, providing multiple 
> > versions of the
> > ++    same package, i.e. `stable` vs. `snapshot`. To select a specific 
> > branch,
> > ++    suffix the package name with % sign follwed by the branch name, i.e. 
> > `gimp%stable`.
> > ++
> > +     This provider supports the `install_options` and `uninstall_options`
> > +     attributes, which allow command-line flags to be passed to pkg_add 
> > and pkg_delete.
> > +     These options should be specified as an array where each element is 
> > either a
> > +-     string or a hash."
> > ++    string or a hash."
> > + 
> > +   commands :pkginfo => "pkg_info",
> > +            :pkgadd => "pkg_add",
> > +@@ -18,220 +22,94 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
> > +   defaultfor 'os.name' => :openbsd
> > +   confine 'os.name' => :openbsd
> > + 
> > +-  has_feature :versionable
> > +   has_feature :install_options
> > +   has_feature :uninstall_options
> > +-  has_feature :upgradeable
> >     has_feature :supports_flavors
> >   
> > -+  mk_resource_methods
> > -+
> >     def self.instances
> > -     packages = []
> > - 
> > -@@ -46,12 +48,6 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
> > - 
> > -             packages << new(hash)
> > -             hash = {}
> > +-    packages = []
> > +-
> > ++    final = []
> > +     begin
> > +-      execpipe(listcmd) do |process|
> > +-        # our regex for matching pkg_info output
> > +-        regex = /^(.*)-(\d[^-]*)[-]?([\w-]*)(.*)$/
> > +-        fields = [:name, :ensure, :flavor]
> > +-        hash = {}
> > ++      packages = listcmd
> > ++      packages.each { |package, value|
> > ++        if !package.empty?()
> > ++          value[:provider] = self.name
> > ++          final << new(value)
> > ++        end
> > ++      }
> > ++      return final
> > + 
> > +-        # now turn each returned line into a package object
> > +-        process.each_line { |line|
> > +-          match = regex.match(line.split[0])
> > +-          if match
> > +-            fields.zip(match.captures) { |field, value|
> > +-              hash[field] = value
> > +-            }
> > +-
> > +-            hash[:provider] = self.name
> > +-
> > +-            packages << new(hash)
> > +-            hash = {}
> >  -          else
> >  -            unless line =~ /Updating the pkgdb/
> >  -              # Print a warning on lines we can't match, but move
> >  -              # on, since it should be non-fatal
> >  -              warning(_("Failed to match line %{line}") % { line: line })
> >  -            end
> > -           end
> > -         }
> > -       end
> > -@@ -67,26 +63,17 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
> > +-          end
> > +-        }
> > +-      end
> > +-
> > +-      return packages
> > +     rescue Puppet::ExecutionFailure
> > +-      return nil
> > ++      nil
> > +     end
> >     end
> >   
> > -   def latest
> > +   def self.listcmd
> > +-    [command(:pkginfo), "-a"]
> > +-  end
> > +-
> > +-  def latest
> >  -    parse_pkgconf
> >  -
> >  -    if @resource[:source][-1, 1] == ::File::SEPARATOR
> > @@ -45,55 +92,51 @@
> >  -      e_vars = {}
> >  -    end
> >  -
> > -     if @resource[:flavor]
> > -       query = "#{@resource[:name]}--#{@resource[:flavor]}"
> > -     else
> > +-    if @resource[:flavor]
> > +-      query = "#{@resource[:name]}--#{@resource[:flavor]}"
> > +-    else
> >  -      query = @resource[:name]
> > -+      query = @resource[:name] + "--"
> > -     end
> > - 
> > +-    end
> > +-
> >  -    output = Puppet::Util.withenv(e_vars) { pkginfo "-Q", query }
> >  -    version = properties[:ensure]
> > -+    output = Puppet::Util.withenv({}) {pkginfo "-Q", query}
> > - 
> > -     if output.nil? or output.size == 0 or output =~ /Error from /
> > -       debug "Failed to query for #{resource[:name]}"
> > +-
> > +-    if output.nil? or output.size == 0 or output =~ /Error from /
> > +-      debug "Failed to query for #{resource[:name]}"
> >  -      return version
> > -+      return properties[:ensure]
> > -     else
> > -       # Remove all fuzzy matches first.
> > -       output = output.split.select { |p| p =~ 
> > /^#{resource[:name]}-(\d[^-]*)[-]?(\w*)/ }.join
> > -@@ -95,21 +82,22 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
> > - 
> > -     if output =~ /^#{resource[:name]}-(\d[^-]*)[-]?(\w*) \(installed\)$/
> > -       debug "Package is already the latest available"
> > +-    else
> > +-      # Remove all fuzzy matches first.
> > +-      output = output.split.select { |p| p =~ 
> > /^#{resource[:name]}-(\d[^-]*)[-]?(\w*)/ }.join
> > +-      debug "pkg_info -Q for #{resource[:name]}: #{output}"
> > +-    end
> > +-
> > +-    if output =~ /^#{resource[:name]}-(\d[^-]*)[-]?(\w*) \(installed\)$/
> > +-      debug "Package is already the latest available"
> >  -      return version
> > -+      return properties[:ensure]
> > -     else
> > -       match = /^(.*)-(\d[^-]*)[-]?(\w*)$/.match(output)
> > -       debug "Latest available for #{resource[:name]}: #{match[2]}"
> > - 
> > +-    else
> > +-      match = /^(.*)-(\d[^-]*)[-]?(\w*)$/.match(output)
> > +-      debug "Latest available for #{resource[:name]}: #{match[2]}"
> > +-
> >  -      if version.to_sym == :absent || version.to_sym == :purged
> > -+      if properties[:ensure].to_sym == :absent
> > -         return match[2]
> > -       end
> > - 
> > -       vcmp = version.split('.').map { |s| s.to_i } <=> 
> > match[2].split('.').map { |s| s.to_i }
> > -+      vcmp = properties[:ensure].split('.').map{|s|s.to_i} <=> 
> > match[2].split('.').map{|s|s.to_i}
> > -       if vcmp > 0
> > -         # The locally installed package may actually be newer than what a 
> > mirror
> > -         # has. Log it at debug, but ignore it otherwise.
> > +-        return match[2]
> > +-      end
> > +-
> > +-      vcmp = version.split('.').map { |s| s.to_i } <=> 
> > match[2].split('.').map { |s| s.to_i }
> > +-      if vcmp > 0
> > +-        # The locally installed package may actually be newer than what a 
> > mirror
> > +-        # has. Log it at debug, but ignore it otherwise.
> >  -        debug "Package #{resource[:name]} #{version} newer then available 
> > #{match[2]}"
> >  -        return version
> > -+        debug "Package #{resource[:name]} #{properties[:ensure]} newer 
> > then available #{match[2]}"
> > -+        return properties[:ensure]
> > -       else
> > -         return match[2]
> > -       end
> > -@@ -120,57 +108,25 @@ Puppet::Type.type(:package).provide :openbsd, 
> > :parent 
> > -     self.install(true)
> > -   end
> > - 
> > +-      else
> > +-        return match[2]
> > +-      end
> > +-    end
> > +-  end
> > +-
> > +-  def update
> > +-    self.install(true)
> > +-  end
> > +-
> >  -  def parse_pkgconf
> >  -    unless @resource[:source]
> >  -      if Puppet::FileSystem.exist?("/etc/pkg.conf")
> > @@ -111,7 +154,20 @@
> >  -              end
> >  -            end
> >  -          end
> > --        end
> > ++    regex_fuzzy = /^(.*)--([\w-]+)?(%[^w]+)?$/
> > ++    f = []
> > ++    f = pkginfo("-a", "-z").split("\n")
> > ++    packages = {}
> > ++    f.each do |line|
> > ++        match = regex_fuzzy.match(line.split[0])
> > ++        name = match.captures[0]
> > ++        flavor = match.captures[1]
> > ++        branch = match.captures[2]
> > ++        if branch.nil?
> > ++                  pname = name
> > ++        else
> > ++                  pname = name + branch
> > +         end
> >  -
> >  -        unless @resource[:source]
> >  -          raise Puppet::Error,
> > @@ -121,14 +177,18 @@
> >  -        raise Puppet::Error,
> >  -              _("You must specify a package source or configure an 
> > installpath in /etc/pkg.conf")
> >  -      end
> > --    end
> > --  end
> > --
> > -   def install(latest = false)
> > ++        packages[pname] = { :name => pname, :flavor => flavor, :branch => 
> > branch, :ensure => "present" }
> > +     end
> > ++    packages
> > +   end
> > + 
> > +-  def install(latest = false)
> > ++  def install
> >       cmd = []
> >   
> >  -    parse_pkgconf
> > --
> > ++    full_name = get_full_name(action="install")
> > + 
> >  -    if @resource[:source][-1, 1] == ::File::SEPARATOR
> >  -      e_vars = { 'PKG_PATH' => @resource[:source] }
> >  -      full_name = get_full_name(latest)
> > @@ -139,27 +199,24 @@
> >  -
> >  +    cmd << '-r'
> >       cmd << install_options
> > --    cmd << full_name
> > -+    cmd << get_full_name(latest)
> > +     cmd << full_name
> >   
> > -     if latest
> > +-    if latest
> >  -      cmd.unshift('-rz')
> > -+      cmd.unshift('-z')
> > -     end
> > - 
> > --    Puppet::Util.withenv(e_vars) { pkgadd cmd.flatten.compact }
> >  +    # pkg_add(1) doesn't set the return value upon failure so we have to 
> > peek
> >  +    # at it's output to see if something went wrong.
> >  +    output = Puppet::Util.withenv({}) { pkgadd cmd.flatten.compact }
> > -+    require 'pp'
> > -+    pp output
> >  +    if output =~ /Can't find /
> >  +      self.fail "pkg_add returned: #{output.chomp}"
> > -+    end
> > +     end
> > +-
> > +-    Puppet::Util.withenv(e_vars) { pkgadd cmd.flatten.compact }
> >     end
> >   
> > -   def get_full_name(latest = false)
> > -@@ -179,11 +135,20 @@ Puppet::Type.type(:package).provide :openbsd, 
> > :parent 
> > +-  def get_full_name(latest = false)
> > ++  def get_full_name(action="install")
> > +     # In case of a real update (i.e., the package already exists) then
> > +     # pkg_add(8) can handle the flavors. However, if we're actually
> >       # installing with 'latest', we do need to handle the flavors. This is
> >       # done so we can feed pkg_add(8) the full package name to install to
> >       # prevent ambiguity.
> > @@ -168,53 +225,35 @@
> >  -    elsif latest
> >  -      # Don't depend on get_version for updates.
> >  -      @resource[:name]
> > -+    if resource[:flavor]
> > -+      # If :ensure contains a version, use that instead of looking it up.
> > -+      # This allows for installing packages with the same stem, but 
> > multiple
> > -+      # version such as postfix-VERSION-flavor.
> > -+      if @resource[:ensure].to_s =~ /(\d[^-]*)$/
> > -+        use_version = @resource[:ensure]
> > -+      else
> > -+        use_version = ''
> > -+      end      
> > -+      "#{resource[:name]}-#{use_version}-#{resource[:flavor]}"
> > -+    elsif resource[:name].to_s.match(/[a-z0-9]%[0-9a-z]/i)
> > -+        "#{resource[:name]}"
> > -+    elsif not latest
> > -+      "#{resource[:name]}--"
> > -     else
> > -       # If :ensure contains a version, use that instead of looking it up.
> > -       # This allows for installing packages with the same stem, but 
> > multiple
> > -@@ -194,33 +159,41 @@ Puppet::Type.type(:package).provide :openbsd, 
> > :parent 
> > -         use_version = get_version
> > -       end
> > +-    else
> > +-      # If :ensure contains a version, use that instead of looking it up.
> > +-      # This allows for installing packages with the same stem, but 
> > multiple
> > +-      # version such as openldap-server.
> > +-      if @resource[:ensure].to_s =~ /(\d[^-]*)$/
> > +-        use_version = @resource[:ensure]
> > +-      else
> > +-        use_version = get_version
> > +-      end
> >   
> >  -      [@resource[:name], use_version, 
> > @resource[:flavor]].join('-').gsub(/-+$/, '')
> > -+      if resource[:flavor]
> > -+        [ @resource[:name], use_version, 
> > @resource[:flavor]].join('-').gsub(/-+$/, '')
> > -+      else
> > -+        [ @resource[:name], use_version ]
> > -+      end
> > ++    name_branch_regex = /^(\S*)(%\w*)$/
> > ++    match = name_branch_regex.match(@resource[:name])
> > ++    if match
> > ++      use_name = match.captures[0]
> > ++      use_branch = match.captures[1]
> > ++    else
> > ++      use_name = @resource[:name]
> > ++      use_branch = ''
> >       end
> > -   end
> > +-  end
> >   
> > -   def get_version
> > +-  def get_version
> >  -    execpipe([command(:pkginfo), "-I", @resource[:name]]) do |process|
> >  -      # our regex for matching pkg_info output
> >  -      regex = /^(.*)-(\d[^-]*)[-]?(\w*)(.*)$/
> >  -      master_version = 0
> >  -      version = -1
> > -+    pkg_search_name = @resource[:name]
> > -+    unless pkg_search_name.match(/[a-z0-9]%[0-9a-z]/i) and not 
> > @resource[:flavor]
> > -+      # we are only called when no flavor is specified
> > -+      # so append '--' to the :name to avoid patch versions on flavors
> > -+      pkg_search_name << "--"
> > -+    end
> > -+    # our regex for matching pkg_info output
> > -+    regex = /^(.*)-(\d[^-]*)[-]?(\w*)(.*)$/
> > -+    master_version = 0
> > -+    version = -1
> > - 
> > +-
> >  -      process.each_line do |line|
> >  -        match = regex.match(line.split[0])
> >  -        if match
> > @@ -224,29 +263,36 @@
> >  -
> >  -          master_version = version unless master_version > version
> >  -        end
> > -+    # pkg_info -I might return multiple lines, i.e. flavors
> > -+    matching_pkgs = pkginfo("-I", "pkg_search_name")
> > -+    matching_pkgs.each_line do |line|
> > -+      if match = regex.match(line.split[0])
> > -+        # now we return the first version, unless ensure is latest
> > -+        version = match.captures[1]
> > -+        return version unless @resource[:ensure] == "latest"
> > -+        master_version = version unless master_version > version
> > -       end
> > -+    end
> > - 
> > +-      end
> > +-
> >  -      return master_version unless master_version == 0
> >  -      return '' if version == -1
> > -+    return master_version unless master_version == 0
> > -+    return '' if version == -1
> > -+    raise Puppet::Error, _("%{version} is not available for this 
> > package") % { version: version }
> > - 
> > +-
> >  -      raise Puppet::Error, _("%{version} is not available for this 
> > package") % { version: version }
> > --    end
> > -   rescue Puppet::ExecutionFailure
> > -     return nil
> > ++    if @resource[:flavor]
> > ++      return "#{use_name}--#{@resource[:flavor]}#{use_branch}"
> > ++    else
> > ++      return "#{use_name}--#{use_branch}"
> > +     end
> > +-  rescue Puppet::ExecutionFailure
> > +-    return nil
> > ++
> >     end
> > -@@ -239,7 +212,7 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
> > + 
> > +   def query
> > +-    # Search for the version info
> > +-    if pkginfo(@resource[:name]) =~ /Information for 
> > (inst:)?#{@resource[:name]}-(\S+)/
> > +-      return { :ensure => $2 }
> > +-    else
> > +-      return nil
> > ++    pkg = self.class.instances.find do |package|
> > ++      @resource[:name] == package.name
> > +     end
> > ++    pkg ? pkg.properties : nil
> > +   end
> > + 
> > +   def install_options
> > +@@ -239,15 +117,19 @@ Puppet::Type.type(:package).provide :openbsd, 
> > :parent 
> >     end
> >   
> >     def uninstall_options
> > @@ -254,4 +300,27 @@
> >  +    [join_options(resource[:uninstall_options])]
> >     end
> >   
> > -   def uninstall
> > +-  def uninstall
> > +-    pkgdelete uninstall_options.flatten.compact, @resource[:name]
> > ++  def uninstall(purge = false)
> > ++    if purge
> > ++      pkgdelete "-c", "-qq", uninstall_options.flatten.compact, 
> > get_full_name(action="uninstall")
> > ++    else
> > ++      pkgdelete uninstall_options.flatten.compact, 
> > get_full_name(action="uninstall")
> > ++    end
> > +   end
> > + 
> > +   def purge
> > +-    pkgdelete "-c", "-q", @resource[:name]
> > ++    uninstall(purge=true)
> > +   end
> > + 
> > +   def flavor
> > +@@ -256,7 +138,6 @@ Puppet::Type.type(:package).provide :openbsd, :parent 
> > + 
> > +   def flavor=(value)
> > +     if flavor != @resource.should(:flavor)
> > +-      uninstall
> > +       install
> > +     end
> > +   end
> 
>

Attachment: puppet7-package.diff
Description: Binary data

Reply via email to