icecc / icecream

Distributed compiler with a central scheduler to share build load
GNU General Public License v2.0
1.6k stars 252 forks source link

Clang-cl support and various other improvements #520

Open HC-pel opened 4 years ago

HC-pel commented 4 years ago

We are using Icecream extensively at HaCon. Over the years we developed various fixes and enhancements to better suit our needs. We recently rebased those changes on the latest release and want to share them now.

First of all our general setup and constraints, which may explain the need for some of the changes below:

The changes and additions are as follows:

The pull request currently contains all the changes squashed together. We are aware this is not an ideal way to review the changes, it is the result of a difficult rebase to the latest version. We want to use this pull request to discuss which changes make sense for the upstream version of Icecream. The attached commit can be seen as a reference/preview of what exactly the changes are. Once we know which bits are relevant we can attempt to create cleaner commits separated into the individual features, changes and fixes - and probably also handled in smaller pull requests.

To note is also that we defined our own Icecream protocol versions starting at 100. The related checks will be updated to the next-lowest available official protocol version as well.

We are looking forward to your feedback!

llunak commented 4 years ago

Yes, we generally are interested in any changes that are generally usable, and indeed it should not be done as one big patch.

Comments on the parts I see in the commit:

But there are more changes in the diff and it's difficult to review this way without spending a lot of time on it, I'm not even sure which changes belong together. I suggest you start with small things that should be simple, such as the various fixes, and then continue with the more complicated ones. This way it'll be simpler for both sides to start doing something with it, and as it'll progress it'll be easier to see what belongs upstream and what is probably not worth it.

HC-pel commented 4 years ago

Sorry for the long delay. We split up the changes into smaller commits now: https://github.com/HC-pel/icecream/commits/individual We'll create separate pull requests for the various changes.

About clang-cl: We are using it for cross compiling, yes. It is a somewhat niche usage. However, with clang-cl cross compiling Visual Studio compatible binaries has become much simpler, so for projects targetting both platforms it could be very useful. I agree that there should be tests though, we'll look into that.

I do distinctly remember having to add header files into the tarball to work around compilation errors. I'm not sure if it was fixed by this specific change, however. It's possible that it was accidently reintrooduced in the rebase. I'll analyze what exactly is necessary.