-
Notifications
You must be signed in to change notification settings - Fork 657
Developing on Windows? #39
Comments
Around 48% of developers are on Windows according to Stack Overflow's developer surveys (https://insights.stackoverflow.com/survey/2019#technology-_-developers-primary-operating-systems) so this seems like a reasonable suggestion. A technology would need good native Windows support (not just WSL) to get widespread adoption. |
I tried to run the test suite on windows in the git bash, but failed at this:
|
After debugging a bit, I found that it fails at file paths including backslashes at this point in
The printed output is then:
With 't' not being a valid hex code. I admit I have no idea where to start searching to fix this though. Does anyone have more insight? |
I have to admit I didn't really think about Windows and figured that WSL would have bash. Is there any shell language that is supported out of the box on Linux and Windows? I really don't want to write what is essentially a list of commands to execute in Node. The overhead is unnecessary and for things like |
@MareikeTaeubner Reason for that error: We have a markup abstraction #4 for passing around formatted strings. It makes it so that we don't need to worry about ANSI formatting in those callsites, and that we can render in the future to HTML etc. It looks HTML-ish and the strings are parsed with the same semantics as JavaScript strings. Unfortunately this means that |
I've created #82 which removes unescaping markup attribute values. I can't think of a place where we'd even want that. I've kept Windows paths in mind, including UNC paths, when designing the path handling in the rest of Rome, so hopefully the rest is fine. I assume so since the vendored Rome successfully compiled trunk. |
@MareikeTaeubner Could you try from |
Sadly it still fails with the same path. :( Personally I don't have a problem with using the gitbash - you install it with git anyway. A space in the username (and therefore in the path of the homedir) seemed to be an issue on my husbands machine though. I'll have another look at that after this one. Edit: Added the entire stacktrace since it looks a little bit different than 3 days ago
|
Ah, we haven't updated the version of Rome that compiles trunk so the error is still in the vendored version. 3238667 updates it. |
Created #86 to add Windows to GitHub Actions. |
Nice! After this I was able to run the test suite! :D ...just to fail at another point ^^"
Then I tried to run it again to see whether I can reproduce it again I failed at this (and the output was a lot shorter):
At the third try, I got another error ^^":
Is there some kind of state I have to clear? |
Yeah... For some reason the test worker is timing out. I haven't managed to repro in my Windows VM but it's failing via GitHub Actions too. I've also converted all the bash scripts to Node in #86. |
Looks like the test worker timeout was due to CRLF line endings. When we run the parser tests, we assert that the returned pretty formatted AST is identical to one we have on disk. However, the one on disk has CRLF line endings while the one we've produced as LF line endings. This then causes every single parser test to fail. This then causes a diff to be computed for every single test, which on such large input is quite expensive. This triggers the test timeout. I've done a few things to improve this:
Combined with the conversion of the bash scripts to Node, I can confirm that I can run all the necessary scripts in my Windows VM and #86 is now (most likely) passing GitHub Actions which are now also enabled for Windows tests. I'll leave this issue open until I get some confirmation but I suspect this is hopefully resolved. |
I can confirm too that running with git bash works on my machine too, great work! It's a shame that git bash/WSL is required, but I suppose it's not too much of a leap - most Windows devs are going to have git bash installed at least. There should probably be something added to CONTRIBUTING.md too regarding developing on Windows right? |
@tom-sherman thanks to the node scripts, you can run it on the cmd too. I didn't check the powershell though. |
Ah I missed that bit, cool! Just need to remember to prefix with |
yes. I agree, that documenting that detail for windows devs would take out an unecessary hurdle, when starting out with the project. |
I'll close this now. Thanks for confirming! Feel free to open additional issues if anything breaks on Windows but now that we have CI and I have a working VM we should be able to fix potential issues quickly. |
Currently all of the scripts are written in bash which is possible to get working on WSL, but it may be a bridge too far for a lot of people. Maybe these scripts could be ported to Node?
The text was updated successfully, but these errors were encountered: