homebrew-core icon indicating copy to clipboard operation
homebrew-core copied to clipboard

llvm 15.0.1

Open BrewTestBot opened this issue 1 year ago • 10 comments

Created by brew bump


Created with brew bump-formula-pr.

BrewTestBot avatar Sep 20 '22 06:09 BrewTestBot

@Bo98 what do you think about using CLANG_RESOURCE_DIR to get rid of the minor and/or patch versions from that path, so we don't need to keep rebuilding ccls?

carlocab avatar Sep 21 '22 23:09 carlocab

I'm guessing it's like that because the contents are not API/ABI stable.

What other distros do is add a major version symlink, but keep the default returned by clang -print-resource-dir to the full versioned path. Can then tell ccls to use the symlink when building.

Bo98 avatar Sep 22 '22 04:09 Bo98

==> brew audit ccls --online --git --skip-style
==> FAILED
  ccls:
    * 0.20220729 is not a GitHub pre-release but 'ccls' is in the GitHub prerelease allowlist.

We might need to ship a pre-release again in the future, though.

Do we need a github_prerelease_allowlist_not_a_prerelease_allowlist.json?

carlocab avatar Sep 22 '22 11:09 carlocab

We might need to ship a pre-release again in the future, though.

How often does that actually happen? Do we ship every prerelease?

Sounds like it should just be removed from the allowlist for now.

Bo98 avatar Sep 22 '22 14:09 Bo98

We might need to ship a pre-release again in the future, though.

How often does that actually happen? Do we ship every prerelease?

