-
-
Notifications
You must be signed in to change notification settings - Fork 99
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
Port to MSVC with CMake script #567
Conversation
Hey @harrysummer I'm really glad to see this contribution coming through. I haven't used Windows myself in over a decade now, but it will make a difference to the ecosystem and open this up to more use cases. Can I suggest we do a bit of rebase work with this branch? I understand it's been a work in progress, but it's hard to even review and will certainly make a mess in the history if merged as is. For example there are nearly a dozen bumps to the submodule version. This will make any future work with I'd be happy to help with this. In fact since this is a PR I can even do some of that work real quick myself if it's okay with you that I muck around in this branch. |
@alerque Do you mean squash-and-merge? I think GitHub has that feature when you are to merge a PR if want a clean history. I'm also fine if you want me to do the squash rebase if that makes code review easier. |
No I don't think this should all get squashed into one commit at merge time, having some granular history to follow how it came together is a good thing. I just think the history needs a bit of cleanup using rebase (which yes, can squash related commits into one, etc.). The idea isn't to flatten the whole thing, just to clean up the artifacts a bit. |
OK. I think I can handle with reorganizing this PR. If you think it's good, I will put the changes into 4 commits:
For libtexpdf, there will two commits:
|
That would certainly work. I was thinking even less drastic than that. For example I just did a rebase on your branch and stuck it on my fork that cleans of the commit messages, squashes a couple of redundant things, etc. If you like I can push that branch in place of the one this PR is based on as a starting point. Some further changes will probably be necessary. In particular the submodule commits might need revisiting before merging, but... |
…to corresponding win32 function. Change to guarding macro from __MINGW32__ to WIN32.
Commit clean done. |
I will have a look at this soon. One question: we have half an appveyor CI build on Windows set up already, but obviously I never got it working. Would it be easier to work with that rather than Azure Pipeline? (I know very little about either.) |
I was going to mention, if this PR sticks to Azure it should also add a commit that nukes the Appveyor bits. |
I never used Appveyor before but it looks to me Appveyor is queuing a long time before it starts to build. On the other hand, Azure Pipeline is way faster and you could have 10 parallel building jobs for free. I think this is currently the main benefit to use Azure Pipeline. In the future, I am positive that Azure Pipeline will get even better integration with Microsoft's GitHub. It is just a few clicks to set all things up. I am here to help if you need anything. One thing that could possibly be missing is setting up pull request as a triggering condition. We can add that later after CI is setup. |
OK, I have signed up for Azure pipeline and associated a Github trigger with this repository. What I need from you is the "azure-pipelines.yml" file which defines your pipeline (I can't view the details of your pipeline at the URL you mentioned above). You can send me that by mail or attach it here and I'll set it up on my account. |
Oh, duh, I just found it in your commits. Never mind. Let me try this. |
/AzurePipelines run |
Command 'run EDIT:' is not supported by Azure Pipelines. Supported commands help: Get descriptions, examples and documentation about supported commands Example: help "command_name" run: Run all pipelines or a specific pipeline for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify a pipeline to run. Example: "run" or "run pipeline_name" See additional documentation. |
I've merged most of this into devel, and added the azure pipeline YAML file into master. Let's see what that does. |
Great! I also updated the commit (removed my temporary build script and updated build badge) so that the Azure Pipeline CI is building on this PR too. |
Looks like the devel branch build failed because |
* fontconfig: Add generated headers and sources (generated from MinGW). Add Win32 compatible layer. Add CMake script. * ICU: Fix bugs in CMake script. * lua: Add CMake script (based on torch/luajit-rocks). * srlua: Change the main entry to set SILE_PATH environment variable which is needed by sile.lua.
Clean the commits again. I think it's ready to merge after the CI build passes. |
Excellent, thank you so much! |
(Found this issue from the link in the README.md) What is the status of the build on Windows? I've tried to run it with the latest VisualStudio, which includes CMake. Here's my output. I'd be happy to help with this project, I just need a bit of guidance to get started... |
This PR aims to make it possible to build SILE on Windows. The building script is written in CMake. Dependencies are fetched and built in the building process. The PR would resolve #410 and workaround #82 (by avoiding using MinGW).
Steps towards merging:
.gitmodules
to point libtexpdf to master branch in https://github.com/simoncozens/libtexpdf .README.md
.Note:
ExternalProject
commands which prefers CMake project for best integration. However, not all dependencies come with a CMake script for building. To workaround this, we are either using some unofficial git repositories for those projects, or applying a patch with CMake script and minimal neccessary changes. To summarize,SILE_PATH
before running SILE)