[PATCH] patchbomb: check that targets exist at the publicurl

[Augie Fackler]

On Mon, Oct 12, 2015 at 1:18 PM, Pierre-Yves David
<pierre-yves.david at ens-lyon.org> wrote:
>
>
> On 10/12/2015 10:14 AM, Augie Fackler wrote:
>>
>> On Mon, Oct 12, 2015 at 09:11:35AM -0700, Pierre-Yves David wrote:
>>>
>>> # HG changeset patch
>>> # User Pierre-Yves David <pierre-yves.david at fb.com>
>>> # Date 1444626783 25200
>>> #      Sun Oct 11 22:13:03 2015 -0700
>>> # Node ID 004264d03eca64967c9288559cfc18be0bcab630
>>> # Parent  9ca13d10881d7044b79d903ad64653f6541591f1
>>> # EXP-Topic pb.publicurl
>>> # Available At http://hg.netv6.net/marmoute-wip/mercurial/
>>> #              hg pull http://hg.netv6.net/marmoute-wip/mercurial/ -r
>>> 004264d03eca
>>> patchbomb: check that targets exist at the publicurl
>>>
>>> Advertising that the patch are available to be pulled requires that to be
>>> true.
>>> So we check revision availability on the remote before sending any email.
>>>
>>> diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
>>> --- a/hgext/patchbomb.py
>>> +++ b/hgext/patchbomb.py
>>> @@ -525,10 +525,39 @@ def patchbomb(ui, repo, *revs, **opts):
>>>       if outgoing:
>>>           revs = _getoutgoing(repo, dest, revs)
>>>       if bundle:
>>>           opts['revs'] = [str(r) for r in revs]
>>>
>>> +    # check if revision exist on the public destination
>>> +    publicurl = repo.ui.config('patchbomb', 'publicurl')
>>> +    if publicurl is not None:
>>> +        repo.ui.debug('checking that revision exist in the public repo')
>>> +        try:
>>> +            publicpeer = hg.peer(repo, {}, publicurl)
>>> +        except error.RepoError:
>>> +            repo.ui.write_err(_('unable to access public repo: %s\n')
>>> +                              % publicurl)
>>> +            raise
>>> +        if not publicpeer.capable('known'):
>>> +            repo.ui.debug('skipping existence checks: public repo too
>>> old')
>>> +        else:
>>> +            out = [repo[r] for r in revs]
>>> +            known = publicpeer.known(h.node() for h in out)
>>> +            missing = []
>>> +            for idx, h in enumerate(out):
>>> +                if not known[idx]:
>>> +                    missing.append(h)
>>> +            if missing:
>>> +                if 1 < len(missing):
>>> +                    msg = _('public "%s" is missing %s and %i others')
>>> +                    msg %= (publicurl, missing[0], len(missing) - 1)
>>> +                else:
>>> +                    msg = _('public url %s is missing %s')
>>> +                    msg %= (publicurl, missing[0])
>>> +                hint = _('use "hg push %s -r %s"') % (publicurl,
>>> missing[0])
>>
>>
>> shouldn't the hint be something like ' '.join('-r ' + node for node in
>> heads(missing))?
>
>
> There might be multiple heads, but we should probably hint for pushing heads
> yes.
>
>
>>> +                raise error.Abort(msg, hint=hint)
>>> +
>>>       # start
>>>       if date:
>>>           start_time = util.parsedate(date)
>>>       else:
>>>           start_time = util.makedate()
>>> diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
>>> --- a/tests/test-patchbomb.t
>>> +++ b/tests/test-patchbomb.t
>>> @@ -2843,17 +2843,36 @@ single rev
>>>     +d
>>>
>>>   Test pull url header
>>>   =================================
>>>
>>> +basic version
>>> +
>>>     $ echo 'intro=auto' >> $HGRCPATH
>>> -  $ echo 'publicurl=http://example.com/myrepo/' >> $HGRCPATH
>>> +  $ echo "publicurl=$TESTTMP/t2" >> $HGRCPATH
>>>     $ hg email --date '1980-1-1 0:1' -n -t foo -s test -r '10' | grep
>>> '^#'
>>> -  # HG changeset patch
>>> -  # User test
>>> -  # Date 5 0
>>> -  #      Thu Jan 01 00:00:05 1970 +0000
>>> -  # Branch test
>>> -  # Node ID 3b6f1ec9dde933a40a115a7990f8b320477231af
>>> -  # Parent  2f9fa9b998c5fe3ac2bd9a2b14bfcbeecbc7c268
>>> -  # Available At http://example.com/myrepo/
>>> -  #              hg pull http://example.com/myrepo/ -r 3b6f1ec9dde9
>>> +  abort: public url $TESTTMP/t2 is missing 3b6f1ec9dde9
>>> +  (use "hg push $TESTTMP/t2 -r 3b6f1ec9dde9")
>>> +  [1]
>>> +
>>> +remote missing
>>> +
>>> +  $ echo 'publicurl=$TESTTMP/missing' >> $HGRCPATH
>>> +  $ hg email --date '1980-1-1 0:1' -n -t foo -s test -r '10'
>>> +  unable to access public repo: $TESTTMP/missing
>>> +  abort: repository $TESTTMP/missing not found!
>>> +  [255]
>>> +
>>> +node missing at remote
>>> +
>>> +  $ hg clone -r '9' . ../t3
>>> +  adding changesets
>>> +  adding manifests
>>> +  adding file changes
>>> +  added 3 changesets with 3 changes to 3 files
>>> +  updating to branch test
>>> +  3 files updated, 0 files merged, 0 files removed, 0 files unresolved
>>> +  $ echo 'publicurl=$TESTTMP/t3' >> $HGRCPATH
>>> +  $ hg email --date '1980-1-1 0:1' -n -t foo -s test -r '10'
>>> +  abort: public url $TESTTMP/t3 is missing 3b6f1ec9dde9
>>
>>
>> Aborting seems heavy-handed here. For trivial patches I might want to
>> continue anyway, so maybe prompt?
>
>
> Meh, broken pull url will be really annoying on the other end. The way to go
> is probably more to allow automatic push on patchbomb.

My (unstated, I should have mentioned it) assumption would be that the
pull URL would be omitted if it was broken.

> In the mean-time, this is still an experimental featureso I would says it is
> fine.

Fix the hint for multiple heads and I'll take it.

>
> --
> Pierre-Yves David