Re: [Monotone-devel] update clearing execute permissions [was re:git fast-export]

[Thomas Moschny]

Derek Scherger wrote:
> I've committed a fix on net.venge.monotone for this problem in
> b1cec3176fd56af29275c2b620f8766b4382eec8 which fixes two associated
> xfailed tests in the monotone testsuite (tests/attr_mtn_execute and
> tests/defecting_attriibutes).

Great! Had something similar, but not yet working, in my workspace for a
while. I factored out the code to set/get the x bit in f954b59c88.. .

One question though: in editable_working_tree::clear_attr(), you call
lua.hook_apply_attribute with an empty string for 'value'. So, while
this doesn't lead to wrong behavior in the case of mtn:executable, I
think that's not correct in general. We should distinguish between the
two cases where some attribute has been or is set with any value (even
with an empty value), and where it is not set at all.

So in short, I think we should pass NIL to the lua hook when an
attribute is cleared, instead of "", because the latter could be a valid
value.

> It would be good to get some eyes on this change from people who
> understand the monotone internals. For those people, this change mainly
> affects the update and pluck commands and the stuff I'm marginally
> worried about is the make_revision_for_workspace and
> resolve_merge_conflicts calls because the "merged" roster may have some
> dormant attributes on it that previously were not there. From what I can
> tell, a cset between a roster with no "foo" attr and one with a dormant
> "foo" attr is empty but if it wasn't this change might cause some problems.

As far as I know the revision (i.e. changeset) format doesn't have the
vocabulary to talk about "dormant" attributes, and imho it shouldn't
need to.

Regards,
Thomas