2009/3/12 Daniel Carosone
<address@hidden>
> Do you see that as a blocker for 0.43? While the attribute handling is
> still not perfect, it already got a lot better, no?
I'm not sure that it's a blocker but it would be nice to not be changing behaviour like this from release to release if we can help it.
Is this also the case for a forward update - ie, from a rev with an
explicit setting to a rev where the attr has been dropped?
At the moment update will clear the execute bits when moving from a rev with the attr set to one with it dropped or with it set to anything other than "true".
If we change the hook to only do things when mtn:execute is *set* then update will only clear the execute bits when it's *set* to "false". If it was set to any other value or if it was dropped update wouldn't touch the bits.
If so, I'd say it could be worth more consideration - otherwise it's
rare enough that it can be addressed with documentation in the
meantime.
I'm not convinced this is the wrong behaviour in either case (dropping
the attr amounts to a "don't care" setting), but some thought about
"least surprise" for user expectations is needed.
The counter-argument is that "update" should produce the same result as
"checkout", plus local changes, and "checkout" defaults to no-exec.
This is exactly the case where it came up, update didn't produce "correct" (equivalent to checkout) results in terms of execute bits and checkout can't be used for revisions that don't have branch certs. So there's no good way to get to those revisions with proper execute permission settings.
In terms of checkout and update agreeing on execute permissions I think the current behaviour is "correct" but then revert is the odd man out. If you update to a rev with no mtn:execute attribute on some file, chmod +x that file and then mtn revert that file the execute bits will not be cleared even though both checkout and update would clear them.
Another thought here is that revert should use the same editable-tree machinery that checkout and update use to do their work. Then it would get similar results. This turns out to be tricky in a few different ways though. First, update can fail due to workspace conflicts and people probably wouldn't expect revert to fail. The other thing is that reverting a drop is not eactly an addition so things would have to be done carefully in this case to avoid breaking file lifecycles.
So.. perhaps "diff" or "status" should show the x-bit set somehow as a
workspace change carried across from the update? Could be useful in
general to find files that have mismatched attrs
I had wondered about this as well. The execute bits do feel somewhat like file content and maybe monotone should just track these values similarly and manage the mtn:execute attribute automatically.