[PATCH 1 of 5] extdata: add extdatasource reader

Kevin Bullock kbullock+mercurial at ringworld.org
Fri Sep 23 13:52:53 EDT 2016


> On Sep 23, 2016, at 12:47, Matt Mackall <mpm at selenic.com> wrote:
> 
> On Thu, 2016-09-22 at 18:20 -0500, Kevin Bullock wrote:
>>> 
>>> On Sep 22, 2016, at 13:21, Matt Mackall <mpm at selenic.com> wrote:
[...]
>>> +        try:
>>> +            src = util.popen(cmd)
>> Erm, don't we want to use util.popen2 or one of the other variants that use
>> subprocess instead?
> 
> The universal advantages of subprocess are overstated. For the simple task of
> reading stdout from a subprocess, util.popen is perfectly suited. If it wasn't..
> we'd fix util.popen.

Related to my reply below: if we use popen, does stderr not get captured? I.e. will the user see the stderr output in their terminal?

>> ...and maybe handle ENOENT gracefully?
> 
> We can't, because cmd is an arbitrary shell expression.

I mean that it would be nice to inform a user somehow that their arbitrary shell expression failed and what the error was (ENOENT meaning "command not found" in this case).

pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
Kevin R. Bullock



More information about the Mercurial-devel mailing list