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

feat: allow font install on linux #18874

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
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
6 changes: 5 additions & 1 deletion Library/Homebrew/cask/artifact/moved.rb
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ def move_back(skip: false, force: false, adopt: false, command: nil, **options)

def delete(target, force: false, successor: nil, command: nil, **_)
ohai "Removing #{self.class.english_name} '#{target}'"
raise CaskError, "Cannot remove undeletable #{self.class.english_name}." if MacOS.undeletable?(target)
raise CaskError, "Cannot remove undeletable #{self.class.english_name}." if undeletable?(target)

return unless Utils.path_occupied?(target)

Expand All @@ -196,6 +196,10 @@ def delete(target, force: false, successor: nil, command: nil, **_)
Utils.gain_permissions_remove(target, command:)
end
end

def undeletable?(target); end
end
end
end

require "extend/os/cask/artifact/moved"
25 changes: 23 additions & 2 deletions Library/Homebrew/cask/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@
T::Hash[Symbol, String],
)

sig { returns(T::Hash[Symbol, T.untyped]) }
sig { returns(T::Hash[Symbol, String]) }
def self.defaults
{
languages: LazyObject.new { MacOS.languages },
languages: LazyObject.new { ::OS::Mac.languages },
}.merge(DEFAULT_DIRS).freeze
end

Expand Down Expand Up @@ -197,6 +197,8 @@
end

DEFAULT_DIRS.each_key do |dir|
next if dir == :fontdir

define_method(dir) do
T.bind(self, Config)
explicit.fetch(dir, env.fetch(dir, default.fetch(dir)))
Expand All @@ -208,6 +210,16 @@
end
end

sig { returns(T.any(String, Pathname)) }
def fontdir
get_dir_path(:fontdir)

Check warning on line 215 in Library/Homebrew/cask/config.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/config.rb#L215

Added line #L215 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

would be good if this or any of the other lines missing test coverage could have some added

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this was actually part of my attempt to fix the DEFAULTS because the whole definition of methods based on hash keys made my head spin.

If we want to keep the hash thing, I think that would work now as well. So I can remove these methods again if you prefer.

Copy link
Member

Choose a reason for hiding this comment

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

Don't mind either way.

end

sig { params(path: String).returns(Pathname) }
def fontdir=(path)
explicit[:fontdir] = Pathname(path).expand_path

Check warning on line 220 in Library/Homebrew/cask/config.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/config.rb#L220

Added line #L220 was not covered by tests
end

sig { params(other: Config).returns(T.self_type) }
def merge(other)
self.class.new(explicit: other.explicit.merge(explicit))
Expand All @@ -221,5 +233,14 @@
explicit:,
}.to_json(*options)
end

private

sig { params(dir: Symbol).returns(T.any(String, Pathname)) }
def get_dir_path(dir)
T.cast(explicit.fetch(dir, env.fetch(dir, default.fetch(dir))), T.any(String, Pathname))

Check warning on line 241 in Library/Homebrew/cask/config.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/config.rb#L241

Added line #L241 was not covered by tests
end
end
end

require "extend/os/cask/config"
16 changes: 13 additions & 3 deletions Library/Homebrew/cask/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ def fetch(quiet: nil, timeout: nil)
satisfy_cask_and_formula_dependencies
end

sig { void }
def stage
odebug "Cask::Installer#stage"

Expand All @@ -88,6 +89,7 @@ def stage
raise e
end

sig { void }
def install
start_time = Time.now
odebug "Cask::Installer#install"
Expand Down Expand Up @@ -152,6 +154,7 @@ def check_deprecate_disable
end
end

sig { void }
def check_conflicts
return unless @cask.conflicts_with

Expand All @@ -168,6 +171,7 @@ def check_conflicts
end
end

sig { void }
def uninstall_existing_cask
return unless @cask.installed?

Expand Down Expand Up @@ -196,6 +200,7 @@ def download(quiet: nil, timeout: nil)
timeout:)
end

