For those who've been following along I've made some changes to how attribute changes are handled in the workspace.
The old code had a function called update_any_attrs, which went through all nodes in the workspace roster and called the lua attr_functions for any live attr. This was called by pretty much every command that modified the workspace in any way, which could cause unexpected attribute changes. For example reverting, adding, renaming, deleting, etc. one file could end up setting attributes on other files. Also, this call would not do anything for workspace files with attributes that were removed or cleared, so updating to a rev that cleared an attribute would leave the attribute set on associated workspace files.
The initial fix for update not clearing attributes in rev b1cec3176fd56af29275c2b620f8766b4382eec8 made a change in work.cc to remove two FIXME_ROSTERS entries on the clear_attr and set_attr workspace functions and make these actually work when applying a cset to the workspace. With these working properly I was able to continue on and remove all of the calls to update_any_attrs in subsequent revisions. I've also changed revert to call the attr_functions for any existing attrs (live or dead) on reverted paths so that attributes can be properly reverted.
There is now one small inconsistency with the mtn:execute attribute, in particular, with the default attr_functions lua hook. This hook currently looks like this:
function(filename, value) if (value == "true") then
set_executable(filename) else clear_executable(filename) end end
which will set execute permissions if mtn:execute is set to true and clear them if it is set to anything else (false, nil, foobar). This hook is called with value=nil for dormant attributes, it is not called for paths that have never had the mtn:execute attribute set, which causes the inconsistency:
- upating from a revision with no mtn:execute on a file to one that has mtn:execute set to true will set the file's execute bits - updating to another revision with mtn:execute set to false on the same file will clear the file's execute bits
- updating back to the revision with mtn:execute set to true will again set the file's execute bits * updating to the initial revision where no mtn:execute attr existed will leave the file's execute bits set
This isn't a huge problem but maybe we can do better. Thomas Moschny has suggested that we should only set execute bits when mtn:execute=true and only clear them when mtn:execute=false which essentially means that unless there are specific instructions monotone shouldn't touch the execute bits. This seems like a good suggestion and I'm curious what others think.
value = "true" -- set execute bits value = "false" -- clear execute bits value = "anything else" -- currently clears execute bits, but perhaps should not touch them value = nil -- (dormant attr) currently clears execute bits, but perhaps shoult not touch them