It happened for the two or three most recent releases (though it eventually gets fixed as we've seen here).

In particular we can miss ccls updates by a couple of months because upstream marks them as pre-releases for some reason even though they are not.

Sounds like it should just be removed from the allowlist for now.

Yea, I'll remove it separately.

carlocab avatar Sep 22 '22 21:09 carlocab

This is taking too long. Will experiment with this diff locally:

diff --git a/Formula/llvm.rb b/Formula/llvm.rb
index 385d7b994bc..77521433494 100644
--- a/Formula/llvm.rb
+++ b/Formula/llvm.rb
@@ -233,8 +233,9 @@ class Llvm < Formula
 
       cflags = ENV.cflags&.split || []
       cxxflags = ENV.cxxflags&.split || []
+      stage1_targets = %w[clang llvm-profdata runtimes]
 
-      if OS.mac?
+      stage1_targets << if OS.mac?
         extra_args << "-DLLVM_ENABLE_LIBCXX=ON"
         # Prevent CMake from defaulting to `lld` when it's found next to `clang`.
         # This can be removed after CMake 3.25. See:
@@ -245,7 +246,7 @@ class Llvm < Formula
 
         # NOTE: do not enable LTO on Linux, because this creates static archives that are not portable.
         #       This is not an issue on macOS, where bottles are built and installed on the same version.
-        args << "-DLLVM_ENABLE_LTO=ON"
+        args << "-DLLVM_ENABLE_LTO=Thin"
         # LTO creates object files not recognised by Apple libtool.
         args << "-DCMAKE_LIBTOOL=#{llvmpath}/stage1/bin/llvm-libtool-darwin"
 
@@ -258,10 +259,14 @@ class Llvm < Formula
           cxxflags << "-isystem#{toolchain_path}/usr/include"
           cxxflags << "-isystem#{macos_sdk}/usr/include"
         end
+
+        "llvm-libtool-darwin"
       else
         # Make sure CMake doesn't try to pass C++-only flags to C compiler.
         extra_args << "-DCMAKE_C_COMPILER=#{ENV.cc}"
         extra_args << "-DCMAKE_CXX_COMPILER=#{ENV.cxx}"
+
+        "lld"
       end
 
       extra_args << "-DCMAKE_C_FLAGS=#{cflags.join(" ")}" unless cflags.empty?
@@ -273,7 +278,7 @@ class Llvm < Formula
       # the one we consume the data with.
       mkdir llvmpath/"stage1" do
         system "cmake", "-G", "Unix Makefiles", "..", *extra_args, *std_cmake_args
-        system "cmake", "--build", "."
+        system "cmake", "--build", ".", "--target", *stage1_targets
       end
 
       # Barring the stage where we generate the profile data, there is no benefit to
@@ -332,7 +337,7 @@ class Llvm < Formula
                         "-DCMAKE_C_FLAGS=#{instrumented_cflags.join(" ")}",
                         "-DCMAKE_CXX_FLAGS=#{instrumented_cxxflags.join(" ")}",
                         *instrumented_extra_args, *std_cmake_args
-        system "cmake", "--build", ".", "--target", "clang", "lld", "compiler-rt"
+        system "cmake", "--build", ".", "--target", "clang", "lld", "runtimes"
 
         # We run some `check-*` targets to increase profiling
         # coverage. These do not need to succeed.

carlocab avatar Sep 22 '22 23:09 carlocab

What other distros do is add a major version symlink, but keep the default returned by clang -print-resource-dir to the full versioned path. Can then tell ccls to use the symlink when building.

Fedora: https://src.fedoraproject.org/rpms/clang/c/43b5ca7f2bb9459e3c7cd55fffbf94ab68027a20 Debian: https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/blob/15/debian/libclang-common-X.Y-dev.links.in

Looka like most of the rest don't do anything. None I find change the default CLANG_RESOURCE_DIR.

Bo98 avatar Sep 23 '22 01:09 Bo98

Yea, I've decided against changing it. Will probably file an issue at ccls to ask if using a major-versioned symlink is safe.

On another note, I was looking at this recently: https://clang.llvm.org/docs/UsersManual.html#configuration-files

We can use this to point CLANG_CONFIG_FILE_SYSTEM_DIR somewhere sensible, and write the appropriate config file to handle --sysroot.

I wonder if it's worth doing that to avoid

  1. the pour_bottle? only_if: :clt_installed requirement
  2. needing to reinstall this with OS upgrades

This may need to wait for https://reviews.llvm.org/D134337.

Thoughts?

carlocab avatar Sep 23 '22 02:09 carlocab

Wasn't aware of that, seems like a nice feature. Bit unfortunate that having a user config completely disables the system config though, so I'd guess we'd have to not enable that part given how important a default sysroot is.

I wonder if it's worth doing that to avoid

  1. the pour_bottle? only_if: :clt_installed requirement
  2. needing to reinstall this with OS upgrades

I'm assuming you're talking about pairing it with some text replacement?

Bo98 avatar Sep 23 '22 03:09 Bo98

I'm assuming you're talking about pairing it with some text replacement?

Yes. If we don't want it to be too complicated, we can just do it in post_install, with maybe a caveat that says Do 'brew postinstall llvm' when you upgrade your OS version. Or make brew call post_install when needed, so any rewriting logic is contained in this formula.

carlocab avatar Sep 23 '22 03:09 carlocab

I'm guessing it's like that because the contents are not API/ABI stable.

What other distros do is add a major version symlink, but keep the default returned by clang -print-resource-dir to the full versioned path. Can then tell ccls to use the symlink when building.

Seems like upstream recommends rebuilding even with minor version bumps:

Agree. When LLVM and Clang packages have minor release upgrades, all downstream packages should be recompiled. llvm-project does try hard to not change ABI, though.

So I'll skip the symlink for now.

carlocab avatar Sep 23 '22 04:09 carlocab

I disagree with the stance "all downstream packages should be recompiled". But in terms of ccls specifically, sure.

Bo98 avatar Sep 23 '22 04:09 Bo98

Yea I don't plan to recompile everything either; just ccls.

carlocab avatar Sep 23 '22 05:09 carlocab

:shipit: @carlocab has triggered a merge.

BrewTestBot avatar Sep 26 '22 22:09 BrewTestBot