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
Unknown Object (File)
Sun, Sep 8, 11:24 AM
Unknown Object (File)
Fri, Aug 30, 10:58 PM
Unknown Object (File)
Wed, Aug 28, 10:06 PM
Unknown Object (File)
Tue, Aug 27, 6:23 AM
Unknown Object (File)
Sun, Aug 25, 7:18 PM
Unknown Object (File)
Sun, Aug 18, 4:16 PM
Unknown Object (File)
Sun, Aug 18, 2:46 PM
Unknown Object (File)
Aug 7 2024, 11:52 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

Branch
first-mercurial-commit
Lint
Lint Passed
Unit
No Test Coverage

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
51

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!