D8454: phabricator: ensure that `phabsend` is given a contiguous, linear commit range

[marmoute (Pierre-Yves David)]

marmoute added inline comments.

INLINE COMMENTS

> mharbison72 wrote in phabricator.py:1314
> > Are the revs already sorted ?
> 
> Yes, see line 1298 above.  (Which has test coverage IIRC)
> 
> > Did we filter set with multiple head/root already?
> 
> We did not.  I thought I saw something in evolve or another extension that required a linear set, and this is getting complicated enough that it probably deserves a utility function.  But I think before doing this, we should get to the bottom of D8457 <https://phab.mercurial-scm.org/D8457> in case there are other ways to hit that.
> 
> > (also, not that if revs is a smartset, you can use first() and last() on them IIRC.)
> 
> It's returned by `scmutil.revrange()` and has `sort()`, so I assume it is?

>>   Are the revs already sorted ?
>
> Yes, see line 1298 above. (Which has test coverage IIRC)

Great

>>   Did we filter set with multiple head/root already?
>
> We did not.

Okay, then the current code will fail to detect non-linearity from other head or root.

As an interresting side effect, a non linear set will have more than one head and more than one root. So implementing root/head checking will reach your current goal for free.

You can do so using `repo.revs('heads(%ld)')` and `repo.revs('heads(%ld)')`. You could report all extra roots but the minimal one and/or all extra heads but the maximal ones.

>> (also, not that if revs is a smartset, you can use first() and last() on them IIRC.)
>
> It's returned by scmutil.revrange() and has sort(), so I assume it is?

I doubled checked, it is.

> phabricator.py:1318
> +    ):
> +        raise error.Abort(_(b"cannot phabsend non-linear revisions"))
> +

This error message does not specify the revision it it complaining about it. It would be better to do so. It make the error simpler to understand and resolve for the user.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8454/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8454

To: mharbison72, #hg-reviewers
Cc: marmoute, Kwan, mercurial-devel