-
Notifications
You must be signed in to change notification settings - Fork 649
Fix windows build #961
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix windows build #961
Conversation
Added Windows compatible version of rust-civet. Regenerated a package-lock.json because of sha512 integrity issues.
I did not understand why the git repository were removed each time the `git::init` function was removed while they were created only once per thread (?). I also re-coded a remove_dir_all function for the Windows platform because the stdlib's one kept failing with 'Access denied' when deleting git repositories.
de8a5a3
to
e58df41
Compare
e58df41
to
4486c0e
Compare
OpenSSL is now installed via pre-compiled binaries instead of vcpkg, and libpq should be installed system-wide when asking for PostgresQL as a service. Also, cargo's local registry is now invalidated when the Cargo.lock file changes, avoiding cache size issues.
3ccfe0c
to
6d04a36
Compare
@carols10cents This PR is now ready for review.
If anyone has more information on what this code is supposed to do, can you please explain ? https://github.com/rust-lang/crates.io/blob/master/src/tests/git.rs#L26-L32 From what I understand it removes the checkout and bare folders each time the function is called but creates them only once, when the program is launched. |
Yes, that's what this function does. It's only called in the tests, for each |
I don't understand why it would delete the folders each time the function is called (at least once for each test, so ~140 calls) but only create them once. Reading Following this logic I don't understand why the folders were created in the first place ? I guess its safe to delete... |
Sorry this has taken me so long to get to :( I'm having issues building civet-sys 0.2.0 on macos, going to file an issue over there.... EDIT: bug filed: wycats/rust-civet#12 |
Instead of simply assuming that files are immediately deleted, we move them and then mark them for deletion. This makes deleting a folder slower but much more reliable. For more information, see Niall Douglas intervention at CppCon 2015
…fix-windows-build
So, I've been thinking a lot about this and I'm not sure I want this PR to be merged. However, I'd like to use this opportunity to define the platform support of the crates.io project: should we support running the application on Windows/macOS/Linux or should we only support Linux as a target and tell people to use our newly introduced docker support (#1106) to develop the application locally ? I think we need to have an answer to this question because of its consequences. If we tell people that we only officially support running the application on Linux, then maybe we shouldn't have to care about its dependency not building on macOS. However, if we tell people that we will support all three major OS, then we need to step up our CI infrastructure to make sure that we don't accidentally break the project on either Windows or macOS and that our future development will have to be compatible with all three major OS which might complicate things given that in the end, the application will probably only run on Linux when used in production. Finally, it would seem that even though the project isn't so young anymore, there hasn't been any issue about the backend not building on Windows MSVC, although it is not documented anywhere in the setup docs. Worse, it is even assumed that the backend can currently be build on Windows MSVC, although it never was possible due to its, now outdated, civetweb dependency. Which means that it might not be that much of a big deal, and it would seem that we haven't drove off any potential contributor. Moreover, with the imminent release of the Windows Subsystem for Linux, any person with Windows 10 FCU would be able to get this project running without us doing anything except requiring contributors to use the WSL. tl;dr I'm mostly worried that due to its current & past inactivity, the Windows support for the backend might not get enough contribution to justify its integration. Currently blocked on wycats/rust-civet#13. |
I'm +1 for pushing people toward docker and/or WSL. Both seem like reasonable paths towards development on Windows. |
For entirely selfish reasons, I don't want to break the ability to develop natively on macos; I prefer not to use docker :) I agree that until we get more Windows contributors, the docker support for windows is probably sufficient. I'm going to close this. Thanks for your work! |
Windows MSVC being a Tier1 platform it would seems logical that crates.io should be able to build on it.
TODO:
0.10.0
(blocked on Fix windows build wycats/rust-civet#10)PS:
I'm sorry for messing with the package-lock.json but it kept complaining about package integrity issues so I had to delete it and regenerate it in order to silence it, please forgive me. If anyone has clues about it I'll gladly try (Seems that updating npm to the latest version (v5.4.2) fixed the problem.npm cache clean
&npm cache verify
didn't helped me as much as I hoped)