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

Move to a custom build of Clang 6.0.1 #2869

Merged
merged 12 commits into from
May 5, 2018
Merged

Move to a custom build of Clang 6.0.1 #2869

merged 12 commits into from
May 5, 2018

Conversation

DHowett-MSFT
Copy link

@DHowett-MSFT DHowett-MSFT commented Apr 21, 2018

This pull request moves WinObjC to Clang 6.

  • All commits labelled C6Compat were changes required to build the project.
  • All commits labelled NFC are not final/not-for-checkin.
  • The directory LLVM/lib contains upstream vendored files.

Fixes #2837, #2806, #1647, #405, #2341, #1140, #160.
Closes #1769.


This change is Reviewable

@DHowett-MSFT DHowett-MSFT requested a review from MSFTFox April 21, 2018 17:37
@DHowett-MSFT
Copy link
Author

This pull request also breaks ARM support.

@DHowett-MSFT
Copy link
Author

Fixes #2837 #2806 #1647 #405 #2341 #1140 #160.
Closes #1769.

@DHowett
Copy link
Member

DHowett commented Apr 21, 2018

TODO: Include patches in tools/msvc/patches, but don't package them.

@MSFTFox
Copy link
Member

MSFTFox commented Apr 23, 2018

Why the custom build of clang 6? I thought the intention was to move away from having our own binaries here.
Edit
I realized this is probably an intermediate step as our proposed changes get merged with official binaries. There's no reason for us to halt changes blocked on outside dependencies.

@@ -2,7 +2,8 @@
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

<PropertyGroup Label="Globals">
<TargetFramework>uap10.0</TargetFramework>
<TargetFramework>NuGet,Version=v1.0</TargetFramework>
<PackageTargetFallback>net11;net20;net35;net40;net403;net45;net451;net452;net46;net461;net462;net47;net471;netcore;netcore45;netcore451;netcore50;win8;win81;win10;sl4;sl5;wp;wp7;wp75;wp8;wp81;wpa81;uap;uap10;netstandard1.0;netstandard1.1;netstandard1.2;netstandard1.3;netstandard1.4;netstandard1.5;netstandard1.6;netstandard2.0;netcoreapp1.0;netcoreapp2.0;monoandroid;monotouch;monomac;xamarinios;xamarinmac;xamarinpsthree;xamarinpsfour;xamarinpsvita;xamarinwatchos;xamarintvos;xamarinxboxthreesixty;xamarinxboxone</PackageTargetFallback>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoah.... Quite the fallbacks here. What are the ramifications of this? Did we severely spike which packages we require to contribute to Islandwood by including all these now? I'm all for manipulating NuGet if it helps us unblock certain troublesome scenarios but a giant list of all these frameworks seems strange in the least.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exciting stuff!!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is copied from Nugetizer-3000's default template, and is required so that package target lookup doesn't fail during restore. It doesn't add any additional dependencies!

@DHowett-MSFT
Copy link
Author

We have clang changes that may never be suitable for upstreaming; our compromise is to upstream what we can and provide patches for what we can't.

@wjk
Copy link

wjk commented May 3, 2018

I have a question about this PR. I noticed that the MSVC-derived binaries in the LLVM/bin folder (mspdb*, c2.dll, etc.) have been removed. Having watched Clang's upstream development, I know that their MSVC compatibility (and, in particular, their ability to generate and manipulate PDB files) is much improved over where it was just a few years ago. Furthermore, @DHowett-MSFT's comment about "clang-6.0.0+ms" implies that we are moving to upstream(-ish) Clang instead of a heavily modified, Microsoft-specific fork. Should this PR be merged, will it mean that the Clang compiler used will be open-source (or, at least, can be replaced with a self-built Clang and not cause too much breakage)?

Even though WinObjC was never really suitable for its intended purpose IMHO, I still think it has some interesting ideas under the hood that I might want to extract and use in my own projects. (In particular, I was thinking of adding support for the Mac frameworks — AppKit et al — as well as the iOS ones, and creating a macOS-to-Windows app bridge.) Should the Microsoft-proprietary compiler be finally removed, this would greatly aid the implementation of these ideas.

Thanks for answering my question, and keep up the good work! 😄

@DHowett-MSFT
Copy link
Author

Hi @wjk!

That's a very astute observation. We're planning on upstreaming any Clang changes that make sense and switching to an upstream build at our earliest convenience. For now, though, we have patches staged for upstreaming in issue #2860, and the ones that haven't made it up will be included in this pull request.

I'm very interested in your plan to support Mac frameworks. Would you mind e-mailing me at duhowett@microsoft.com to continue this conversation?

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.

5 participants