This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1431

sshpeer: add a configurable hint for the ssh error message
ClosedPublic

Authored by zuza on Nov 15 2017, 7:43 PM.

Details

Reviewers
durham
mitrandir
durin42
Group Reviewers
hg-reviewers
Commits
rHG8b1c887d52e7: sshpeer: add a configurable hint for the ssh error message
Summary

Adding a possibility to configure error hint to be shown in the case of problems with SSH. Example of such hint can be "Please see http://company/internalwiki/ssh.html".

Test Plan
  • Ran hg pull with broken link and verified the output has no hint by default:
pulling from ssh://brokenrepository.com//repo
remote: ssh: Could not resolve hostname brokenrepository.com: Name or service not known
abort: no suitable response from remote hg!
pulling from ssh://brokenrepository.com//repo
remote: ssh: Could not resolve hostname brokenrepository.com: Name or service not known
abort: no suitable response from remote hg!
(Please see http://company/internalwiki/ssh.html)

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

zuza created this revision.Nov 15 2017, 7:43 PM
zuza edited the summary of this revision. (Show Details)Nov 15 2017, 7:51 PM
zuza edited the test plan for this revision. (Show Details)
zuza added reviewers: durham, mitrandir.
zuza updated this revision to Diff 3540.Nov 15 2017, 7:58 PM
zuza edited the test plan for this revision. (Show Details)Nov 15 2017, 8:48 PM
zuza updated this revision to Diff 3541.
zuza updated this revision to Diff 3542.Nov 15 2017, 8:52 PM
durham requested changes to this revision.Nov 15 2017, 9:05 PM
durham added inline comments.
mercurial/sshpeer.py
210

The standard Mercurial way to format this would be something like:

msg = self.ui.config("ssh", "errormessage",
                     _("no suitable response from remote hg"))
self._abort(error.RepoError(msg))
tests/test-ssh.t
311

These lines seem unrelated to your change and therefore probably shouldn't have changed. Usually this is caused by your editor trying to remove trailing whitespace from a file, then run-tests -i adding the missing new line back in. Can you undo these new line changes? 'hg uncommit tests/test-ssh.t && hg commit -i' could make it pretty straightforward.

447

Same for this type of change

This revision now requires changes to proceed.Nov 15 2017, 9:05 PM
durham added a comment.Nov 15 2017, 9:05 PM

I'd also update the commit summary with an example of what a better message might be. Like "there was an ssh error, please see http://company/internalwiki/ssh.html"

zuza updated this revision to Diff 3548.Nov 15 2017, 9:18 PM
zuza marked 3 inline comments as done.Nov 15 2017, 9:19 PM
yuja added a subscriber: yuja.Nov 16 2017, 8:07 AM
In D1431#23709, @durham wrote:

I'd also update the commit summary with an example of what a better message might be. Like "there was an ssh error, please see http://company/internalwiki/ssh.html"

So the purpose of this change is quite similar to ui.supportcontact, right?

My two cents. I think it's probably better to make an additional hint configurable,
instead of replacing the original message.

mercurial/sshpeer.py
207–209

new config needs to be registered to the configitems module.

zuza edited the summary of this revision. (Show Details)Nov 16 2017, 1:58 PM
zuza edited the test plan for this revision. (Show Details)
zuza retitled this revision from sshpeer: making the ssh error message configurable to sshpeer: add a configurable hint for the ssh error message.
zuza updated this revision to Diff 3572.
zuza marked an inline comment as done.Nov 16 2017, 1:59 PM
durin42 accepted this revision.Nov 17 2017, 4:03 PM
durin42 requested changes to this revision.Nov 17 2017, 5:06 PM

D'oh: this also needs to be documented, probably in hg help config. That's from mercurial/help/config.txt - hopefully it'll be fairly straightforward to add?

This revision now requires changes to proceed.Nov 17 2017, 5:06 PM
zuza updated this revision to Diff 3631.Nov 20 2017, 4:44 AM
durin42 accepted this revision.Nov 20 2017, 6:29 PM
Closed by commit rHG8b1c887d52e7: sshpeer: add a configurable hint for the ssh error message (authored by zuza). · Explain WhyNov 20 2017, 6:36 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/configitems.py (3 lines)
Mmercurial/help/config.txt (4 lines)
Mmercurial/sshpeer.py (5 lines)
Mtests/test-ssh.t (17 lines)

Diff 3688

mercurial/configitems.py

default=True, default=True,
) )
coreconfigitem('ui', 'slash', coreconfigitem('ui', 'slash',
default=False, default=False,
) )
coreconfigitem('ui', 'ssh', coreconfigitem('ui', 'ssh',
default='ssh', default='ssh',
) )
coreconfigitem('ui', 'ssherrorhint',
default=None,
)
coreconfigitem('ui', 'statuscopies', coreconfigitem('ui', 'statuscopies',
default=False, default=False,
) )
coreconfigitem('ui', 'strict', coreconfigitem('ui', 'strict',
default=False, default=False,
) )
coreconfigitem('ui', 'style', coreconfigitem('ui', 'style',
default='', default='',

mercurial/help/config.txt

(default: False) (default: False)
``statuscopies`` ``statuscopies``
Display copies in the status command. Display copies in the status command.
``ssh`` ``ssh``
Command to use for SSH connections. (default: ``ssh``) Command to use for SSH connections. (default: ``ssh``)
``ssherrorhint``
A hint shown to the user in the case of SSH error (e.g.
``Please see http://company/internalwiki/ssh.html``)
``strict`` ``strict``
Require exact command names, instead of allowing unambiguous Require exact command names, instead of allowing unambiguous
abbreviations. (default: False) abbreviations. (default: False)
``style`` ``style``
Name of style to use for command output. Name of style to use for command output.
``supportcontact`` ``supportcontact``

mercurial/sshpeer.py

sub = util.popen4(cmd, bufsize=0) sub = util.popen4(cmd, bufsize=0)
self._pipeo, self._pipei, self._pipee, self._subprocess = sub self._pipeo, self._pipei, self._pipee, self._subprocess = sub
self._pipei = util.bufferedinputpipe(self._pipei) self._pipei = util.bufferedinputpipe(self._pipei)
self._pipei = doublepipe(self.ui, self._pipei, self._pipee) self._pipei = doublepipe(self.ui, self._pipei, self._pipee)
self._pipeo = doublepipe(self.ui, self._pipeo, self._pipee) self._pipeo = doublepipe(self.ui, self._pipeo, self._pipee)
def badresponse(): def badresponse():
self._abort(error.RepoError(_('no suitable response from ' msg = _("no suitable response from remote hg")
'remote hg'))) hint = self.ui.config("ui", "ssherrorhint")
self._abort(error.RepoError(msg, hint=hint))
yujaUnsubmitted
Done

new config needs to be registered to the configitems module.

yuja: new config needs to be registered to the configitems module.
durhamUnsubmitted
Done

The standard Mercurial way to format this would be something like:

msg = self.ui.config("ssh", "errormessage",
                     _("no suitable response from remote hg"))
self._abort(error.RepoError(msg))
durham: The standard Mercurial way to format this would be something like: ``` msg = self.ui.config…
try: try:
# skip any noise generated by remote shell # skip any noise generated by remote shell
self._callstream("hello") self._callstream("hello")
r = self._callstream("between", pairs=("%s-%s" % ("0"*40, "0"*40))) r = self._callstream("between", pairs=("%s-%s" % ("0"*40, "0"*40)))
except IOError: except IOError:
badresponse() badresponse()
lines = ["", "dummy"] lines = ["", "dummy"]

tests/test-ssh.t

changeset: 4:6c0482d977a3 changeset: 4:6c0482d977a3
parent: 0:1160648e36ce parent: 0:1160648e36ce
user: test user: test
date: Thu Jan 01 00:00:00 1970 +0000 date: Thu Jan 01 00:00:00 1970 +0000
summary: z summary: z
clone bookmarks clone bookmarks
durhamUnsubmitted
Done

These lines seem unrelated to your change and therefore probably shouldn't have changed. Usually this is caused by your editor trying to remove trailing whitespace from a file, then run-tests -i adding the missing new line back in. Can you undo these new line changes? 'hg uncommit tests/test-ssh.t && hg commit -i' could make it pretty straightforward.

durham: These lines seem unrelated to your change and therefore probably shouldn't have changed.
$ hg -R ../remote bookmark test $ hg -R ../remote bookmark test
$ hg -R ../remote bookmarks $ hg -R ../remote bookmarks
* test 4:6c0482d977a3 * test 4:6c0482d977a3
$ hg clone -e "\"$PYTHON\" \"$TESTDIR/dummyssh\"" ssh://user@dummy/remote local-bookmarks $ hg clone -e "\"$PYTHON\" \"$TESTDIR/dummyssh\"" ssh://user@dummy/remote local-bookmarks
requesting all changes requesting all changes
adding changesets adding changesets
adding manifests adding manifests
adding file changes adding file changes
stderr from remote commands should be printed before stdout from local code (issue4336) stderr from remote commands should be printed before stdout from local code (issue4336)
$ hg clone remote stderr-ordering $ hg clone remote stderr-ordering
updating to branch default updating to branch default
3 files updated, 0 files merged, 0 files removed, 0 files unresolved 3 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ cd stderr-ordering $ cd stderr-ordering
$ cat >> localwrite.py << EOF $ cat >> localwrite.py << EOF
> from mercurial import exchange, extensions > from mercurial import exchange, extensions
> >
durhamUnsubmitted
Done

Same for this type of change

durham: Same for this type of change
> def wrappedpush(orig, repo, *args, **kwargs): > def wrappedpush(orig, repo, *args, **kwargs):
> res = orig(repo, *args, **kwargs) > res = orig(repo, *args, **kwargs)
> repo.ui.write('local stdout\n') > repo.ui.write('local stdout\n')
> return res > return res
> >
> def extsetup(ui): > def extsetup(ui):
> extensions.wrapfunction(exchange, 'push', wrappedpush) > extensions.wrapfunction(exchange, 'push', wrappedpush)
> EOF > EOF
> crash = ${TESTDIR}/crashgetbundler.py > crash = ${TESTDIR}/crashgetbundler.py
> EOF > EOF
$ hg --config ui.ssh="\"$PYTHON\" $TESTDIR/dummyssh" pull $ hg --config ui.ssh="\"$PYTHON\" $TESTDIR/dummyssh" pull
pulling from ssh://user@dummy/remote pulling from ssh://user@dummy/remote
searching for changes searching for changes
remote: abort: this is an exercise remote: abort: this is an exercise
abort: pull failed on remote abort: pull failed on remote
[255] [255]
abort with no error hint when there is a ssh problem when pulling
$ hg pull ssh://brokenrepository
pulling from ssh://brokenrepository/
remote: ssh: Could not resolve hostname brokenrepository: Name or service not known
abort: no suitable response from remote hg!
[255]
abort with configured error hint when there is a ssh problem when pulling
$ hg pull ssh://brokenrepository --config ui.ssherrorhint="Please see http://company/internalwiki/ssh.html"
pulling from ssh://brokenrepository/
remote: ssh: Could not resolve hostname brokenrepository: Name or service not known
abort: no suitable response from remote hg!
(Please see http://company/internalwiki/ssh.html)
[255]