-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Not meaning to butt in on my own patch thread, but is there anything
particularly wrong with the code below?

This patch was generated from the latest head of the day.

I'll have a couple of hours coming up to work on this so would like to
get some feedback if possible.

Thanks!

Trevor

On 01/04/2010 09:42 PM, Trevor Vaughan - Onyx Point wrote:
> From: Trevor Vaughan <[email protected]>
> 
> 
> Signed-off-by: Trevor Vaughan - Onyx Point <[email protected]>
> ---
>  lib/puppet/type/file/mode.rb |  115 +++++++++++++++++++++++++++++++++--------
>  1 files changed, 92 insertions(+), 23 deletions(-)
> 
> diff --git a/lib/puppet/type/file/mode.rb b/lib/puppet/type/file/mode.rb
> index 83034cb..dd582cf 100755
> --- a/lib/puppet/type/file/mode.rb
> +++ b/lib/puppet/type/file/mode.rb
> @@ -4,9 +4,8 @@
>  module Puppet
>      Puppet::Type.type(:file).newproperty(:mode) do
>          require 'etc'
> -        desc "Mode the file should be.  Currently relatively limited:
> -            you must specify the exact mode the file should be.
> - 
> +        desc "Mode the file should be.  You may specify either the precise 
> octal mode or the POSIX symbolic mode per GNU coreutils chmod.
> +
>              Note that when you set the mode of a directory, Puppet always
>              sets the search/traverse (1) bit anywhere the read (4) bit is 
> set. 
>              This is almost always what you want: read allows you to list the
> @@ -22,9 +21,74 @@ module Puppet
>  
>              In this case all of the files underneath ``/some/dir`` will have 
>              mode 644, and all of the directories will have mode 755."
> -
>          @event = :file_changed
>  
> +        # This is a helper that takes a symbolic string and a file path and
> +        # returns the adjusted decimal representation octal file mode (0700,
> +        # etc...)
> +        def sym2oct(str,path)
> +            if str.to_s =~ /^\d+$/
> +                value = str
> +            else
> +                curmode = "00000"
> +                if File.exists?(path) then
> +                    curmode = "%o" % File.stat(path).mode
> +                    curmode = curmode[-5 .. -1]
> +                end
> +                value = Integer(curmode)
> +    
> +                base =  {
> +                        "u" => 6,
> +                        "g" => 3,
> +                        "o" => 0
> +                        }
> +
> +                left =  {
> +                        "u" => 05700, 
> +                        "g" => 03070, 
> +                        "o" => 01007, 
> +                        "a" => 07777
> +                        }
> +
> +                right = {
> +                        "r" => 00444, 
> +                        "w" => 00222, 
> +                        "x" => 00111, 
> +                        "s" => 06000, 
> +                        "t" => 01000, 
> +                        "u" => 00700, 
> +                        "g" => 00070, 
> +                        "o" => 00007 
> +                        }
> +
> +                reg = /^(([ugoa]+)([+-=])([rwxst]+|[ugo]),?)+$/
> +    
> +                str.split(",").each do |cmd|
> +                    match = cmd.match(reg) or return curmode
> +                    # The following vars are directly dependent on the
> +                    # structure of the regex (reg) above
> +                    who = match[2]
> +                    what = match[3]
> +                    how = match[4].split(//).uniq.to_s
> +                    if how =~ /^[ugo]$/ then
> +                      who.split(//).uniq.each do |lhv|
> +                        right[how] = ( ((value << (base[lhv] - base[how])) & 
> right[lhv]) | ( value & ~right[lhv] ) ) & 0777
> +                      end
> +                    end
> +                    who = who.split(//).inject(num=0) {|num,b| num |= 
> left[b]; num }
> +                    how = how.split(//).inject(num=0) {|num,b| num |= 
> right[b]; num }
> +                    mask = who & how
> +                    case what
> +                        when "+": value = value | mask
> +                        when "-": value = value & ~mask
> +                        when "=": value = ( mask & who ) | ( value & ~(who & 
> 0777) )
> +                    end
> +                end
> +            end
> +            Integer("0%o" % value)
> +        end
> +
> +
>          # Our modes are octal, so make sure they print correctly.  Other
>          # valid values are symbols, basically
>          def is_to_s(currentvalue)
> @@ -52,29 +116,35 @@ module Puppet
>          end
>  
>          munge do |should|
> -            # this is pretty hackish, but i need to make sure the number is 
> in
> -            # octal, yet the number can only be specified as a string right 
> now
> +            # This handles both numbers and symbolic modes matching the regex
> +            # /^(([ugoa]+)([+-=])([rwx]+),?)$/ 
> +            #
> +            # Note: This now returns a string and the accepting function must
> +            # know how to handle it!
> +            lregex = /^(([ugoa]+)([+-=])([rwxst]+|[ugo]),?)+$/
> +
>              value = should
> +
>              if value.is_a?(String)
> -                unless value =~ /^\d+$/
> -                    raise Puppet::Error, "File modes can only be numbers, 
> not %s" %
> +                if value =~ /^\d+$/ then
> +                    unless value =~ /^0/
> +                        value = "0#{value}"
> +                    end
> +
> +                    old = value
> +                    begin
> +                        value = Integer(value)
> +                    rescue ArgumentError => detail
> +                        raise Puppet::DevError, "Could not convert %s to 
> integer" %
> +                            old.inspect
> +                    end
> +                elsif value.match(lregex).nil? then
> +                    raise Puppet::DevError, "Symbolic mode %s does not match 
> #{lregex}" %
>                          value.inspect
>                  end
> -                # Make sure our number looks like octal.
> -                unless value =~ /^0/
> -                    value = "0" + value
> -                end
> -                old = value
> -                begin
> -                    value = Integer(value)
> -                rescue ArgumentError => detail
> -                    raise Puppet::DevError, "Could not convert %s to 
> integer" %
> -                        old.inspect
> -                end
>              end
> -
> -            return value
> -        end
> +            return sym2oct(value,@resource[:path])
> +       end
>  
>          # If we're a directory, we need to be executable for all cases
>          # that are readable.  This should probably be selectable, but eh.
> @@ -121,7 +191,6 @@ module Puppet
>  
>          def sync
>              mode = self.should
> -
>              begin
>                  File.chmod(mode, @resource[:path])
>              rescue => detail

- -- 
Trevor Vaughan
 Vice President, Onyx Point, Inc.
 email: [email protected]
 phone: 410-541-ONYX (6699)

- -- This account not approved for unencrypted sensitive information --
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAktLyd8ACgkQyWMIJmxwHpSlfgCeIFFL7tvfzTesGLJeUVbja14h
irwAmwUoPGAB4LdCm20OnBVjiocLUg9Y
=JriN
-----END PGP SIGNATURE-----
-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en.


Reply via email to