[Updated] D11680: push: add option to abort on dirty working copy if parent is pushed

[Alphare (Raphaël Gomès)]

This revision now requires changes to proceed.
Alphare added inline comments.
Alphare requested changes to this revision.

INLINE COMMENTS

> cmdutil.py:1118
> +    if repo.ui.config(b'commands', b'push.check-dirty') == b'abort':
> +        # We assume that commits back to the public ancestors will be pushed.
> +        if repo.revs('only(%ln, public()) & parents()', nodes):

This assumption seems false to me? For example, you can push only some drafts (if the remote already has some of them) and your parent(s) might be a secret?

I think this revset would make more sense: `(parents() and ancestors(%ln)) and not secret()`. You're asking the question "am I pushing my parents", regardless of phase... excluding secrets.

Also, this is done really high up and happens before the discovery, which means that you're not really working with the actual set of revisions you're pushing. This could probably cause subtle bugs with some extensions? Might as well not take the risk and do it inside `exchange.push`.

> commands.py:5753
>              if revs:
> -                revs = [repo[r].node() for r in logcmdutil.revrange(repo, revs)]
> -                if not revs:
> +                nodes = [
> +                    repo[r].node() for r in logcmdutil.revrange(repo, revs)

See my above comment about doing this later in the exchange when we have the final set of revisions.

REPOSITORY
  rHG Mercurial

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

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

To: martinvonz, #hg-reviewers, Alphare
Cc: Alphare, spectral, marmoute, pulkit, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20211203/60e45b26/attachment-0002.html>