Run on windows #71
Run on windows #71
Conversation
Since sh does not support windows and the amount of sh that gitlint uses is actually pretty small, replace the sh use in gitlint with a custom implementation that uses pythons subprocess. * Add an implementation of sh.git that allows gitlint to work * Minimally update unit tests to match new implementation * Remove the windows check that stops gitlint running on windows After the change: * sh dependency moved to test-requirements no longer required to run gitlint * gitlint runs on windows now. * Unit tests pass on unix without sh * Unit tests fail on windows * QA tests fail on windows because they still need sh
* Update config and hooks * Update lots of tests * Use codec.open to support reading UTF-8 on windows with py27 All the changes were to do with forward vs backward slashes. Use os.path.join everywhere it's needed and add some re.escape() in to deal with the cases where a backslash formatted path gets put into a regexp. An abspath or two also required to make it all work on both platforms. ConfigParser.readfp is deprecated in py3.3 but read_file is not available in 2.7. Seems readfp does not give a warning in 3.6 so disable the warning rather than add another if block for 2.7 support
|
Hi, this is awesome - thanks for doing this.
Yes! This was intentionally done a while back to make it easier for this change to be implemented at some point in the future - glad you found that :-) I'm going to need some quality time to review and test this change myself. Given the foundational nature of this change, I'm inclined to be quite careful with accepting it without doing my own due diligence. I've been burnt before by the dozens of edge-cases that come with subprocess/shell execution (hence why I went to using In addition, I only tend to work on gitlint a couple of times a year, when I can make the time (this is a hobby project :-)). I promise I'll take a decent look at this at some point, but just a heads up that it might be a while (think months) before this actually get's merged and released. Also, wrt Travis, I've see random integration test failures in the past - I'm fairly convinced this is a Travis issue because I've never had those issues outside of Travis and retriggering usually resolves these issues. Thanks again for this PR! |
|
It seems I had a bit more work to do to support all the python versions required. I've marked this as wip while I sort those out. I'll change the title and drop a comment here when it's passing. |
The older versions of python create filename.pyc files rather than __pycache__ folders. Clean those up as well to hopefully make the build more reliable. And trigger another travis build to see if it works.
|
I think these are good now. You're right the travis tests don't seem to be 100% reliable. I had 2.7 failing and 3.6 passing. I changed a commit message and now 2.7 passed and 3.6 fails ( but works locally). I've tested 2.6, 2.7, 3.3 and 3.6 locally so fairly confident the travis failures are not real ones. |
|
I just noticed that sh is still installed by setup.py. It will be skipped on windows so it's not doing any harm but it's no longer required to run gitlint (only for dev) so it could be removed. Let me know if you prefer to remove it. |
|
Any thoughts about moving forward with this? |
jorisroovers
force-pushed
the
master
branch
2 times, most recently
from
1bb47e1 to
af74eb2
Compare
|
I just merged a branch to master that provides basic support for Windows. The relevant commits were heavily inspired by this PR, so really appreciate the work that was put in here. Thanks again! In the end, I had to rewrite things slightly to align with some changes that had happened in the meantime and to incorporate some other tweaks. In any case, I'll be pushing a new release with experimental windows support later in the week. Please use #20 or any of the other relevant issues for follow-up, I'm closing out this PR. |
This MR updates gitlint to run on windows. The unit tests also run on windows but the qa tests don't.
The amount of sh functionality used by gitlint is quite small and neatly encapsulated inside the _git method. Originally I thought I could drop in a replacement for _git but nearly all the tests mock out sh so that would have required modifying almost all of them. Replacing sh meant I could keep the unit tests largely unchanged to support the shift to windows.
The changes for windows are straight-forward, mostly in the unit tests, slashes need to be
path.joinand the encoding when reading sample files needs to be explicity specified (the commit has more details).The QA tests use much more of the sh functionality (particularly fake ttys) so I left those as is for now. Some are easy enough to adapt (see this wip commit for examples: byrney@50bc2c9) but others require significant effort or may never be runnable on windows.
If there is appetite I can do another PR to make the majority of QA run on both platforms and mark the rest to skip on windows.
Caveats
winpty(due to the way it detects terminal vs pipe)sh.pymay be confusing sinceshis still used in qa. The alternative is a big search/replace on the tests to use a new name forgitlint/sh.pyRelated Issues: #20