Skip to content

Commit

Permalink
cask/artifact: check the bundle version when using --adopt.
Browse files Browse the repository at this point in the history
This makes `--adopt` considerably faster and more useful for application
bundles by checking the bundle version before failing to adopt the
bundle.

This could be further extended by e.g. checking if auto-updates are
enabled.

While we're here, also allow `adopt` to act a bit more like `force` in
a few other places assuming this initial check passes.
  • Loading branch information
MikeMcQuaid committed Mar 14, 2024
1 parent a7d7748 commit a554366
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 11 deletions.
40 changes: 31 additions & 9 deletions Library/Homebrew/cask/artifact/moved.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,34 @@ def move(adopt: false, force: false, verbose: false, predecessor: nil, reinstall
else
if adopt
ohai "Adopting existing #{self.class.english_name} at '#{target}'"
same = command.run(
"/usr/bin/diff",
args: ["--recursive", "--brief", source, target],
verbose:,
print_stdout: verbose,
).success?

source_plist = Pathname("#{source}/Contents/Info.plist")
target_plist = Pathname("#{target}/Contents/Info.plist")
same = if source_plist.exist? &&
(source_bundle_version = Homebrew::BundleVersion.from_info_plist(source_plist)) &&

Check warning on line 59 in Library/Homebrew/cask/artifact/moved.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/artifact/moved.rb#L56-L59

Added lines #L56 - L59 were not covered by tests
target_plist.exist? &&
(target_bundle_version = Homebrew::BundleVersion.from_info_plist(target_plist))

Check warning on line 61 in Library/Homebrew/cask/artifact/moved.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/artifact/moved.rb#L61

Added line #L61 was not covered by tests
if source_bundle_version.short_version == target_bundle_version.short_version
if source_bundle_version.version == target_bundle_version.version
true
else
onoe "The bundle version of #{source} is #{source_bundle_version.version} but " \
"is #{target_bundle_version.version} for #{target}!"
false

Check warning on line 68 in Library/Homebrew/cask/artifact/moved.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/artifact/moved.rb#L68

Added line #L68 was not covered by tests
end
else
onoe "The bundle short version of #{source} is #{source_bundle_version.short_version} but " \
"is #{target_bundle_version.short_version} for #{target}!"
false

Check warning on line 73 in Library/Homebrew/cask/artifact/moved.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/artifact/moved.rb#L73

Added line #L73 was not covered by tests
end
else
command.run(
"/usr/bin/diff",
args: ["--recursive", "--brief", source, target],
verbose:,
print_stdout: verbose,
).success?
end

unless same
raise CaskError,
Expand All @@ -73,7 +95,7 @@ def move(adopt: false, force: false, verbose: false, predecessor: nil, reinstall

message = "It seems there is already #{self.class.english_article} " \
"#{self.class.english_name} at '#{target}'"
raise CaskError, "#{message}." unless force
raise CaskError, "#{message}." if !force && !adopt

opoo "#{message}; overwriting."
delete(target, force:, command:, **options)
Expand Down Expand Up @@ -120,13 +142,13 @@ def matching_artifact?(cask)
end
end

def move_back(skip: false, force: false, command: nil, **options)
def move_back(skip: false, force: false, adopt: false, command: nil, **options)
FileUtils.rm source if source.symlink? && source.dirname.join(source.readlink) == target

if Utils.path_occupied?(source)
message = "It seems there is already #{self.class.english_article} " \
"#{self.class.english_name} at '#{source}'"
raise CaskError, "#{message}." unless force
raise CaskError, "#{message}." if !force && !adopt

opoo "#{message}; overwriting."
delete(source, force:, command:, **options)
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/cask/artifact/symlinked.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def summarize_installed

private

def link(force: false, command: nil, **_options)
def link(force: false, adopt: false, command: nil, **_options)
unless source.exist?
raise CaskError,
"It seems the #{self.class.link_type_english_name.downcase} " \
Expand All @@ -54,7 +54,7 @@ def link(force: false, command: nil, **_options)
message = "It seems there is already #{self.class.english_article} " \
"#{self.class.english_name} at '#{target}'"

if force && target.symlink? &&
if (force || adopt) && target.symlink? &&

Check warning on line 57 in Library/Homebrew/cask/artifact/symlinked.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/cask/artifact/symlinked.rb#L57

Added line #L57 was not covered by tests
(target.realpath == source.realpath || target.realpath.to_s.start_with?("#{cask.caskroom_path}/"))
opoo "#{message}; overwriting."
Utils.gain_permissions_remove(target, command:)
Expand Down

0 comments on commit a554366

Please sign in to comment.