The file folding map, frequently used on macOS, had two issues:
- the means for converting it to Python didn't work
- a minor typo when copying the python code, where != became ==
With this, the rust code passes all tests on macOS.
Alphare | |
pulkit |
hg-reviewers |
The file folding map, frequently used on macOS, had two issues:
With this, the rust code passes all tests on macOS.
I'm currently doing a full test run on a case-insensitive file system. If it passes, perhaps we can change the platform check from an error to a warning?
No Linters Available |
No Unit Test Coverage |
As a minor addendum: With this change, the Rust dirstate code almost works with case folding, but has issues detecting folded directory names. This appears mostly unimplemented AFAICT.
Thanks for fixing these. The code for handling filename normalization is incomplete, and I'm pretty much assuming some subtle bugs exist for whatever bits of it are implemented.
We (read: Octobus) are planning a dirstate overhaul in the next few months that will probably have a big influence on the structure of the code, FYI. That said, the normalization code itself will still be useful and the overall logic should be similar, so feel free to have a stab at it.
I'm not a big fan of the idea of people using this version with incomplete normalization by lifting the compiler error since it may cause bugs, wrong repo states or even security issues.
To be blunt, the compiler error has nothing to do with normalisation. It just prevents anyone from trying the Rust code on any platform other than Linux. A warning would be fine, in my opinion, but an error is quite aggressive. As far as I can tell, the Rust code correctly disables itself when detecting a case-insensitive file system. Yes, this is quite common on macOS, but I happen to have almost all source code on a case-sensitive volume. I was also able to trigger the bug on FreeBSD (after disabling the error) and Linux, by creating a case-insensitive ZFS filesystem.
(Personally, I've installed an hg with Rust extensions on my Mac; I have yet to run into issues since fixing this.)
Anyway, this change has nothing to do with the error; it just fixes some bugs in the existing code.
Let's downgrade to a warning, you're right. I'm not sure people will see it, but Rust extensions are still experimental enough that it's not a big issue, I was probably being overzealous.
(Personally, I've installed an hg with Rust extensions on my Mac; I have yet to run into issues since fixing this.)
Nice!
Anyway, this change has nothing to do with the error; it just fixes some bugs in the existing code.
Yep, this patch should definitely be taken.