sig { void }
def verify_has_sha
odebug "Checking cask has checksum"
return if @cask.sha256 != :no_check
Expand All @@ -213,6 +218,12 @@ def primary_container
end
end

sig { returns(ArtifactSet) }
def artifacts
@cask.artifacts
end

sig { params(to: Pathname).void }
def extract_primary_container(to: @cask.staged_path)
odebug "Extracting primary container"

Expand Down Expand Up @@ -242,7 +253,6 @@ def extract_primary_container(to: @cask.staged_path)

sig { params(predecessor: T.nilable(Cask)).void }
def install_artifacts(predecessor: nil)
artifacts = @cask.artifacts
already_installed_artifacts = []

odebug "Installing artifacts"
Expand Down Expand Up @@ -301,6 +311,7 @@ def check_macos_requirements
raise CaskError, @cask.depends_on.macos.message(type: :cask)
end

sig { void }
def check_arch_requirements
return if @cask.depends_on.arch.nil?

Expand All @@ -316,6 +327,7 @@ def check_arch_requirements
"but you are running #{@current_arch}."
end

sig { returns(T::Array[T.untyped]) }
def cask_and_formula_dependencies
return @cask_and_formula_dependencies if @cask_and_formula_dependencies

Expand Down Expand Up @@ -489,8 +501,6 @@ def finalize_upgrade

sig { params(clear: T::Boolean, successor: T.nilable(Cask)).void }
def uninstall_artifacts(clear: false, successor: nil)
artifacts = @cask.artifacts

odebug "Uninstalling artifacts"
odebug "#{::Utils.pluralize("artifact", artifacts.length, include_count: true)} defined", artifacts

Expand Down
2 changes: 2 additions & 0 deletions Library/Homebrew/cask/quarantine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -266,3 +266,5 @@ def self.app_management_permissions_granted?(app:, command:)
end
end
end

require "extend/os/cask/quarantine"
5 changes: 5 additions & 0 deletions Library/Homebrew/extend/os/cask/artifact/moved.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# typed: strict
# frozen_string_literal: true

require "extend/os/mac/cask/artifact/moved" if OS.mac?
require "extend/os/linux/cask/artifact/moved" if OS.linux?
Copy link
Member

@dduugg dduugg Jan 9, 2025

Choose a reason for hiding this comment

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

Not something to take on in this PR, but I wonder if we should have a single require file for all of an OS's extensions, rather than introduce three more require files here. 🤔

Copy link
Member

Choose a reason for hiding this comment

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

I think this approach is maybe a bit nicer for now.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. I also realized that the requires need to come after the generic versions, due to sorbet limitations, so that would be a tricky dance to pull off anyway.

4 changes: 4 additions & 0 deletions Library/Homebrew/extend/os/cask/config.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# typed: strict
# frozen_string_literal: true

require "extend/os/linux/cask/config" if OS.linux?
4 changes: 4 additions & 0 deletions Library/Homebrew/extend/os/cask/quarantine.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# typed: strict
# frozen_string_literal: true

require "extend/os/linux/cask/quarantine" if OS.linux?
23 changes: 23 additions & 0 deletions Library/Homebrew/extend/os/linux/cask/artifact/moved.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# typed: strict
# frozen_string_literal: true

module OS
module Linux
module Cask
module Artifact
module Moved
extend T::Helpers

requires_ancestor { ::Cask::Artifact::Moved }

sig { params(target: Pathname).returns(T::Boolean) }
def undeletable?(target)
!target.parent.writable?

Check warning on line 15 in Library/Homebrew/extend/os/linux/cask/artifact/moved.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/extend/os/linux/cask/artifact/moved.rb#L15

Added line #L15 was not covered by tests
end
end
end
end
end
end

Cask::Artifact::Moved.prepend(OS::Linux::Cask::Config)
30 changes: 30 additions & 0 deletions Library/Homebrew/extend/os/linux/cask/config.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# typed: strict
# frozen_string_literal: true

