Page MenuHomePhabricator

Fix the exception when watching the first commit of a mercurial repo
ClosedPublic

Authored by epriestley on Dec 6 2013, 2:51 PM.
Tags
None
Referenced Files
F14481793: D7730.diff
Mon, Dec 30, 5:21 AM
Unknown Object (File)
Sat, Dec 21, 3:13 AM
Unknown Object (File)
Mon, Dec 9, 2:25 AM
Unknown Object (File)
Tue, Dec 3, 2:48 AM
Unknown Object (File)
Nov 28 2024, 11:22 AM
Unknown Object (File)
Nov 26 2024, 5:12 PM
Unknown Object (File)
Oct 30 2024, 2:24 PM
Unknown Object (File)
Oct 21 2024, 2:18 PM
Tokens
"Orange Medal" token, awarded by richardvanvelzen.

Details

Summary

Most checks were actually in place, but ExecFuture throws a CommandException which wasn't taken into account.

Test Plan

look at the first command and no longer saw an exception. Also, other commits worked as well.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

I think I came up with a significant simplification inline. Let me know if that works for you?

src/applications/diffusion/query/rawdiff/DiffusionMercurialRawDiffQuery.php
28

I think we can do this instead:

if ($against === null) {
  $against = '('.$commit.'^ or null)';
}

...

diff -U %d --git --rev %s --rev %s -- %s

That is, the revset (x^ or null) seems to correctly select the parent if one exists, or the empty state if one does not.

Mercurial isn't happy about the : syntax with this revset, but seems fine if we specify --rev twice.

This has the notable advantage of letting us delete all the error handling.

That indeed seems to work as well, and is a lot cleaner. :)

I do not have access to my working station until monday, so I'll just abandon this revision. Thanks!

epriestley edited reviewers, added: richardvanvelzen; removed: epriestley.

Oh, the reason we never caught this is that it works fine on my machine:

>>> orbital /INSECURE/repos/hg-private-remote $ hg diff -U 65535 --git --rev '5fbcc48003f65bfcbd88383049daf87d8bb6c1a2^':'5fbcc48003f65bfcbd88383049daf87d8bb6c1a2' -- 'FILE'
>>> orbital /INSECURE/repos/hg-private-remote $ echo $?
0
>>> orbital /INSECURE/repos/hg-private-remote $ hg --version
Mercurial Distributed SCM (version 2.2.3+87-d99d0b559084)
(see http://mercurial.selenic.com for more information)

Presumably, this must be a recent change.

epriestley updated this revision to Unknown Object (????).Dec 9 2013, 3:08 AM
  • Simpler fix.
  • Ran through the test plan locally without issues.

Resolves the issue and is almost nice to look at. :)

Cool, thanks for tracking this down!