On Tue, Oct 26, 2010 at 5:29 PM, Jesse Wolfe <jes5...@gmail.com> wrote:
> File modes can now be specified as symbolic strings, in the same formats
> that are supported by GNU chmod.
> If a new file is created (or if a file is replaced with a directory, or
> vice versa), all unspecified permissions default to 'denied'.
> Otherwise, a file's permissions are considered in-sync as long as chmod
> with the specified mode would not cause any permission bits to change.
> This means it's now possible to have "don't care" permissions:
> For example,
> file{"example" : mode => "a-x"}
> will remove execute permissions, but leave it possible to manage the
> read and write permissions outside of puppet.
>
> - @should.each { |val| return true if is == val or is == val.to_s }
> + @should.inject(false) { |acc, desired| acc ||
> property_matches?(desired, is) }
>
So isn't this the same as:
+ @should.any? { |desired| property_matches?(desired, is) }
- mode_int = mode ? Puppet::Util::Octal.integerForOctal(mode) : nil
> + mode_int = mode ? Puppet::Util::FileMode.bits_for_mode(mode, 0) : nil
>
Now I'm feeling silly for my to_i(8) / to_s(8) notes on the previous commit.
> # If we're a directory, we need to be executable for all cases
> # that are readable. This should probably be selectable, but eh.
> def dirmask(value)
> - if FileTest.directory?(@resource[:path])
> + if FileTest.directory?(@resource[:path]) and value =~ /^\d+$/
> value = Puppet::Util::Octal.integerForOctal(value)
> value |= 0100 if value & 0400 != 0
> value |= 010 if value & 040 != 0
>
I'm not understanding why we don't do this with symbolic modes as well...the
logic of "dirs must be executable" doesn't depend on the representation does
it?
> + def property_matches?(desired, current)
> + return false unless current
> + current_bits = Puppet::Util::Octal.integerForOctal(current)
> + desired_bits = Puppet::Util::FileMode.bits_for_mode(desired,
> current_bits, stat_is_directory?)
> + desired_bits == current_bits
> + end
> +
>
I'm probably just being dense, but I can't square this with "don't care
bits" as mentioned in the commit message.
> def retrieve
> # If we're not following links and we're a link, then we just turn
> # off mode management entirely.
>
> - if stat = @resource.stat(false)
> + if has_stat?
> unless defined?(@fixed)
> @should &&= @should.collect { |s| self.dirmask(s) }
> end
> - return Puppet::Util::Octal.octalForInteger(stat.mode & 007777)
> + return Puppet::Util::Octal.octalForInteger(stat_mode & 007777)
> else
> return :absent
> end
> end
>
> + def has_stat?
> + @resource.stat(false)
> + end
> +
> + def stat_mode
> + @resource.stat.mode & 007777
> + end
> +
> + def stat_is_directory?
> + has_stat? and @resource.stat.directory?
> + end
> +
>
So I'm stuck on trying to remember / re-figure-out what passing false to
stat() does. I know I've known in the past, but the mind forgets pain. Is
it significant that the previous version was using the value of the local
variable stat which came from a @resource.stat(false) call (stat.mode) and
this version is using @resource.stat (without the false) in stat_mode?
...and the rest of it is going to have to wait until tomorow because I'm
pooped and Puppet::Util::FileMode looks like it deserves more brain cells
than I have left.
-- Markus
-----------------------------------------------------------
The power of accurate observation is
commonly called cynicism by those
who have not got it. ~George Bernard Shaw
------------------------------------------------------------
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to puppet-...@googlegroups.com.
To unsubscribe from this group, send email to
puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
