This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6616

extdata: avoid crashing inside subprocess when we get a revset parse error
ClosedPublic

Authored by durin42 on Jul 8 2019, 2:05 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHGea6558db1011: extdata: avoid crashing inside subprocess when we get a revset parse error

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

durin42 created this revision.Jul 8 2019, 2:05 PM
pulkit accepted this revision.Jul 8 2019, 5:42 PM
This revision is now accepted and ready to land.Jul 8 2019, 5:42 PM
yuja added a subscriber: yuja.Jul 9 2019, 8:18 PM
if proc:
  • proc.communicate()

+ try:
+ proc.communicate()
+ except ValueError:
+ # This happens if we started iterating src and then
+ # get a parse error on a line. It should be safe to ignore.
+ pass

No idea what to do, but maybe the process would be still running if
ValueError occurred.

Revision Contents
Changeset List

PathPackages
Mmercurial/scmutil.py (7 lines)
Mtests/test-extdata.t (11 lines)

Diff 15825

mercurial/scmutil.py

k = encoding.tolocal(k) k = encoding.tolocal(k)
try: try:
data[revsingle(repo, k).rev()] = encoding.tolocal(v) data[revsingle(repo, k).rev()] = encoding.tolocal(v)
except (error.LookupError, error.RepoLookupError): except (error.LookupError, error.RepoLookupError):
pass # we ignore data for nodes that don't exist locally pass # we ignore data for nodes that don't exist locally
finally: finally:
if proc: if proc:
try:
proc.communicate() proc.communicate()
except ValueError:
# This happens if we started iterating src and then
# get a parse error on a line. It should be safe to ignore.
pass
if src: if src:
src.close() src.close()
if proc and proc.returncode != 0: if proc and proc.returncode != 0:
raise error.Abort(_("extdata command '%s' failed: %s") raise error.Abort(_("extdata command '%s' failed: %s")
% (cmd, procutil.explainexit(proc.returncode))) % (cmd, procutil.explainexit(proc.returncode)))
return data return data

tests/test-extdata.t

test a zero-exiting source that emits garbage to confuse the revset parser test a zero-exiting source that emits garbage to confuse the revset parser
$ cat > badparse.txt <<'EOF' $ cat > badparse.txt <<'EOF'
> +---------------------------------------+ > +---------------------------------------+
> 9de260b1e88e > 9de260b1e88e
> EOF > EOF
BUG: this should print the revset parse error It might be nice if this error message mentioned where the bad string
$ hg log -qr "extdata(badparse)" 2>&1 | grep ValueError came from (eg line X of extdata source S), but the important thing is
ValueError: Mixing iteration and read methods would lose data that we don't crash before we can print the parse error.
$ hg log -qr "extdata(badparse)"
hg: parse error at 0: not a prefix: +
(+---------------------------------------+
^ here)
[255]
test template support: test template support:
$ hg log -r:3 -T "{node|short}{if(extdata('notes'), ' # {extdata('notes')}')}\n" $ hg log -r:3 -T "{node|short}{if(extdata('notes'), ' # {extdata('notes')}')}\n"
06254b906311 # first post 06254b906311 # first post
e8342c9a2ed1 # this is buggy :( e8342c9a2ed1 # this is buggy :(
f6ed99a58333 # this change is great! f6ed99a58333 # this change is great!
9de260b1e88e 9de260b1e88e