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
F13969456: D9450.id22557.diff
Thu, Oct 17, 2:37 AM
F13965693: D9450.id22565.diff
Wed, Oct 16, 3:45 AM
Unknown Object (File)
Sep 27 2024, 1:36 PM
Unknown Object (File)
Sep 23 2024, 2:35 PM
Unknown Object (File)
Sep 14 2024, 10:59 PM
Unknown Object (File)
Sep 14 2024, 10:57 PM
Unknown Object (File)
Sep 14 2024, 10:57 PM
Unknown Object (File)
Sep 14 2024, 10:56 PM
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).