From 47bd896737af2b8f1be5a99bfdc4dede1133034e Mon Sep 17 00:00:00 2001 From: Iulian Onofrei Date: Tue, 13 Mar 2018 12:57:41 +0200 Subject: [PATCH 1/4] Simplified icon versioning logic It now makes sure that the "-Versioned" directory exists, instead of deleting and copying the original one. After this, it uses the original icons and only creates the versioned icons with the final version. --- .../helper/icon_versioning_helper.rb | 33 ++++++++++--------- spec/icon_versioning_action_spec.rb | 28 ++++++++++------ 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/lib/fastlane/plugin/icon_versioning/helper/icon_versioning_helper.rb b/lib/fastlane/plugin/icon_versioning/helper/icon_versioning_helper.rb index ded5bcf..aea20c4 100644 --- a/lib/fastlane/plugin/icon_versioning/helper/icon_versioning_helper.rb +++ b/lib/fastlane/plugin/icon_versioning/helper/icon_versioning_helper.rb @@ -29,13 +29,16 @@ def initialize(params) def run() versioned_appiconset_path = self.class.get_versioned_path(self.appiconset_path) - FileUtils.remove_entry(versioned_appiconset_path, force: true) - FileUtils.copy_entry(self.appiconset_path, versioned_appiconset_path) + Dir.mkdir(versioned_appiconset_path) unless Dir.exist?(versioned_appiconset_path) - Dir.glob("#{versioned_appiconset_path}/*.png").each do |icon_path| - next if self.ignored_icons_regex && !(icon_path =~ self.ignored_icons_regex).nil? + Dir.glob("#{self.appiconset_path}/*.png").each do |original_icon_path| + versioned_icon_path = self.class.get_versioned_path(original_icon_path) - version_icon(icon_path) + if self.ignored_icons_regex && !(original_icon_path =~ self.ignored_icons_regex).nil? + FileUtils.copy(original_icon_path, versioned_icon_path) + else + version_icon(original_icon_path, versioned_icon_path) + end end end @@ -43,8 +46,8 @@ def self.get_versioned_path(path) return path.gsub(/([^.]+)(\.appiconset)/, '\1-Versioned\2') end - private def version_icon(icon_path) - image = MiniMagick::Image.open(icon_path) + private def version_icon(original_icon_path, versioned_icon_path) + image = MiniMagick::Image.open(original_icon_path) width = image[:width] height = image[:height] @@ -55,14 +58,14 @@ def self.get_versioned_path(path) band_top_position = height - band_height - blurred_icon_path = suffix(icon_path, 'blurred') - mask_icon_path = suffix(icon_path, 'mask') - text_base_icon_path = suffix(icon_path, 'text_base') - text_icon_path = suffix(icon_path, 'text') - temp_icon_path = suffix(icon_path, 'temp') + blurred_icon_path = suffix(versioned_icon_path, 'blurred') + mask_icon_path = suffix(versioned_icon_path, 'mask') + text_base_icon_path = suffix(versioned_icon_path, 'text_base') + text_icon_path = suffix(versioned_icon_path, 'text') + temp_icon_path = suffix(versioned_icon_path, 'temp') MiniMagick::Tool::Convert.new do |convert| - convert << icon_path + convert << original_icon_path convert << '-blur' << "#{band_blur_radius}x#{band_blur_sigma}" convert << blurred_icon_path end @@ -94,7 +97,7 @@ def self.get_versioned_path(path) end MiniMagick::Tool::Convert.new do |convert| - convert << icon_path + convert << original_icon_path convert << blurred_icon_path convert << mask_icon_path convert << '-composite' @@ -111,7 +114,7 @@ def self.get_versioned_path(path) convert << text_icon_path convert << '-geometry' << "+0+#{band_top_position}" convert << '-composite' - convert << icon_path + convert << versioned_icon_path end File.delete(text_base_icon_path, text_icon_path, temp_icon_path) diff --git a/spec/icon_versioning_action_spec.rb b/spec/icon_versioning_action_spec.rb index 65ada9c..c2d6a1f 100644 --- a/spec/icon_versioning_action_spec.rb +++ b/spec/icon_versioning_action_spec.rb @@ -4,9 +4,17 @@ let(:configuration) { FastlaneCore::Configuration } describe '#run' do + let(:original_appiconset_path) { './spec/fixtures/Valid.appiconset' } + + before(:each) do + versioned_appiconset_path = action_helper.get_versioned_path(original_appiconset_path) + + FileUtils.remove_entry(versioned_appiconset_path, force: true) + end + it 'versions the icons in the appiconset directory' do options = { - appiconset_path: './spec/fixtures/Correct.appiconset', + appiconset_path: original_appiconset_path, text: 'test' } @@ -23,7 +31,7 @@ it 'versions all the icons in the appiconset directory' do options = { - appiconset_path: './spec/fixtures/Valid.appiconset', + appiconset_path: original_appiconset_path, text: 'test' } @@ -35,16 +43,16 @@ expect(Pathname.new(versioned_appiconset_path)).to exist - Dir.glob("#{versioned_appiconset_path}/*.png").each do |icon_path| - original_icon_path = icon_path.gsub(/-Versioned/, '') + Dir.glob("#{original_appiconset_path}/*.png").each do |original_icon_path| + versioned_icon_path = action_helper.get_versioned_path(original_icon_path) - expect(FileUtils.identical?(original_icon_path, icon_path)).to be false + expect(FileUtils.identical?(original_icon_path, versioned_icon_path)).to be false end end it 'versions all the icons in the appiconset directory except the ignored one' do options = { - appiconset_path: './spec/fixtures/Valid.appiconset', + appiconset_path: original_appiconset_path, text: 'test', ignored_icons_regex: /_ultra/ } @@ -57,12 +65,12 @@ expect(Pathname.new(versioned_appiconset_path)).to exist - Dir.glob("#{versioned_appiconset_path}/*.png").each do |icon_path| - original_icon_path = icon_path.gsub(/-Versioned/, '') + Dir.glob("#{original_appiconset_path}/*.png").each do |original_icon_path| + versioned_icon_path = action_helper.get_versioned_path(original_icon_path) - is_ignored = !(icon_path =~ options[:ignored_icons_regex]).nil? + is_ignored = !(original_icon_path =~ options[:ignored_icons_regex]).nil? - expect(FileUtils.identical?(original_icon_path, icon_path)).to eq(is_ignored) + expect(FileUtils.identical?(original_icon_path, versioned_icon_path)).to eq(is_ignored) end end end From 5f2f506b3b5a361dd3825ba10e2eca9fb3dc9d04 Mon Sep 17 00:00:00 2001 From: Iulian Onofrei Date: Tue, 13 Mar 2018 13:43:38 +0200 Subject: [PATCH 2/4] Removed superfluous checks from some tests --- spec/icon_versioning_action_spec.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/spec/icon_versioning_action_spec.rb b/spec/icon_versioning_action_spec.rb index c2d6a1f..469c922 100644 --- a/spec/icon_versioning_action_spec.rb +++ b/spec/icon_versioning_action_spec.rb @@ -39,10 +39,6 @@ action.run(config) - versioned_appiconset_path = action_helper.get_versioned_path(options[:appiconset_path]) - - expect(Pathname.new(versioned_appiconset_path)).to exist - Dir.glob("#{original_appiconset_path}/*.png").each do |original_icon_path| versioned_icon_path = action_helper.get_versioned_path(original_icon_path) @@ -61,10 +57,6 @@ action.run(config) - versioned_appiconset_path = action_helper.get_versioned_path(options[:appiconset_path]) - - expect(Pathname.new(versioned_appiconset_path)).to exist - Dir.glob("#{original_appiconset_path}/*.png").each do |original_icon_path| versioned_icon_path = action_helper.get_versioned_path(original_icon_path) From 4212de0f1bce6b334d5d5c990588fde42d6ac417 Mon Sep 17 00:00:00 2001 From: Iulian Onofrei Date: Tue, 13 Mar 2018 13:50:51 +0200 Subject: [PATCH 3/4] Fixed code style --- .../plugin/icon_versioning/helper/icon_versioning_helper.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/fastlane/plugin/icon_versioning/helper/icon_versioning_helper.rb b/lib/fastlane/plugin/icon_versioning/helper/icon_versioning_helper.rb index aea20c4..3cd3f9b 100644 --- a/lib/fastlane/plugin/icon_versioning/helper/icon_versioning_helper.rb +++ b/lib/fastlane/plugin/icon_versioning/helper/icon_versioning_helper.rb @@ -1,6 +1,7 @@ -require 'fastlane_core/ui/ui' require 'mini_magick' +require 'fastlane_core/ui/ui' + module Fastlane UI = FastlaneCore::UI unless Fastlane.const_defined?('UI') From 41ca0afbb59882912074910b0b0cf2665d1dafa0 Mon Sep 17 00:00:00 2001 From: Iulian Onofrei Date: Tue, 13 Mar 2018 14:22:25 +0200 Subject: [PATCH 4/4] Added versioned icons caching The original icons are versioned only if the respective versioned icon is missing or was externally changed. --- .rubocop.yml | 1 + .../helper/icon_versioning_helper.rb | 26 +++++++++++++++++++ spec/icon_versioning_action_spec.rb | 17 ++++++++++++ 3 files changed, 44 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index 8f38764..bf0ccba 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -175,6 +175,7 @@ Style/MethodCallWithArgsParentheses: - context - before - after + - to_not Style/DefWithParentheses: Enabled: false diff --git a/lib/fastlane/plugin/icon_versioning/helper/icon_versioning_helper.rb b/lib/fastlane/plugin/icon_versioning/helper/icon_versioning_helper.rb index 3cd3f9b..20ff8a6 100644 --- a/lib/fastlane/plugin/icon_versioning/helper/icon_versioning_helper.rb +++ b/lib/fastlane/plugin/icon_versioning/helper/icon_versioning_helper.rb @@ -1,4 +1,7 @@ +require 'digest' +require 'fileutils' require 'mini_magick' +require 'yaml' require 'fastlane_core/ui/ui' @@ -7,6 +10,8 @@ module Fastlane module Helper class IconVersioningHelper + CACHE_FILE_NAME = 'cache.yml'.freeze + attr_accessor :appiconset_path attr_accessor :text @@ -32,15 +37,36 @@ def run() Dir.mkdir(versioned_appiconset_path) unless Dir.exist?(versioned_appiconset_path) + cache_file_path = File.join(versioned_appiconset_path, CACHE_FILE_NAME) + + if File.exist?(cache_file_path) + cache = YAML.load_file(cache_file_path) + else + cache = {} + end + Dir.glob("#{self.appiconset_path}/*.png").each do |original_icon_path| versioned_icon_path = self.class.get_versioned_path(original_icon_path) + unless cache[original_icon_path].nil? + if File.exist?(versioned_icon_path) + versioned_icon_sha = Digest::SHA2.file(versioned_icon_path).hexdigest + cached_icon_sha = cache[original_icon_path] + + next if versioned_icon_sha == cached_icon_sha + end + end + if self.ignored_icons_regex && !(original_icon_path =~ self.ignored_icons_regex).nil? FileUtils.copy(original_icon_path, versioned_icon_path) else version_icon(original_icon_path, versioned_icon_path) end + + cache[original_icon_path] = Digest::SHA2.file(versioned_icon_path).hexdigest end + + File.open(cache_file_path, 'w') { |file| file.write(cache.to_yaml) } end def self.get_versioned_path(path) diff --git a/spec/icon_versioning_action_spec.rb b/spec/icon_versioning_action_spec.rb index 469c922..2e7eeac 100644 --- a/spec/icon_versioning_action_spec.rb +++ b/spec/icon_versioning_action_spec.rb @@ -65,5 +65,22 @@ expect(FileUtils.identical?(original_icon_path, versioned_icon_path)).to eq(is_ignored) end end + + it 'versions all the icons except the cached one' do + options = { + appiconset_path: original_appiconset_path, + text: 'test' + } + + config = configuration.create(action.available_options, options) + + expect_any_instance_of(action_helper).to receive(:version_icon).twice.and_call_original + + action.run(config) + + expect_any_instance_of(action_helper).to_not receive(:version_icon).and_call_original + + action.run(config) + end end end