Skip to content
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

Library building for Windows compilers #1476

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open

Conversation

partouf
Copy link
Member

@partouf partouf commented Dec 7, 2024

Part of #1332

Will need to address the following:

  • Building with CMake on Windows
  • Building with MingW compilers
  • Building with MSVC compilers
  • Sending packages to Conan
  • Whatever else comes up

Will not include (things to be added later):

  • Windows builder packer image
  • Infra setup for Windows builder
  • Non-C++ stuff

@partouf
Copy link
Member Author

partouf commented Dec 7, 2024

Does not introduce a new LibraryBuilder class in order to reuse as much as possible from existing (proven) code

@partouf
Copy link
Member Author

partouf commented Dec 13, 2024

msvc builds done, now remains only conan stuff which is probably the hardest

@partouf
Copy link
Member Author

partouf commented Dec 13, 2024

TODO

  • Copy some extra files to Win SDK directory

image

@partouf
Copy link
Member Author

partouf commented Jan 9, 2025

Am at a a bit of deadlock now. Will write it down in the hopes that I miraculously think of the correct solution.

The problem:

When building the libraries for Windows, I force the recipe for conan to be a 'package install' which means it packages the install directory and you say to cmake hey install it to that directory.

This is done to avoid having headers on disk permanently like we have for most of our Linux libraries.

But the consequence of that is that the conan recipe changes, and that's replaced on the conan server for that library version.

Now, technically, CE doesn't actually use the recipes. It's only for telling conan what to zip up and upload. But if the recipe is changed, will it also say it needs reuploading for compilers that already have an upload? I need to figure this out first. I think I can check this by uploading a msvc build and then try to upload a build for a gcc that we've already done in the past. But I do need to disable force uploading before I do that.

Doing that now before continuing this writeup.

@partouf
Copy link
Member Author

partouf commented Jan 9, 2025

Ok, so changing the recipe doesn't cause issues with checking if a build has already been done.

Next issue:

Not having a directory where all the libraries are installed is obviously an issue for our ce_install build command. Because it first has to be installed before being able to build it.

I have tested the process simply by calling ce_install install first for the library and version that I was testing, but obviously this is terrible for automation.

So we need a way to temporary install the library version before building it.

The question is how, options are:

  • Have an option to install as part of the build command
  • Handle the installation outside of the python script

Either way, it will always cause installation of the library and will slow down the build even if there's nothing to build.
Then again, compared to looping over all the compilers and checking if it needs to be built, it's probably negligible.

I think it might be best to integrate it into the python script, since the installable is already available here https://github.com/compiler-explorer/infra/blob/main/bin/lib/ce_install.py#L478

Will give that a go.

@partouf
Copy link
Member Author

partouf commented Jan 10, 2025

Was pretty easy to add

@partouf partouf marked this pull request as ready for review January 12, 2025 13:48
@partouf
Copy link
Member Author

partouf commented Jan 12, 2025

continuing in #1491

@partouf partouf requested a review from mattgodbolt January 12, 2025 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant