From 189c64d8104d2e0daf23449bb55f89d2adb1db3e Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 8 Oct 2024 17:45:14 -0400 Subject: [PATCH] Remove deprecated imagemagick usage --- .devcontainer/Dockerfile | 2 +- .github/workflows/test-ruby.yml | 86 ------------------- Dockerfile | 2 - README.md | 4 +- Vagrantfile | 1 - .../dimension/software_versions_dimension.rb | 26 +----- app/models/preview_card.rb | 4 +- config/application.rb | 8 +- config/imagemagick/policy.xml | 27 ------ config/initializers/paperclip.rb | 7 -- config/initializers/vips.rb | 50 ++++++----- lib/paperclip/blurhash_transcoder.rb | 10 +-- lib/paperclip/color_extractor.rb | 13 +-- spec/models/media_attachment_spec.rb | 4 +- spec/models/preview_card_spec.rb | 6 -- 15 files changed, 35 insertions(+), 215 deletions(-) delete mode 100644 config/imagemagick/policy.xml diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 3aa0bbf7da..ed8484f5b8 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -9,7 +9,7 @@ RUN /bin/bash --login -i -c "nvm install" # Install additional OS packages RUN apt-get update && \ export DEBIAN_FRONTEND=noninteractive && \ - apt-get -y install --no-install-recommends libicu-dev libidn11-dev ffmpeg imagemagick libvips42 libpam-dev + apt-get -y install --no-install-recommends libicu-dev libidn11-dev ffmpeg libvips42 libpam-dev # Disable download prompt for Corepack ENV COREPACK_ENABLE_DOWNLOAD_PROMPT=0 diff --git a/.github/workflows/test-ruby.yml b/.github/workflows/test-ruby.yml index 770cd72a1b..72644038be 100644 --- a/.github/workflows/test-ruby.yml +++ b/.github/workflows/test-ruby.yml @@ -172,92 +172,6 @@ jobs: env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} - test-libvips: - name: Libvips tests - runs-on: ubuntu-24.04 - - needs: - - build - - services: - postgres: - image: postgres:14-alpine - env: - POSTGRES_PASSWORD: postgres - POSTGRES_USER: postgres - options: >- - --health-cmd pg_isready - --health-interval 10ms - --health-timeout 3s - --health-retries 50 - ports: - - 5432:5432 - - redis: - image: redis:7-alpine - options: >- - --health-cmd "redis-cli ping" - --health-interval 10ms - --health-timeout 3s - --health-retries 50 - ports: - - 6379:6379 - - env: - DB_HOST: localhost - DB_USER: postgres - DB_PASS: postgres - DISABLE_SIMPLECOV: ${{ matrix.ruby-version != '.ruby-version' }} - RAILS_ENV: test - ALLOW_NOPAM: true - PAM_ENABLED: true - PAM_DEFAULT_SERVICE: pam_test - PAM_CONTROLLED_SERVICE: pam_test_controlled - OIDC_ENABLED: true - OIDC_SCOPE: read - SAML_ENABLED: true - CAS_ENABLED: true - BUNDLE_WITH: 'pam_authentication test' - GITHUB_RSPEC: ${{ matrix.ruby-version == '.ruby-version' && github.event.pull_request && 'true' }} - MASTODON_USE_LIBVIPS: true - - strategy: - fail-fast: false - matrix: - ruby-version: - - '3.2' - - '.ruby-version' - steps: - - uses: actions/checkout@v4 - - - uses: actions/download-artifact@v4 - with: - path: './' - name: ${{ github.sha }} - - - name: Expand archived asset artifacts - run: | - tar xvzf artifacts.tar.gz - - - name: Set up Ruby environment - uses: ./.github/actions/setup-ruby - with: - ruby-version: ${{ matrix.ruby-version}} - additional-system-dependencies: ffmpeg libpam-dev - - - name: Load database schema - run: './bin/rails db:create db:schema:load db:seed' - - - run: bin/rspec --tag attachment_processing - - - name: Upload coverage reports to Codecov - if: matrix.ruby-version == '.ruby-version' - uses: codecov/codecov-action@v4 - with: - files: coverage/lcov/mastodon.lcov - env: - CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} - test-e2e: name: End to End testing runs-on: ubuntu-latest diff --git a/Dockerfile b/Dockerfile index c91f10de0f..0e7c3cdd2d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -69,8 +69,6 @@ ENV \ PATH="${PATH}:/opt/ruby/bin:/opt/mastodon/bin" \ # Optimize jemalloc 5.x performance MALLOC_CONF="narenas:2,background_thread:true,thp:never,dirty_decay_ms:1000,muzzy_decay_ms:0" \ - # Enable libvips, should not be changed - MASTODON_USE_LIBVIPS=true \ # Sidekiq will touch tmp/sidekiq_process_has_started_and_will_begin_processing_jobs to indicate it is ready. This can be used for a readiness check in Kubernetes MASTODON_SIDEKIQ_READY_FILENAME=sidekiq_process_has_started_and_will_begin_processing_jobs diff --git a/README.md b/README.md index 17d9eefb57..05d0ea29f3 100644 --- a/README.md +++ b/README.md @@ -90,8 +90,8 @@ A **Vagrant** configuration is included for development purposes. To use it, com To set up **macOS** for native development, complete the following steps: -- Install [Homebrew] and run `brew install postgresql@14 redis imagemagick -libidn nvm` to install the required project dependencies +- Install [Homebrew] and run `brew install postgresql@14 redis vips libidn nvm` + to install the required project dependencies - Use a Ruby version manager to activate the ruby in `.ruby-version` and run `nvm use` to activate the node version from `.nvmrc` - Run the `bin/setup` script, which will install the required ruby gems and node diff --git a/Vagrantfile b/Vagrantfile index 89f5536edc..925b4192ee 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -29,7 +29,6 @@ sudo apt-get install \ libpq-dev \ libxml2-dev \ libxslt1-dev \ - imagemagick \ nodejs \ redis-server \ redis-tools \ diff --git a/app/lib/admin/metrics/dimension/software_versions_dimension.rb b/app/lib/admin/metrics/dimension/software_versions_dimension.rb index 84ffc41d74..0090989792 100644 --- a/app/lib/admin/metrics/dimension/software_versions_dimension.rb +++ b/app/lib/admin/metrics/dimension/software_versions_dimension.rb @@ -10,7 +10,7 @@ class Admin::Metrics::Dimension::SoftwareVersionsDimension < Admin::Metrics::Dim protected def perform_query - [mastodon_version, ruby_version, postgresql_version, redis_version, elasticsearch_version, libvips_version, imagemagick_version, ffmpeg_version].compact + [mastodon_version, ruby_version, postgresql_version, redis_version, elasticsearch_version, libvips_version, ffmpeg_version].compact end def mastodon_version @@ -72,8 +72,6 @@ class Admin::Metrics::Dimension::SoftwareVersionsDimension < Admin::Metrics::Dim end def libvips_version - return unless Rails.configuration.x.use_vips - { key: 'libvips', human_key: 'libvips', @@ -82,28 +80,6 @@ class Admin::Metrics::Dimension::SoftwareVersionsDimension < Admin::Metrics::Dim } end - def imagemagick_version - return if Rails.configuration.x.use_vips - - imagemagick_binary = Paperclip.options[:is_windows] ? 'magick convert' : 'convert' - - version_output = Terrapin::CommandLine.new(imagemagick_binary, '-version').run - version_match = version_output.match(/Version: ImageMagick (\S+)/)[1].strip - - return nil unless version_match - - version = version_match - - { - key: 'imagemagick', - human_key: 'ImageMagick', - value: version, - human_value: version, - } - rescue Terrapin::CommandNotFoundError, Terrapin::ExitStatusError, Paperclip::Errors::CommandNotFoundError, Paperclip::Errors::CommandFailedError - nil - end - def ffmpeg_version version_output = Terrapin::CommandLine.new(Rails.configuration.x.ffprobe_binary, '-show_program_version -v 0 -of json').run version = Oj.load(version_output, mode: :strict, symbol_keys: true).dig(:program_version, :version) diff --git a/app/models/preview_card.rb b/app/models/preview_card.rb index 7579178f83..0970df8379 100644 --- a/app/models/preview_card.rb +++ b/app/models/preview_card.rb @@ -39,7 +39,7 @@ class PreviewCard < ApplicationRecord include Attachmentable IMAGE_MIME_TYPES = ['image/jpeg', 'image/png', 'image/gif', 'image/webp'].freeze - LIMIT = Rails.configuration.x.use_vips ? 8.megabytes : 2.megabytes + LIMIT = 8.megabytes BLURHASH_OPTIONS = { x_comp: 4, @@ -63,7 +63,7 @@ class PreviewCard < ApplicationRecord belongs_to :author_account, class_name: 'Account', optional: true has_attached_file :image, - processors: [Rails.configuration.x.use_vips ? :lazy_thumbnail : :thumbnail, :blurhash_transcoder], + processors: [:lazy_thumbnail, :blurhash_transcoder], styles: ->(f) { image_styles(f) }, convert_options: { all: '-quality 90 +profile "!icc,*" +set date:modify +set date:create +set date:timestamp' }, validate_media_type: false diff --git a/config/application.rb b/config/application.rb index cfeed02e98..3eb26f2569 100644 --- a/config/application.rb +++ b/config/application.rb @@ -95,13 +95,7 @@ module Mastodon require 'mastodon/redis_configuration' ::REDIS_CONFIGURATION = Mastodon::RedisConfiguration.new - config.x.use_vips = ENV['MASTODON_USE_LIBVIPS'] == 'true' - - if config.x.use_vips - require_relative '../lib/paperclip/vips_lazy_thumbnail' - else - require_relative '../lib/paperclip/lazy_thumbnail' - end + require_relative '../lib/paperclip/vips_lazy_thumbnail' end config.x.captcha = config_for(:captcha) diff --git a/config/imagemagick/policy.xml b/config/imagemagick/policy.xml deleted file mode 100644 index e2aa202f27..0000000000 --- a/config/imagemagick/policy.xml +++ /dev/null @@ -1,27 +0,0 @@ - - - - - - - - - - - - - - - - diff --git a/config/initializers/paperclip.rb b/config/initializers/paperclip.rb index ed16d50a76..835b84b5e7 100644 --- a/config/initializers/paperclip.rb +++ b/config/initializers/paperclip.rb @@ -182,10 +182,3 @@ unless defined?(Seahorse) end end end - -# Set our ImageMagick security policy, but allow admins to override it -ENV['MAGICK_CONFIGURE_PATH'] = begin - imagemagick_config_paths = ENV.fetch('MAGICK_CONFIGURE_PATH', '').split(File::PATH_SEPARATOR) - imagemagick_config_paths << Rails.root.join('config', 'imagemagick').expand_path.to_s - imagemagick_config_paths.join(File::PATH_SEPARATOR) -end diff --git a/config/initializers/vips.rb b/config/initializers/vips.rb index a539d7035c..97e08451e9 100644 --- a/config/initializers/vips.rb +++ b/config/initializers/vips.rb @@ -1,35 +1,33 @@ # frozen_string_literal: true -if Rails.configuration.x.use_vips - ENV['VIPS_BLOCK_UNTRUSTED'] = 'true' +ENV['VIPS_BLOCK_UNTRUSTED'] = 'true' - require 'vips' +require 'vips' - unless Vips.at_least_libvips?(8, 13) - abort <<~ERROR.squish - Incompatible libvips version (#{Vips.version_string}), please install libvips >= 8.13 - ERROR - end - - Vips.block('VipsForeign', true) - - %w( - VipsForeignLoadNsgif - VipsForeignLoadJpeg - VipsForeignLoadPng - VipsForeignLoadWebp - VipsForeignLoadHeif - VipsForeignSavePng - VipsForeignSaveSpng - VipsForeignSaveJpeg - VipsForeignSaveWebp - ).each do |operation| - Vips.block(operation, false) - end - - Vips.block_untrusted(true) +unless Vips.at_least_libvips?(8, 13) + abort <<~ERROR.squish + Incompatible libvips version (#{Vips.version_string}), please install libvips >= 8.13 + ERROR end +Vips.block('VipsForeign', true) + +%w( + VipsForeignLoadNsgif + VipsForeignLoadJpeg + VipsForeignLoadPng + VipsForeignLoadWebp + VipsForeignLoadHeif + VipsForeignSavePng + VipsForeignSaveSpng + VipsForeignSaveJpeg + VipsForeignSaveWebp +).each do |operation| + Vips.block(operation, false) +end + +Vips.block_untrusted(true) + # In some places of the code, we rescue this exception, but we don't always # load libvips, so it may be an undefined constant: unless defined?(Vips) diff --git a/lib/paperclip/blurhash_transcoder.rb b/lib/paperclip/blurhash_transcoder.rb index b4ff4a12a0..e9ff1dd9dd 100644 --- a/lib/paperclip/blurhash_transcoder.rb +++ b/lib/paperclip/blurhash_transcoder.rb @@ -19,14 +19,8 @@ module Paperclip private def blurhash_params - if Rails.configuration.x.use_vips - image = Vips::Image.thumbnail(@file.path, 100) - [image.width, image.height, image.colourspace(:srgb).extract_band(0, n: 3).to_a.flatten] - else - pixels = convert(':source -depth 8 RGB:-', source: File.expand_path(@file.path)).unpack('C*') - geometry = options.fetch(:file_geometry_parser).from_file(@file) - [geometry.width, geometry.height, pixels] - end + image = Vips::Image.thumbnail(@file.path, 100) + [image.width, image.height, image.colourspace(:srgb).extract_band(0, n: 3).to_a.flatten] end end end diff --git a/lib/paperclip/color_extractor.rb b/lib/paperclip/color_extractor.rb index fba32ba4cb..37680bdd4f 100644 --- a/lib/paperclip/color_extractor.rb +++ b/lib/paperclip/color_extractor.rb @@ -10,7 +10,7 @@ module Paperclip BINS = 10 def make - background_palette, foreground_palette = Rails.configuration.x.use_vips ? palettes_from_libvips : palettes_from_imagemagick + background_palette, foreground_palette = palettes_from_libvips background_color = background_palette.first || foreground_palette.first foreground_colors = [] @@ -93,17 +93,6 @@ module Paperclip [background_palette, foreground_palette] end - def palettes_from_imagemagick - depth = 8 - - # Determine background palette by getting colors close to the image's edge only - background_palette = palette_from_im_histogram(convert(':source -alpha set -gravity Center -region 75%x75% -fill None -colorize 100% -alpha transparent +region -format %c -colors :quantity -depth :depth histogram:info:', source: File.expand_path(@file.path), quantity: 10, depth: depth), 10) - - # Determine foreground palette from the whole image - foreground_palette = palette_from_im_histogram(convert(':source -format %c -colors :quantity -depth :depth histogram:info:', source: File.expand_path(@file.path), quantity: 10, depth: depth), 10) - [background_palette, foreground_palette] - end - def downscaled_image image = Vips::Image.new_from_file(@file.path, access: :random).thumbnail_image(100) diff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb index 5f91ae0967..5d11264869 100644 --- a/spec/models/media_attachment_spec.rb +++ b/spec/models/media_attachment_spec.rb @@ -219,9 +219,7 @@ RSpec.describe MediaAttachment, :attachment_processing do describe 'ogg with cover art' do let(:media) { Fabricate(:media_attachment, file: attachment_fixture('boop.ogg')) } let(:expected_media_duration) { 0.235102 } - - # The libvips and ImageMagick implementations produce different results - let(:expected_background_color) { Rails.configuration.x.use_vips ? '#268cd9' : '#3088d4' } + let(:expected_background_color) { '#268cd9' } it 'sets correct file metadata' do expect(media) diff --git a/spec/models/preview_card_spec.rb b/spec/models/preview_card_spec.rb index 0fe76c37b0..bac29046ea 100644 --- a/spec/models/preview_card_spec.rb +++ b/spec/models/preview_card_spec.rb @@ -3,12 +3,6 @@ require 'rails_helper' RSpec.describe PreviewCard do - describe 'file size limit', :attachment_processing do - it 'is set differently whether vips is enabled or not' do - expect(described_class::LIMIT).to eq(Rails.configuration.x.use_vips ? 8.megabytes : 2.megabytes) - end - end - describe 'Validations' do describe 'url' do it { is_expected.to allow_values('http://example.host/path', 'https://example.host/path').for(:url) }