__clean_path: rewrite, deprecate parameters, make it recursive

[4nd3r]

• Use plain rm to remove files/dirs found by explorer.
• Remove -maxdepth and make type recursive.
• Add parameters for find's -name, -path and -regex.
• Deprecate -pattern and -exclude while maintaining backwards compatibility (with -maxdepth 1).

[sideeffect42]

I like the changed parameters and increased POSIX compatibility.

But my main gripes are with: Use plain rm to remove files/dirs found by explorer.

While I like the change that skonfig -d now shows which files were actually removed, I think there are also problems coming along with this change.

In the old version the find was re-run in code-remote making it use up-to-date results (modulo maybe some another process is modifying the directory concurrently races). Now with this approach however it could be that explorer/list is run quite a bit before the actual code-remote and the explorer's results not being current anymore, thus leaving files which should've been deleted or trying to delete files which don't exist anymore. Another issue with this approach is that it would fail with file names containing newline characters.

Was the listing of the actual files to be removed in code-remote a design-goal of these changes?

[4nd3r]

Now with this approach however it could be that explorer/list is run quite a bit before the actual code-remote and the explorer's results not being current anymore, thus leaving files which should've been deleted or trying to delete files which don't exist anymore.

For files, which doesn't exist, that's why there's -f, because proposed change makes type recursive and parent dir might get delete before others.

Race condition with files (dis)appearing can happen in both ways.

Another issue with this approach is that it would fail with file names containing newline characters.

World will collapse anyway if such file is found.

Was the listing of the actual files to be removed in code-remote a design-goal of these changes?

Yes, but we can also have messages :thinking:

ander@m720q:~$ echo __clean_path /tmp/cleantest | skonfig -i - localhost -n
INFO: localhost: Starting dry run
INFO: localhost: Processing __clean_path/tmp/cleantest
INFO: localhost: Finished dry run in 1.00 seconds
ander@m720q:~$ skonfig -d localhost | grep messages:
messages: __clean_path/tmp/cleantest:/tmp/cleantest/foo

[4nd3r]

We need messages, but there's bit of duplication going on - first list explorer and then $__messages_out. I think symlinking here is good optimization because then we only write that list twice - first list explorer and then appending to global message file.

[sideeffect42]

Now with this approach however it could be that explorer/list is run quite a bit before the actual code-remote and the explorer's results not being current anymore, thus leaving files which should've been deleted or trying to delete files which don't exist anymore.

For files, which doesn't exist, that's why there's -f, because proposed change makes type recursive and parent dir might get delete before others.

True, should we add -depth? It's POSIX after all.

Race condition with files (dis)appearing can happen in both ways.

Yes, I was thinking of some case where one skonfig object creates a mess which you want to clean up with __clean_path after the other is finished.

Say you install or upgrade a package and it creates one of these nice .dpkg-old (or whatever that package manages uses) files, but you don't want them because they break the service.

__package x

require=__package/x \
__clean_path /etc/x --rm-name '*.dpkg-old'

# later
__check_messages restart_x ...

Since explorers are not bound to require it could be that explorer/list runs before __package is finished, leaving the .dpkg-old file around.

Another issue with this approach is that it would fail with file names containing newline characters.

World will collapse anyway if such file is found.

I hope not! (Or do we rely on the fact that Apple uses Icon\r, harr harr)

[sideeffect42]

We need messages, but there's bit of duplication going on - first list explorer and then $__messages_out.

Messages are cool, because it would also allow to reuse the result of this type later on, e.g. in __check_messages.

It's really unfortunate that messages cannot be generated from code-local...

I think symlinking here is good optimization because then we only write that list twice - first list explorer and then appending to global message file.

Iff we know the $__messages_out file is fresh for every object. (I think it currently is)

[4nd3r]

If we know the $__messages_out file is fresh for every object.

Yes, it is:

This way overwriting any of the two files by accident does not interfere with other types.

Source: https://www.cdi.st/manual/latest/cdist-messaging.html

[4nd3r]

@sideeffect42 ping