@indygreg I think you saw this failure mode, I'd appreciate it if you could check if this fixes the watchman failures I introduced (I don't use watchman, and so I'm not quite sure how to reproduce.)

In D2939#47601, @indygreg wrote:

This still fails for me with:

  File "/home/gps/src/hg/mercurial/context.py", line 1590, in _dirstatestatus
    clean=clean, unknown=unknown)
  File "/home/gps/src/hg/mercurial/dirstate.py", line 1073, in status
    elif (time != st[stat.ST_MTIME]
IndexError: tuple index out of range

Also, hacking the low-level bser data type at the bser level feels very wrong. I would rather we roll our own C type that holds a reference to the bserObject and provides the necessary object API. Although there are already Mercurial hacks in bser.c for st_* attribute access. So I could be convinced that this hack is acceptable.

Yeah, I mostly wanted to verify a fix before trying to do something principled.

I can try debugging this failure if that would be helpful. Perhaps something in CPython land is evaluating len() and short-circuiting the [8] access.

Yes, debugging would help a ton. I don't really have enough understanding to make enough headway here :(

I may look into this today. Having fsmonitor disabled makes operations on Firefox repos much slower and this is hampering my ability to hack on that repo :/

I managed to debug this.

st[state.ST_MTIME] will invoke PyObject_GetItem. Its implementation does this:

m = o->ob_type->tp_as_mapping;
if (m && m->mp_subscript)
    return m->mp_subscript(o, key);

if (o->ob_type->tp_as_sequence) {
    if (PyIndex_Check(key)) {
        Py_ssize_t key_value;
        key_value = PyNumber_AsSsize_t(key, PyExc_IndexError);
        if (key_value == -1 && PyErr_Occurred())
            return NULL;
        return PySequence_GetItem(o, key_value);
    }

Our bserObjectType defines tp_as_mapping and st[state.ST_MTIME] ends up calling bserobj_getattrro(). That function sees the argument is an integer and ends up calling PySequence_GetItem():

if (PyIndex_Check(name)) {
  i = PyNumber_AsSsize_t(name, PyExc_IndexError);
  if (i == -1 && PyErr_Occurred()) {
    goto bail;
  }
  ret = PySequence_GetItem(obj->values, i);
  goto bail;
}

PySequence_GetItem() does the following:

m = s->ob_type->tp_as_sequence;
if (m && m->sq_item) {
    if (i < 0) {
        if (m->sq_length) {
            Py_ssize_t l = (*m->sq_length)(s);
            if (l < 0)
                return NULL;
            i += l;
        }
    }
    return m->sq_item(s, i);
}

sq_item here is tupleitem(), not bserobj_tuple_item(). It tells the truth and IndexError is raised.

So, I think we want the hack to live in bserobj_getattrro(). I don't think the hack needs to live in bserobj_tuple_item() because that function only ever gets called through bserobj_getattrro().

PTAL - I think this should be ready to go.