[PATCH/puppet 1/1] Fix 2675 - re-add ending slash to directory names

[Marc Fournier]

The ending slash in resource file name gets stripped off by the File.split
operation in munge(). This patch adds it back again.

Signed-off-by: Marc Fournier <marc.f...@camptocamp.com>
---
lib/puppet/type/file.rb | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 34dc445..b870d48 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -39,7 +39,8 @@ module Puppet
# path name. The aim is to use less storage for all common paths in a hierarchy
munge do |value|
path, name = File.split(value)
- { :index => Puppet::FileCollection.collection.index(path), :name => name }
+ terminator = (value[-1,1] == File::SEPARATOR) ? File::SEPARATOR : ''
+ { :index => Puppet::FileCollection.collection.index(path), :name => name + terminator }
end

# and the reverse
--
1.6.3.3

[Marc Fournier]

>
> The ending slash in resource file name gets stripped off by the
> File.split operation in munge(). This patch adds it back again.
>

Duritong and I just realized that this patch introduces another regression. It
would now be possible to have a duplicate definition this way:

file { "foobar1":
ensure => directory,
path => "/tmp/dir4/",
mode => 0666
}
file { "foobar2":
ensure => directory,
path => '/tmp/dir4',
mode => 0600
}

The problem we have is that this example doesn't behave as one would expect
starting from commit cc09c1af:

file { "/tmp/dir3/": ensure => directory }
file { "/tmp/dir3/file3": ensure => present, require => File["/tmp/dir3/"] }

Maybe a better approach would be to emit a warning when a resource with a
trailing slash in its name gets defined ? Or internally adding the user defined
name as an alias so we have both filenames available, the original one and
another without the trailing slash ?

What do you think ?

[Peter Meier]

oh and we wanted to write a failing test for that, so we could test any
of these approaches. but we failed :/ so maybe somebody could give us a
hand in writing a test for that?

cheers pete

[Luke Kanies]

Yeah, you're running into the issue mentioned in the related ticket -
we need to just not consider the trailing slashes, rather than
requiring them, retaining them, or what.

I think it's more complicated to fix this than we'd like -- we need a
class-level (i.e., for Puppet::Type::File) title munging method to
make this consistent, and then the Puppet::Resource::Reference class
needs to use this method to normalize the file names, so that /foo/
bar/ is considered equal to /foo/bar.

--
Meeting, n.:
An assembly of people coming together to decide what person or
department not represented in the room must solve a problem.
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com

[Marc Fournier]

I understand this issue is more complicated than expected. The thing
is, a language feature which worked well with 0.24.x is now broken in
0.25 :-(

As more people will be migrating their clients to 0.25, I'm afraid this
will become an annoying issue.

In my case, migration is blocked until I fix all my manifests. A rough
grep to find all file resources ending with a slash shows I'll have to
fix more than 100 resources and dependencies.

What would be the answer for users in my case:

- fix all your manifests for something that was working before and
will work again in the future ?
- delay upgrading to 0.25 until this complicated issue is resolved ?

Or would it be acceptable to put some sort of workaround in puppet
meanwhile (thinking of auto-adding an alias to each resource ending
with a slash), and remove it once the complicated issue is fixed ?

Marc

[Luke Kanies]

Hmm, I don't really know. I guess we could add a hackish solution, I
just assumed it was a relatively simple regex to fix existing file
resources.

What do others think?

--
SCSI is *not* magic. There are fundamental technical reasons
why it is necessary to sacrifice a young goat to your SCSI chain
now and then.