Puppet

I should add that there are no errors (or output of any kind) related to tidy in the client’s log—unless I introduce an invalid attribute, in which case it is noted (which I mention simply to confirm that this isn’t due to some more fundamental failure to pull the manifest from the master).

Any update on this? I’m experiencing the same exact behavior 0.25.1 (still have to update, but it I rather wait until this is patched).

It is the intended behaviour I think – at least useful as a work-around.

Alright if that is intended behavior the example in the documentation regarding the Tidy type would be inaccurate

tidy { "/tmp":
    age => "1w",
    recurse => false,
    matches => [ "[0-9]pub*.tmp", "*.temp", "tmpfile?" ]
}

Thanks for clarifying :)

Well, I don’t know if this is how it’s supposed to work but it makes absolutely no sense from a user perspective – if I have a directory with a bunch of logfiles I shouldn’t have to ‘recurse’ if it’s a flat directory with some files in and no subdirectories. This just took me about 30 minutes to discover and solve with my first attempt to use tidy{}.

Ashley Penney wrote:

Well, I don’t know if this is how it’s supposed to work but it makes absolutely no sense from a user perspective – if I have a directory with a bunch of logfiles I shouldn’t have to ‘recurse’ if it’s a flat directory with some files in and no subdirectories. This just took me about 30 minutes to discover and solve with my first attempt to use tidy{}.

I tend to agree, I don’t think this was intended behavior and or wasn’t thought out thoroughly before implementing. Luckily my use of Tidy right now it’s not a big issue however if I have files in sub-directories that match my regex and I don’t want them pruned it could be a very annoying / crucial issue.

Markus, thanks for investigating. Personally, I think the only sensible default is recurse=1. Given a directory and a matches value, everyone will expect tidy to scan it accordingly. Defaulting to zero is highly unintuitive.

That being said, I’m delighted to have a fix, so do what you will — as long as the documentation is accurate, a “bad” default is not the end of the world.

Peter —

So how about having recursive default to 1 if (and only if?) matches is specified? Or having it be an error to specify “matches” with recursive false/0?

— Markus

That would be better — but honestly, I think it’s exceedingly unlikely anyone would specify a directory, even without matches, and not expect it to tidy the immediate contents of that directory. Tidying up files in a directory (e.g., based on file modification time) is a very, very common use-case (/tmp being the obvious example).

In contrast, tidying a directory with recurse=0 makes almost no sense in any scenario. What could it mean? Either a noop, which by definition no one would intend to specify, or else a request to remove the directory itself, which seems highly unusual (and only makes sense if it’s empty — otherwise it’s still a noop).

I tend to be quite conservative in these sorts of matters, erring on the side of not deleting things — but in this case the feature is for deleting things and recurse=0 fails to accomplish that. I think users will want and expect tidy to recurse=1 on a directory (even without matches) and will be very surprised if it doesn’t. However, if you disagree, please throw an error if recurse is unspecified for a directory rather than making it a silent noop. Special-casing the matches attribute doesn’t make much sense since others like age might be used instead.

Regardless of what you decide, I appreciate the time and thought you’re putting into solving this. Thanks!

So the difference between match and tests such as age is that, for a given target, match will either always be true or never be true, but the value doesn’t change. It would make sense to write something like:

tidy { "/my/occasional.log": age => '1w' }

with the intent that /my/occasional.log gets tidied after it sits for a week. But

tidy { "/my/occasional.log": matches => "*.txt" }

doesn’t make sense; /my/occasional.log will never match @.txt@, and so this is just a noop. (The converse case with matches => “@.log@” is only a little less silly).

At the moment I’m leaning towards requiring a non-default, non-false, non-zero value for recurse if matches is specified; rather than trying to guess and “do the right thing” when someone specifies something a meaningless setting, we should tell them and make them specify something meaningful.

— Markus

P.S. The killer argument about defaulting to “recurse => 1” is that we just push the problem into different edge cases; what if they specify “recurse => false” (as in the example text)? What if they specify “recurse => 1”? Do we bump it to 2? Or is 1 now a synonym for 0? And so on.

So the explanation for the problem I observed in comment 18 above is that I’m testing on a Mac and my /tmp is a symlink. If I specify “/tmp/” I get the directory but if I just specify “/tmp” I get the symlink (and it doesn’t recurse across it).

As there’s no “links” attribute for tidy this seems to be a limitation, though not as mysterious as it originally seemed. Specifically, it appears that tidy will never recurse across symlinks it encounters.