require "os/linux"

module OS
module Linux
module Cask
module Config
module ClassMethods
DEFAULT_DIRS = T.let({
vst_plugindir: "~/.vst",
vst3_plugindir: "~/.vst3",
fontdir: "#{ENV.fetch("XDG_DATA_HOME", "~/.local/share")}/fonts",
appdir: "~/.config/apps",
}.freeze, T::Hash[Symbol, String])

sig { returns(T::Hash[Symbol, String]) }
def defaults
{
languages: LazyObject.new { Linux.languages },
}.merge(DEFAULT_DIRS).freeze
end
end
end
end
end
end

Cask::Config.singleton_class.prepend(OS::Linux::Cask::Config::ClassMethods)
2 changes: 2 additions & 0 deletions Library/Homebrew/extend/os/linux/cask/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ module Installer

sig { void }
def check_stanza_os_requirements
return if artifacts.all?(::Cask::Artifact::Font)

raise ::Cask::CaskError, "macOS is required for this software."
end
end
Expand Down
22 changes: 22 additions & 0 deletions Library/Homebrew/extend/os/linux/cask/quarantine.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# typed: strict
# frozen_string_literal: true

module OS
module Linux
module Cask
module Quarantine
extend T::Helpers

requires_ancestor { ::Cask::Quarantine }

sig { returns(Symbol) }
def self.check_quarantine_support = :linux

sig { returns(T::Boolean) }
def self.available? = false
end
end
end
end

Cask::Quarantine.prepend(OS::Linux::Cask::Quarantine)
25 changes: 25 additions & 0 deletions Library/Homebrew/extend/os/mac/cask/artifact/moved.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# typed: strict
# frozen_string_literal: true

require "cask/macos"

module OS
module Mac
module Cask
module Artifact
module Moved
extend T::Helpers

requires_ancestor { ::Cask::Artifact::Moved }

sig { params(target: Pathname).returns(T::Boolean) }
def undeletable?(target)
MacOS.undeletable?(target)
end
end
end
end
end
end

Cask::Artifact::Moved.prepend(OS::Mac::Cask::Artifact::Moved)
2 changes: 1 addition & 1 deletion Library/Homebrew/extend/os/mac/readall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

success = T.let(true, T::Boolean)
tap.cask_files.each do |file|
cask = Cask::CaskLoader.load(file)
cask = ::Cask::CaskLoader.load(file)

Check warning on line 23 in Library/Homebrew/extend/os/mac/readall.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/extend/os/mac/readall.rb#L23

Added line #L23 was not covered by tests

# Fine to have missing URLs for unsupported macOS
macos_req = cask.depends_on.macos
Expand Down
12 changes: 12 additions & 0 deletions Library/Homebrew/os/linux.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
raise "Loaded OS::Linux on macOS!" if OS.mac?
# rubocop:enable Homebrew/MoveToExtendOS

@languages = T.let([], T::Array[String])

# Get the OS version.
#
# @api internal
Expand Down Expand Up @@ -56,5 +58,15 @@
Version::NULL
end
end

sig { returns(T::Array[String]) }
def self.languages
return @languages if @languages.present?

os_langs = Utils.popen_read("localectl", "list-locales")
os_langs = os_langs.scan(/[^ \n"(),]+/).map { |item| item.split(".").first.tr("_", "-") }

Check warning on line 67 in Library/Homebrew/os/linux.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/os/linux.rb#L66-L67

Added lines #L66 - L67 were not covered by tests

@languages = os_langs

Check warning on line 69 in Library/Homebrew/os/linux.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/os/linux.rb#L69

Added line #L69 was not covered by tests
end
end
end
1 change: 1 addition & 0 deletions Library/Homebrew/os/mac.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ def self.preferred_perl_version
end
end

sig { returns(T::Array[String]) }
def self.languages
return @languages if @languages

Expand Down
Loading