D2057: rust implementation of hg status

This revision now requires changes to proceed. baymax added a comment. baymax requested changes to this revision. There seems to have been no activities on this Diff for the past 3 Months. By policy, we are automatically moving it out of the `need-review` state. Please, move it back

D2057: rust implementation of hg status

kevincox added a comment. The latest changes are looking really good. I have a couple more comments but I didn't have time for a full review. I'll try to get more reviewed tomorrow. It seems that you still have a lot of stuff still in-flight so I'll try to slowly review the changes as I

D2057: rust implementation of hg status

yuja added a comment. In https://phab.mercurial-scm.org/D2057#46980, @Ivzhh wrote: > In https://phab.mercurial-scm.org/D2057#46726, @yuja wrote: > > > >> I think the only place where you would need to do os-specific code is when > > >> doing serialization and serialization > > >

D2057: rust implementation of hg status

Ivzhh marked 4 inline comments as done. Ivzhh added a comment. In https://phab.mercurial-scm.org/D2057#46726, @yuja wrote: > >> I think the only place where you would need to do os-specific code is when > >> doing serialization and serialization > > > > Yes, that will be feasible

D2057: rust implementation of hg status

Ivzhh marked 45 inline comments as done. Ivzhh added inline comments. INLINE COMMENTS > kevincox wrote in base85.rs:23 > `` can only hold valid utf8 data? Does it make more sense to have `&[u8]` > here for a list of bytes? It should be any &[u8], but the current cpython crate doesn't wrap for

D2057: rust implementation of hg status

Ivzhh updated this revision to Diff 7188. Ivzhh added a comment. - add revlog and mpatch facilities - add changelog parsing - add manifest parsing - path encoding for data store - add dirstate and matcher facilities - add local repository and the supporting modules - use cargo fmt

D2057: rust implementation of hg status

kevincox added a comment. Ah, I forgot to consider the python interop. Now the need for that crate makes sense. Thanks for explaining. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2057 To: Ivzhh, #hg-reviewers, kevincox Cc: quark, yuja, glandium, krbullock,

D2057: rust implementation of hg status

yuja added a comment. >> I think the only place where you would need to do os-specific code is when >> doing serialization and serialization > > Yes, that will be feasible in strictly typed language like Rust. To be clear, I meant serialization/deserialization between filesystem

D2057: rust implementation of hg status

yuja added a comment. > I think the only place where you would need to do os-specific code is when > doing serialization and serialization Yes, that will be feasible in strictly typed language like Rust. > which I think should be handled by

D2057: rust implementation of hg status

kevincox added a comment. I'm not a windows expert but it seems like the rust OsStr, Path and filesystem APIs should handle these conversions for you. I think the only place where you would need to do os-specific code is when doing serialization and serialization which I think should be

D2057: rust implementation of hg status

quark added a comment. https://crates.io/crates/local-encoding seems to be the right choice. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2057 To: Ivzhh, #hg-reviewers, kevincox Cc: quark, yuja, glandium, krbullock, indygreg, durin42, kevincox,

D2057: rust implementation of hg status

yuja added a comment. > I am looking at Mozilla's rust winapi bindings, let me see if I can directly wrap around winapi::um::fileapi::FindFirstFileA That's probably a hard way. I was thinking of

D2057: rust implementation of hg status

Ivzhh added a comment. In https://phab.mercurial-scm.org/D2057#44269, @yuja wrote: > >> Reading that page it seems to claim that filenames should be utf8, not bytes. If utf8, this is what the code does, but if it is bytes that definitely won't work. > > > > IIRC it's bytes

D2057: rust implementation of hg status

yuja added a comment. >> Reading that page it seems to claim that filenames should be utf8, not bytes. If utf8, this is what the code does, but if it is bytes that definitely won't work. > > IIRC it's bytes everyplace except Windows, where we pretend utf8 is real? It's MBCS (i.e.

D2057: rust implementation of hg status

Ivzhh added a comment. Hi everyone, Thank you for your encouragements and comments! I will follow up with all comments and update the code soon. @indygreg It is a great idea to test on Mozilla repo, actually I found several things interesting: 1. I found a bug in my code

D2057: rust implementation of hg status

kevincox added a comment. Rust has platform independent types `PathBuf` and `` for paths and `OsString`

D2057: rust implementation of hg status

indygreg added a comment. In https://phab.mercurial-scm.org/D2057#43989, @durin42 wrote: > In https://phab.mercurial-scm.org/D2057#43988, @kevincox wrote: > > > In https://phab.mercurial-scm.org/D2057#43987, @durin42 wrote: > > > > > Mercurial tries to be principled about always

D2057: rust implementation of hg status

durin42 added a comment. In https://phab.mercurial-scm.org/D2057#43988, @kevincox wrote: > In https://phab.mercurial-scm.org/D2057#43987, @durin42 wrote: > > > Mercurial tries to be principled about always treating filenames as bytes. AIUI

D2057: rust implementation of hg status

kevincox added a comment. In https://phab.mercurial-scm.org/D2057#43987, @durin42 wrote: > Mercurial tries to be principled about always treating filenames as bytes. AIUI https://www.mercurial-scm.org/wiki/WindowsUTF8Plan is still the plan of record there? Reading that page it

D2057: rust implementation of hg status

durin42 added a comment. In https://phab.mercurial-scm.org/D2057#43892, @kevincox wrote: > On a higher level, all of these code appears to be treating file names as strings. This isn't really true and will disallow some valid file names. Maybe we should stick with bytes throughout. Of

D2057: rust implementation of hg status

kevincox requested changes to this revision. kevincox added a comment. This revision now requires changes to proceed. I will try to finish the review later, but it might be quicker if you incorporate some of the changes first since a lot of them are repeated many times. Overall it looks

D2057: rust implementation of hg status

glandium added a comment. Doesn't mononoke have code to read revlogs already? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D2057 To: Ivzhh, #hg-reviewers Cc: glandium, krbullock, indygreg, durin42, kevincox, mercurial-devel

D2057: rust implementation of hg status

indygreg added a comment. First of all - wow! Thanks for writing all this code. There's definitely a lot to work with. And work with it we will! This is definitely too big to review as one commit. If you could do *any* work to split it up, it would be greatly appreciated. I'd focus on

D2057: rust implementation of hg status

Ivzhh added a comment. Hi all, Based on the discussion a few weeks ago, I come up with a solution for review and discussion. After reading the Oxidation plan, the first thought is to bypass python engine and current plugin system IFF on request. If user (maybe background checker of