Fix case sensitivity and realpath issues on Windows#8
Fix case sensitivity and realpath issues on Windows#8savetheclocktower wants to merge 5 commits into
Conversation
confused-Techie
left a comment
There was a problem hiding this comment.
Love to see the love for Windows on this PR! And without a thorough review just wanted to mention there's a few console.log statements left over in spec/git-spec.js that we probably wanna remove
|
I had to pin to We can go back to |
|
Oh, I considered just declaring That means we could move back to |
We thought we'd be able to add a feature to this repo, then bump the usage of
git-utilsinppm, and then there'd be no step three. Instead, we got a headache of the sort you can see narrated in pulsar-edit/ppm#175. Once @idleberg tried to switch to our new fork ofgit-utils— withlibgit2updated and bindings rewritten in N-API — they realized that CI was failing in Windows.It was one symptom of an under-tested code path:
isRepositoryPath, which is supposed to tell us whether an arbitrary path is the root of our repository. It even brags that this is better to use than doing straight-up string comparison of paths becauseisRepositoryPathis supposed to do a lot of realpath and path-separator and case-sensitivity normalization! And yet it wasn't.To get all the specs to pass, I had to add some new strategies for verifying that two paths are equivalent (even if their strings don't match) and for getting the “real” path of a path that may not even exist. But all the tests pass now and the code is arguably even simpler.
Testing
I was able to break the specs by adding one more call to
repo.isRepositoryPath— one that checks a path produced by anode:pathmethod, rather than a path that we got fromgit-utilsitself. And it was stubborn to fix because the CI runner made me do realpath resolution… because long filenames on Windows CI runners can have their directory names truncated to Windows 3.1–style 8.3 filenames. This was annoying and I'm happy that CI is green now.