Add function modified() to select nodes which are marked as dirty

[georgehansper]

This Pull Request adds a new boolean function modified() to path-expressions.

The function modified() reads the status if the dirty flag of the context node,
and returns this as a boolean value.

The allows the current tree to be interrogated, showing which nodes have been modified
since they where loaded.

eg:

augtool> set /files/etc/hosts/00/ipaddr 1.2.3.4
augtool> match /files//*[modified()]
/files/etc/hosts = (none)
/files/etc/hosts/00 = (none)
/files/etc/hosts/00/ipaddr = 1.2.3.4
augtool> 

This Pull Request alters the existing logic around the dirty flag in the tree, so that:

• only the actual nodes modified are marked 'dirty`, and not the parent nodes of the modified node(s).
• the file-node of the modified node(s) is still marked dirty, as before
• all existing functionality controlled by the dirty flag has been preserved by adjusting the logic as required.

eg

augtool> print /files/etc/hosts/1
/files/etc/hosts/1
/files/etc/hosts/1/ipaddr = "127.0.0.1"
/files/etc/hosts/1/canonical = "localhost"
/files/etc/hosts/1/alias[1] = "localhost4"
/files/etc/hosts/1/alias[2] = "localhost.localdomain"
/files/etc/hosts/1/#comment = "ipv4"
augtool> set /files/etc/hosts/1/ipaddr "127.0.0.1"
augtool> set /files/etc/hosts/1/alias[2] "myhost.localdomain"
augtool> match /files//*[modified()]
/files/etc/hosts = (none)
/files/etc/hosts/1/alias[2] = myhost.localdomain
augtool> 

The 1st set command does not change the existing value, so the node is not marked as dirty
The 2nd set command changes the node alias[2] and only this node and it's corresponding file-node /files/etc/hosts are marked as dirty

Only nodes with the dirty flag set are selected by the function modified()

[raphink]

I remember talking about something similar a few years ago on the mailing list, but I do like this approach.

[lutter]

I really like the idea of the modified functionality - my concern with this PR is the performance implication of not passing dirty up the tree. The reason it was passed up the tree was so that it would be easy to determine which files needed saving; with the PR we call tree_save unconditionally, which I think can slow things down quite a bit with large trees (or large unmodified files in the tree, like /etc/services)

It might be better to add a flag modified to struct tree that gets set on the nodes that were actually modified, and keep letting dirty indicate that one of the descendants of a node were modified.

[georgehansper]

This change still passes dirty up the tree, but instead of flagging every parent node, it only flags the node(s) with the file flag set.

The net result is that there is no additional effort required to find the nodes that have both file and dirty set.
The code does not descend the tree past the node(s) with the file flag set, which is the same as before.

ie. this PR does not increase the amount of processing required on a save.

As a side-effect, when changing a single node, the corresponding file node is also matched by the function modified() even though technically, the file node itself has not been modified.

It think this behaviour is 'fair' because the file will actually be written on a save, so it's fair to call the file "modified".

This behaviour is shown in the example above, where /files/etc/hosts = (none) appears, even though it is not the subject of the set command.

Do you think this is desirable behaviour, or would you rather that the file node not in included implicitly like this?

It would not be hard change the file flag to an enumerated type such ash { FILE_CLEAN, FILE_DIRTY, NOT_A_FILE }, so that it doesn't have to 'share' with the dirty flag. But I'm not sure this is really "better".

[lutter]

You are right - I had misread your PR the first time around, and so I don't have an objection anymore to this.

[ekohl]

This has caused a regression in Puppet's use case: voxpupuli/puppet-augeasproviders_sysctl#65 (comment) has a minimal reproducer.

[georgehansper]

@ekohl Thanks fore reporting this, and identifying the commit that provoked the problem.
I will investigate further.

FYI the problem can be reproduced from augtool, too, as follows:

augtool> print /files/etc/sysctl.d/20-fs.conf
augtool> set /files/etc/sysctl.d/20-fs.conf/fs.nr_open 100002
augtool> save
Saved 1 file(s)
augtool> print /files/etc/sysctl.d/20-fs.conf
/files/etc/sysctl.d/20-fs.conf
/files/etc/sysctl.d/20-fs.conf/fs.nr_open = "100002"
augtool> load
augtool> print /files/etc/sysctl.d/20-fs.conf
augtool> load
augtool> print /files/etc/sysctl.d/20-fs.conf
/files/etc/sysctl.d/20-fs.conf
/files/etc/sysctl.d/20-fs.conf/fs.nr_open = "100002"
augtool> 

[ekohl]

Thanks for that and credits to @jay7x for diving into it.