D1581: rust: implementation of `hg`

indygreg (Gregory Szorc) phabricator at mercurial-scm.org
Fri Dec 8 02:32:31 EST 2017


indygreg marked 6 inline comments as done.
indygreg added a comment.


  I removed the standalone distribution code and cleaned things up a bit.
  
  Over 97% of the tests pass with the Rust `hg` on Linux. And the failures seem reasonable.
  
  There's a bit of work to be done around packaging, Windows support, Rust linting, etc. But I'd like to get something landed so others have something to play with and so we're not reviewing a massive patch.
  
  What I'm trying to say is "I think this is ready for a real review."

INLINE COMMENTS

> quark wrote in main.rs:120
> Probably use `PyResult<int>` since `dispatch.run` returns the exit code.

``dispatch.run()`` actually calls ``sys.exit()``. There is a bug around somewhere in this code around exit code handling. But it only presents in a few tests, which is odd. I documented that in the commit message.

> yuja wrote in main.rs:101
> Perhaps we'll need a utility function for platform-specific cstr
> conversion.
> 
> On Unix, the story is simple. We can use OsStrExt. On Windows, 
> maybe we'll have to first convert it to "wide characters" by OsStrExt
> and then call `WideCharToMultiByte` to convert it back to ANSI bytes sequence.
> 
> I have no idea if Rust stdlib provides a proper way to convert
> OsStr to Windows ANSI bytes.
> 
> https://stackoverflow.com/a/38948854
> https://msdn.microsoft.com/en-us/library/windows/desktop/dd374130(v=vs.85).aspx

I documented a potential path forward in the code. I was thinking we can address this is a follow-up because it is non-trivial to implement. And we may want to do something funky, such as inject the raw bytes into `mercurial.argv` or something. Since we can call the Windows API directly, we can actually do that now. This opens up a lot of possibilities for encoding...

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1581

To: indygreg, #hg-reviewers
Cc: yuja, quark, durin42, dlax, mercurial-devel


More information about the Mercurial-devel mailing list