I'm not really happy with this. Why can't eden be replacing the dirstatemap entirely, and be intercepting things at that layer?

@durin42 The issue is that dirstatemap is doing so much "stuff" beyond just storage that Eden prefers to subclass it rather than copy/paste all of the business logic.

In D1354#22991, @mbolin wrote:

@durin42 The issue is that dirstatemap is doing so much "stuff" beyond just storage that Eden prefers to subclass it rather than copy/paste all of the business logic.

That's an argument for further splitting dirstatemap. That said, does D1347 resolve the specific concern that motivated this change?

Is this still relevant? Nobody's answered my question.

If you want, I'd be happy to make the dirstatetuple factory a class-level attribute on the dirstatemap - I'm not quite following what the extra logic you need is, but I'd very much like to make Eden easier, you just need to help me understand the constraints better: I can't read your mind.

@durin42 Here's the implementation of this method we are using in Eden:

def _insert_tuple(self, filename, state, mode, size, mtime):  # override
    if size != MERGE_STATE_BOTH_PARENTS and size != MERGE_STATE_OTHER_PARENT:
        merge_state = MERGE_STATE_NOT_APPLICABLE
    else:
        merge_state = size

    self._map[filename] = (state, mode, merge_state)

As you can see, we want to do some conditional logic before doing the insert into self._map (and we end up dropping one of the tuple fields), so it seemed like the most straightforward thing for Eden to do is to intercept the writes. It's true that we could also redefine dirstatetuple(), but I think that's more confusing because there are places where we need to convert between Eden's internal tuples stored in eden_dirstate_map and the dirstatetuple type that Mercurial expects everywhere else, so I'm disinclined to conflate the two things.

In D1354#28409, @mbolin wrote:

@durin42 Here's the implementation of this method we are using in Eden:

def _insert_tuple(self, filename, state, mode, size, mtime):  # override
    if size != MERGE_STATE_BOTH_PARENTS and size != MERGE_STATE_OTHER_PARENT:
        merge_state = MERGE_STATE_NOT_APPLICABLE
    else:
        merge_state = size
    self._map[filename] = (state, mode, merge_state)

As you can see, we want to do some conditional logic before doing the insert into self._map (and we end up dropping one of the tuple fields), so it seemed like the most straightforward thing for Eden to do is to intercept the writes. It's true that we could also redefine dirstatetuple(), but I think that's more confusing because there are places where we need to convert between Eden's internal tuples stored in eden_dirstate_map and the dirstatetuple type that Mercurial expects everywhere else, so I'm disinclined to conflate the two things.

Ahhhh. That was extremely not obvious from your commit message. If it'd still help, I'd be happy to land this with a clearer log message and a comment on _insert_tuple that it's been extracted to ease the implementation of hg on FUSE filesystems and other similar use cases (is that a good summary?) so that it makes more sense to readers...