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

vtk: drop clang 1316 workaround #209097

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 0 additions & 18 deletions Formula/v/vtk.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class Vtk < Formula

Check warning on line 1 in Formula/v/vtk.rb

View workflow job for this annotation

GitHub Actions / macOS 15-arm64

No bottle built for vtk!

vtk has unbottled dependencies, so a bottle will not be built.

Check warning on line 1 in Formula/v/vtk.rb

View workflow job for this annotation

GitHub Actions / macOS 14-arm64

`brew linkage --cached --test --strict vtk` failed on macOS Sonoma (14) on Apple Silicon!

expat

Check warning on line 1 in Formula/v/vtk.rb

View workflow job for this annotation

GitHub Actions / macOS 14-x86_64

`brew linkage --cached --test --strict vtk` failed on macOS Sonoma (14)!

expat

Check warning on line 1 in Formula/v/vtk.rb

View workflow job for this annotation

GitHub Actions / macOS 13-arm64

`brew linkage --cached --test --strict vtk` failed on macOS Ventura (13) on Apple Silicon!

expat

Check warning on line 1 in Formula/v/vtk.rb

View workflow job for this annotation

GitHub Actions / macOS 13-x86_64

`brew linkage --cached --test --strict vtk` failed on macOS Ventura (13)!

expat
desc "Toolkit for 3D computer graphics, image processing, and visualization"
homepage "https://www.vtk.org/"
url "https://www.vtk.org/files/release/9.4/VTK-9.4.1.tar.gz"
Expand Down Expand Up @@ -47,19 +47,6 @@
uses_from_macos "tcl-tk"
Copy link
Member

@carlocab carlocab Feb 27, 2025

Choose a reason for hiding this comment

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

Probably need depends_on "expat" to avoid breaking installs on macOS 15. See also Homebrew/brew#19315

Copy link
Member Author

@cho-m cho-m Feb 27, 2025

Choose a reason for hiding this comment

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

This is part of the odd situation of dependents of Python now propagating expat. Essentially will end up with dozens of formulae with brew expat linkage unless we handle this differently.

Some alternatives:

Or we just accept expat usage everywhere.

Copy link
Member

Choose a reason for hiding this comment

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

Seems fine to accept it in dependents to me. There's already linkage anyway.

Copy link
Member Author

@cho-m cho-m Feb 27, 2025

Choose a reason for hiding this comment

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

The formulae impacted (i.e. those with uses_from_macos "expat" and [email protected] in dependency tree) are:

  1. aarch64-elf-gdb
  2. afflib
  3. arm-none-eabi-gdb
  4. gdb ([email protected] for now due to gdbgui)
  5. gdcm
  6. graph-tool
  7. i386-elf-gdb
  8. opencolorio
  9. riscv64-elf-gdb
  10. votca
  11. vtk
  12. x86_64-elf-gdb

There were originally more before removing [email protected] from glib runtime dependencies.

One thing about current state is there is a higher risk of mixing expat libraries when installing a combination of Sequoia and Sonoma bottles, i.e. formula bottled/built on Sequoia may be linked to system expat but Sonoma bottle was linked to brew expat and then a project that combines these may crash from multiple expat.

Though we don't really care about this for ncurses and libxml2 dependents which has similar concerns.

Copy link
Member Author

@cho-m cho-m Feb 27, 2025

Choose a reason for hiding this comment

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

Anyway, the existing vtk bottle is still linked to system expat so we shouldn't modify dependency until new bottle is needed:

brew linkage vtk | grep expat
  /usr/lib/libexpat.1.dylib

Either on next version bump or revision bump like #207100

uses_from_macos "zlib"

on_macos do
on_arm do
if DevelopmentTools.clang_build_version == 1316
depends_on "llvm" => :build

# clang: error: unable to execute command: Segmentation fault: 11
# clang: error: clang frontend command failed due to signal (use -v to see invocation)
# Apple clang version 13.1.6 (clang-1316.0.21.2)
fails_with :clang
end
end
end

on_linux do
depends_on "gl2ps"
depends_on "libx11"
Expand All @@ -73,8 +60,6 @@
odie "Try removing netCDF workaround!" if Formula["netcdf"].stable.version > "4.9.2"
inreplace "CMake/FindNetCDF.cmake", "find_package(netCDF CONFIG QUIET)", "# \\0"

ENV.llvm_clang if DevelopmentTools.clang_build_version == 1316 && Hardware::CPU.arm?

python = "python3.13"
qml_plugin_dir = lib/"qml/VTK.#{version.major_minor}"
vtkmodules_dir = prefix/Language::Python.site_packages(python)/"vtkmodules"
Expand Down Expand Up @@ -128,9 +113,6 @@
end

test do
# Force use of Apple Clang on macOS that needs LLVM to build
ENV.clang if DevelopmentTools.clang_build_version == 1316 && Hardware::CPU.arm?

vtk_dir = lib/"cmake/vtk-#{version.major_minor}"
vtk_cmake_module = vtk_dir/"VTK-vtk-module-find-packages.cmake"
assert_match Formula["boost"].version.to_s, vtk_cmake_module.read, "VTK needs to be rebuilt against Boost!"
Expand Down
Loading