Page MenuHomePhabricator

Make retrieving parents for mercurial commits work consistently across platforms
ClosedPublic

Authored by richardvanvelzen on Jun 10 2014, 11:11 AM.
Tags
None
Referenced Files
F14396196: D9450.id.diff
Sun, Dec 22, 6:44 AM
Unknown Object (File)
Thu, Dec 19, 9:38 PM
Unknown Object (File)
Wed, Dec 18, 2:05 PM
Unknown Object (File)
Tue, Dec 17, 1:26 AM
Unknown Object (File)
Fri, Dec 13, 12:45 AM
Unknown Object (File)
Thu, Dec 12, 11:24 AM
Unknown Object (File)
Thu, Dec 12, 8:54 AM
Unknown Object (File)
Wed, Dec 11, 2:20 AM
Subscribers

Details

Summary

Currently the template is single quoted, but Windows only supports double quotes. This meant that the output would be like:

'aabbccddeeffaabbccddeeffaabbccddeeff0123
'

Which is clearly wrong.

This is displayed like that in Phabricator as well, which is confusing.

Test Plan

ran arc diff on a Windows machine and saw the correct behaviour.

Diff Detail

Repository
rARC Arcanist
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

richardvanvelzen retitled this revision from to Make retrieving parents for mercurial commits work consistently across platforms.
richardvanvelzen updated this object.
richardvanvelzen edited the test plan for this revision. (Show Details)
richardvanvelzen added a reviewer: epriestley.

Does this work in cmd.exe? I would expect the "\n" to need to be escaped.

That should be handled by csprintf, right?

No -- there's no way to escape a newline in a command on Windows in cmd.exe that we could find.

All the git commit -m ... stuff had to be changed to "write to a temp file; git commit -F tempfile; delete the temp file".

For example, under cmd.exe, can you write a command which echoes "a\nb" (with a literal newline)? The equivalent of this under Bash:

$ echo "a
> b"
a
b
$

We probably should "handle" this better, but I believe the only way we can handle it is by throwing an exception ("Exception: it is not possible to escape a newline on Windows...").

Windows is so awesome it doesn't need newlines. Oh wait, it sucks!

I can't test this immediately but will push an update when I'm at my windows pc.

Haha, sounds good. If I'm right and this is janky, I expect either using single quotes (so \n is not interpreted at the PHP level) or writing \\n will work; either will pass literal \n to Mercurial and cause it to print out a newline.

richardvanvelzen edited edge metadata.

Don't use the actual newline since Windows doesn't support it.

This time I was smart enough to test it with a merge commit. The result looked okay to me.

Before:

array(3) {
  [0] =>
  string(41) "'0a543949243c9ad6e92f24d018b6a0674c457eb8"
  [1] =>
  string(42) "''ce4e941d2d8befacef926e0358d743db39b386e2"
  [2] =>
  string(1) "'"
}

After:

array(2) {
  [0] =>
  string(40) "0a543949243c9ad6e92f24d018b6a0674c457eb8"
  [1] =>
  string(40) "ce4e941d2d8befacef926e0358d743db39b386e2"
}

Does something like this work instead? I like separating out the parameter...

$futures[$node] = $this->execFutureLocal(
  'parents --template %s --rev %s',
  '{node}\n',
  $node);

Separate out the template parameter

epriestley edited edge metadata.

Cool, thanks!

This revision is now accepted and ready to land.Jun 10 2014, 5:20 PM
epriestley updated this revision to Diff 22567.

Closed by commit rARCb60eaa6487dd (authored by Richard van Velzen <rvanvelzen@experty.com>, committed by @epriestley).