From 844c3cc5df3b5553b4cab6b4dda50a47c440d595 Mon Sep 17 00:00:00 2001 From: Jakub Turek Date: Wed, 1 Feb 2017 09:09:22 +0100 Subject: [PATCH 01/10] Implemented dry-run feature --- bin/synx | 4 +- lib/synx.rb | 2 + lib/synx/file_manager.rb | 39 +++++++++ lib/synx/issue_registry.rb | 38 +++++++++ lib/synx/project.rb | 41 ++++++++-- lib/synx/tabber.rb | 6 +- .../project/object/abstract_object.rb | 14 ++++ .../project/object/pbx_file_reference.rb | 3 +- .../xcodeproj_ext/project/object/pbx_group.rb | 11 ++- spec/spec_helper.rb | 3 + spec/synx/file_manager_spec.rb | 45 ++++++++++ spec/synx/issue_registry_spec.rb | 82 +++++++++++++++++++ spec/synx/original_file_structure.yml | 44 ++++++++++ spec/synx/project_spec.rb | 82 ++++++++++++++++++- spec/synx/tabber_spec.rb | 6 ++ 15 files changed, 407 insertions(+), 13 deletions(-) create mode 100644 lib/synx/file_manager.rb create mode 100644 lib/synx/issue_registry.rb create mode 100644 spec/synx/file_manager_spec.rb create mode 100644 spec/synx/issue_registry_spec.rb create mode 100644 spec/synx/original_file_structure.yml diff --git a/bin/synx b/bin/synx index e37cc84..cd9de7d 100755 --- a/bin/synx +++ b/bin/synx @@ -12,6 +12,7 @@ Clamp do option "--no-sort-by-name", :flag, "disable sorting groups by name" option ["--quiet", "-q"], :flag, "silence all output" option ["--exclusion", "-e"], "EXCLUSION", "ignore an Xcode group while syncing", :multivalued => true + option ["--warn-type", "-w"], "warning|error", "show warnings or errors for files whose paths do not match the project structure. Using this option will not make any changes to the project. Use it as an Xcode build phase to alert the developer.", :attribute_name => :warn_type option ["--version", "-v"], :flag, "shows synx version" do puts "Synx #{Synx::VERSION}" exit(0) @@ -22,7 +23,8 @@ Clamp do puts "You cannot run Synx as root.".red else project = Synx::Project.open(xcodeproj_path) - project.sync(:prune => prune?, :quiet => quiet?, :no_color => no_color?, :no_default_exclusions => no_default_exclusions?, :no_sort_by_name => no_sort_by_name?, :group_exclusions => exclusion_list) + project.sync(:prune => prune?, :quiet => quiet?, :no_color => no_color?, :no_default_exclusions => no_default_exclusions?, :no_sort_by_name => no_sort_by_name?, :group_exclusions => exclusion_list, :warn => warn_type) + exit(project.exit_code) end end diff --git a/lib/synx.rb b/lib/synx.rb index f86df29..4400b5a 100644 --- a/lib/synx.rb +++ b/lib/synx.rb @@ -2,5 +2,7 @@ require "synx/version" require "synx/project" require "synx/tabber" +require "synx/file_manager" +require "synx/issue_registry" require "colorize" diff --git a/lib/synx/file_manager.rb b/lib/synx/file_manager.rb new file mode 100644 index 0000000..4bd0b7c --- /dev/null +++ b/lib/synx/file_manager.rb @@ -0,0 +1,39 @@ +require 'fileutils' + +module Synx + class FileManager + + @options = {} + + class << self + def rm_rf(list) + unless dry_run + FileUtils.rm_rf(list) + end + end + + def mv(src, dest) + unless dry_run + FileUtils.mv(src, dest) + end + end + + def reset + self.options = {} + end + + def options=(options = {}) + @options = options + end + + def options + @options + end + + def dry_run + @options.key?(:warn) + end + end + + end +end diff --git a/lib/synx/issue_registry.rb b/lib/synx/issue_registry.rb new file mode 100644 index 0000000..5854dcf --- /dev/null +++ b/lib/synx/issue_registry.rb @@ -0,0 +1,38 @@ +module Synx + class IssueRegistry + + attr_accessor :output + + def initialize + @issues = {} + @output = $stderr + end + + def add_issue(reason, basename, type = nil) + existing_issue = @issues.each_pair.select { |(name, (reason, type))| name == basename.to_s and type == :not_synchronized } + + if existing_issue.empty? + @issues[basename.to_s] = [reason, type] + end + end + + def issues + @issues.values.map(&:first).sort + end + + def issues_for_basename(partial_basename) + @issues.each_pair.select { |(basename, _)| basename.include? partial_basename.to_s }.map { |_, issue| issue.first }.sort + end + + def issues_count + @issues.size + end + + def print(type) + issues.each do |issue| + output.puts [type, issue].join(': ') + end + end + + end +end diff --git a/lib/synx/project.rb b/lib/synx/project.rb index fc21aa2..639cee5 100644 --- a/lib/synx/project.rb +++ b/lib/synx/project.rb @@ -1,4 +1,3 @@ -require 'fileutils' require 'xcodeproj' module Synx @@ -10,7 +9,7 @@ class Project < Xcodeproj::Project DEFAULT_EXCLUSIONS = %W(/Libraries /Frameworks /Products /Pods) private_constant :DEFAULT_EXCLUSIONS - attr_accessor :delayed_groups_set_path, :group_exclusions, :prune, :sort_by_name + attr_accessor :delayed_groups_set_path, :group_exclusions, :prune, :sort_by_name, :warn_type def sync(options={}) set_options(options) @@ -24,7 +23,8 @@ def sync(options={}) main_group.sort_by_name if self.sort_by_name transplant_work_project Synx::Tabber.decrease - save + print_dry_run_issues + save unless warn_type end def presync_check @@ -50,16 +50,28 @@ def set_options(options) self.group_exclusions |= options[:group_exclusions] if options[:group_exclusions] self.sort_by_name = !options[:no_sort_by_name] + self.warn_type = validated_warn_type(options) Synx::Tabber.options = options + Synx::FileManager.options = options + sync_issues_repository.output = options[:output] unless options[:output].nil? end private :set_options + def validated_warn_type(options) + if options[:warn].to_s == 'warning' or options[:warn].to_s == 'error' + options[:warn] + elsif options[:warn] + Synx::Tabber.puts "Unknown warn-type: #{options[:warn]}".red + abort + end + end + def transplant_work_project # Move the synced entries over Dir.glob(work_root_pathname + "*").each do |path| - FileUtils.rm_rf(work_pathname_to_pathname(Pathname(path))) - FileUtils.mv(path, root_pathname.to_s) + Synx::FileManager.rm_rf(work_pathname_to_pathname(Pathname(path))) + Synx::FileManager.mv(path, root_pathname.to_s) end end private :transplant_work_project @@ -74,7 +86,7 @@ def work_root_pathname else @work_root_pathname = Pathname(File.join(SYNXRONIZE_DIR, root_pathname.basename.to_s)) # Clean up any previous synx and start fresh - FileUtils.rm_rf(@work_root_pathname.to_s) if @work_root_pathname.exist? + Synx::FileManager.rm_rf(@work_root_pathname.to_s) if @work_root_pathname.exist? @work_root_pathname.mkpath @work_root_pathname end @@ -121,6 +133,21 @@ def has_object_for_pathname?(pathname) end end end + + def sync_issues_repository + @sync_issues ||= Synx::IssueRegistry.new + end + + def print_dry_run_issues + sync_issues_repository.print warn_type if warn_type + end + + def exit_code + if warn_type and sync_issues_repository.issues_count > 0 + -1 + else + 0 + end + end end end - diff --git a/lib/synx/tabber.rb b/lib/synx/tabber.rb index 53d28eb..f422058 100644 --- a/lib/synx/tabber.rb +++ b/lib/synx/tabber.rb @@ -33,7 +33,11 @@ def options def puts(str="") str = str.uncolorize if options[:no_color] - output.puts (a_single_tab * @tabbing) + str.to_s unless options[:quiet] + output.puts (a_single_tab * @tabbing) + str.to_s unless quiet + end + + def quiet + options[:quiet] or options[:warn] end def a_single_tab diff --git a/lib/synx/xcodeproj_ext/project/object/abstract_object.rb b/lib/synx/xcodeproj_ext/project/object/abstract_object.rb index 9402578..2ae92cb 100644 --- a/lib/synx/xcodeproj_ext/project/object/abstract_object.rb +++ b/lib/synx/xcodeproj_ext/project/object/abstract_object.rb @@ -41,6 +41,20 @@ def ensure_internal_consistency(group) end end + def track_sync_issues + current_relative_path = real_path.relative_path_from(project.root_pathname).to_s + synced_relative_path = work_pathname.relative_path_from(project.work_root_pathname).to_s + + if current_relative_path != synced_relative_path + issue = "#{readable_type}: #{basename} is not synchronized with file system (should be #{synced_relative_path})." + project.sync_issues_repository.add_issue(issue, basename, :not_synchronized) + end + end + + def readable_type + isa.sub('PBX', '').split(/(?=[A-Z])/).join(' ') + end + def sync(group) raise NotImplementedError end diff --git a/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb b/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb index a95a6f4..19725e6 100644 --- a/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb +++ b/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb @@ -6,7 +6,8 @@ class PBXFileReference def sync(group) if should_sync? if should_move? - FileUtils.mv(real_path.to_s, work_pathname.to_s) + track_sync_issues + Synx::FileManager.mv(real_path.to_s, work_pathname.to_s) # TODO: move out to abstract_object self.source_tree = "" self.path = work_pathname.relative_path_from(parent.work_pathname).to_s diff --git a/lib/synx/xcodeproj_ext/project/object/pbx_group.rb b/lib/synx/xcodeproj_ext/project/object/pbx_group.rb index 4b44931..5b47361 100644 --- a/lib/synx/xcodeproj_ext/project/object/pbx_group.rb +++ b/lib/synx/xcodeproj_ext/project/object/pbx_group.rb @@ -8,6 +8,7 @@ def sync(group) if excluded_from_sync? Synx::Tabber.puts "#{basename}/ (excluded)".yellow else + track_sync_issues Synx::Tabber.puts "#{basename}/".green Synx::Tabber.increase @@ -35,6 +36,8 @@ def excluded_from_sync? end def sort_by_name + before_sorting = children.to_a + children.sort! do |x, y| if x.isa == 'PBXGroup' && !(y.isa == 'PBXGroup') -1 @@ -46,6 +49,10 @@ def sort_by_name 0 end end + + if before_sorting != children.to_a + project.sync_issues_repository.add_issue "Group: #{basename} is not sorted.", real_path, :not_sorted + end end def move_entries_not_in_xcodeproj @@ -97,12 +104,14 @@ def handle_unused_file(file_pathname) is_file_to_prune = prune_file_extensions.include?(file_pathname.extname.downcase) if is_file_to_prune && project.prune + issue = "Unused file not referenced by Xcode project: #{file_pathname.basename}." + project.sync_issues_repository.add_issue(issue, file_pathname.basename, :unused) Synx::Tabber.puts "#{file_pathname.basename} (removed source/image file that is not referenced by the Xcode project)".red return elsif !project.prune || !is_file_to_prune destination = project.pathname_to_work_pathname(file_pathname.parent.realpath) destination.mkpath - FileUtils.mv(file_pathname.realpath, destination) + Synx::FileManager.mv(file_pathname.realpath, destination) if is_file_to_prune Synx::Tabber.puts "#{file_pathname.basename} (source/image file that is not referenced by the Xcode project)".yellow else diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 96d0058..2aa0aba 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,8 +3,11 @@ require 'pry' DUMMY_SYNX_PATH = File.expand_path('../dummy', __FILE__) +DUMMY_SYNX_PROJECT_PATH = File.join(DUMMY_SYNX_PATH, 'dummy.xcodeproj') +DUMMY_SYNX_PBXPROJ_PATH = File.join(DUMMY_SYNX_PROJECT_PATH, 'project.pbxproj') DUMMY_SYNX_TEST_PATH = File.expand_path('../test_dummy', __FILE__) DUMMY_SYNX_TEST_PROJECT_PATH = File.join(DUMMY_SYNX_TEST_PATH, 'dummy.xcodeproj') +DUMMY_SYNX_TEST_PBXPROJ_PATH = File.join(DUMMY_SYNX_TEST_PROJECT_PATH, 'project.pbxproj') FileUtils.rm_rf(DUMMY_SYNX_TEST_PATH) FileUtils.cp_r(DUMMY_SYNX_PATH, DUMMY_SYNX_TEST_PATH) DUMMY_SYNX_TEST_PROJECT = Synx::Project.open(DUMMY_SYNX_TEST_PROJECT_PATH) diff --git a/spec/synx/file_manager_spec.rb b/spec/synx/file_manager_spec.rb new file mode 100644 index 0000000..437cc0d --- /dev/null +++ b/spec/synx/file_manager_spec.rb @@ -0,0 +1,45 @@ +require 'spec_helper' +require 'fileutils' +require 'pathname' + +describe Synx::FileManager do + + before(:each) do + Synx::FileManager.reset + end + + describe "::rm_rf" do + before(:each) { FileUtils.stub(:rm_rf) } + + it "should delete file at given path" do + Synx::FileManager.rm_rf('/the_path') + + expect(FileUtils).to have_received(:rm_rf).with('/the_path') + end + + it "should not delete file if warn flag is set" do + Synx::FileManager.options[:warn] = 'warning' + Synx::FileManager.rm_rf('/the_path') + + expect(FileUtils).to_not have_received(:rm_rf) + end + end + + describe "::mv" do + before(:each) { FileUtils.stub('mv') } + + it "should move file from source to destination" do + Synx::FileManager.mv('/src', '/dest') + + expect(FileUtils).to have_received(:mv).with('/src', '/dest') + end + + it "should not move files if warn flag is set" do + Synx::FileManager.options[:warn] = 'error' + Synx::FileManager.mv('/src', '/dest') + + expect(FileUtils).to_not have_received(:mv) + end + end + +end diff --git a/spec/synx/issue_registry_spec.rb b/spec/synx/issue_registry_spec.rb new file mode 100644 index 0000000..369a58d --- /dev/null +++ b/spec/synx/issue_registry_spec.rb @@ -0,0 +1,82 @@ +require 'spec_helper' +require 'fileutils' +require 'pathname' + +describe Synx::IssueRegistry do + + let(:output) { StringIO.new } + let(:registry) { + registry = Synx::IssueRegistry.new + registry.output = output + registry + } + + it "should return empty output if there are no issues" do + registry.print(:warning) + + expect(output.string).to eq('') + end + + it "should sort issues alphabetically" do + registry.add_issue('The issue', Pathname('the_issue')) + registry.add_issue('An issue', Pathname('an_issue')) + + expect(registry.issues.first).to eq('An issue') + expect(registry.issues.last).to eq('The issue') + end + + it "should be able to filter issues by basename" do + registry.add_issue('Issue in the 1st directory', Pathname('the_directory_file.jpg')) + registry.add_issue('Issue in the 2nd directory', Pathname('dir_file.txt')) + registry.add_issue('Issue in the 3rd directory', Pathname('a_directory_file.png')) + + issues = registry.issues_for_basename Pathname('directory') + + expect(issues).to match_array(['Issue in the 1st directory', 'Issue in the 3rd directory']) + end + + it "should print warnings correctly" do + registry.add_issue('The issue', Pathname('first_path')) + registry.add_issue('The other issue', Pathname('second_path')) + registry.print(:warning) + + expect(output.string).to eq("warning: The issue\n" + + "warning: The other issue\n") + end + + it "should print errors correctly" do + registry.add_issue('Duplicate file', Pathname('duplicate_file')) + registry.add_issue('Missing file', Pathname('missing_file')) + registry.print(:error) + + expect(output.string).to eq("error: Duplicate file\n" + + "error: Missing file\n") + end + + it "should override unused issue with not synchronized issue" do + registry.add_issue('To be discarded', Pathname('file_basename.txt'), :unused) + registry.add_issue('Overwriting issue', Pathname('file_basename.txt'), :not_synchronized) + + issues = registry.issues_for_basename Pathname('file_basename.txt') + + expect(issues).to match_array(['Overwriting issue']) + end + + it "should not override not synchronized issue with unused issue" do + registry.add_issue('Not synchronized', Pathname('file_basename.png'), :not_synchronized) + registry.add_issue('Unused', Pathname('file_basename.png'), :unused) + + issues = registry.issues_for_basename Pathname('file_basename.png') + + expect(issues).to match_array(['Not synchronized']) + end + + it "should return correct issue count" do + registry.add_issue('First', Pathname('file_basename.m'), :not_synchronized) + registry.add_issue('Second', Pathname('file_basename2.h'), :not_synchronized) + registry.add_issue('Third', Pathname('file_basename3.swift'), :not_synchronized) + + expect(registry.issues_count).to eq(3) + end + +end diff --git a/spec/synx/original_file_structure.yml b/spec/synx/original_file_structure.yml new file mode 100644 index 0000000..9187355 --- /dev/null +++ b/spec/synx/original_file_structure.yml @@ -0,0 +1,44 @@ +dummy: + AlreadySynced: + FolderNotInXcodeProj: + AnotherFileNotInXcodeProj.h: + NSObject+abc.h: + NSObject+abc.m: + Wowwww.h: + Wowwww.m: + Core Data.xcdatamodeld: + .xccurrentversion: + Core Data.xcdatamodel: + contents: + Core Data 2.xcdatamodel: + contents: + Core Data 3.xcdatamodel: + contents: + Core Data 4.xcdatamodel: + contents: + stuff.xml: + Woot.h: + Woot.m: + data.json: + dummy-Prefix.pch: + dummy.h: + dummy.m: + en.lproj: + de.lproj: + Localizable.strings: + Localizable.strings: + FileNotInXcodeProj.h: + folderWithGroupNotLinked: + data.json: + image-not-in-xcodeproj.png: + image.png: + ManyFiles.h: + ManyFiles.m: + Wow.h: + Wow.m: +dummyTests: + dummyTests-Info.plist: + dummyTests-prefix.pch: + dummyTests.m: + en.lproj: + InfoPlist.strings: diff --git a/spec/synx/project_spec.rb b/spec/synx/project_spec.rb index 4773769..74f400c 100644 --- a/spec/synx/project_spec.rb +++ b/spec/synx/project_spec.rb @@ -29,6 +29,10 @@ def except!(*keys) describe Synx::Project do + before(:all) { + Synx::FileManager.reset + } + describe "#sync" do def verify_group_structure(group, expected_structure) @@ -94,6 +98,14 @@ def expected_group_structure YAML::load_file(File.expand_path("../expected_group_structure.yml", __FILE__)) end + def original_file_structure + YAML::load_file(File.expand_path("../original_file_structure.yml", __FILE__)) + end + + def original_group_structure + YAML::load_file(File.expand_path("../original_group_structure.yml", __FILE__)) + end + describe "with no additional options" do before(:all) do @@ -164,6 +176,48 @@ def expected_group_structure end end + describe "with warn option enabled" do + + before(:all) do + FileUtils.rm_rf(DUMMY_SYNX_TEST_PATH) + FileUtils.cp_r(DUMMY_SYNX_PATH, DUMMY_SYNX_TEST_PATH) + DUMMY_SYNX_TEST_PROJECT.sync(:warn => 'warning', :output => StringIO.new, :prune => true) + end + + it "should not modify the .pbxproj file" do + expect(FileUtils.identical?(DUMMY_SYNX_PBXPROJ_PATH, DUMMY_SYNX_TEST_PBXPROJ_PATH)).to be(true) + end + + it "should not modify files structure" do + verify_file_structure(Pathname(DUMMY_SYNX_TEST_PROJECT_PATH).parent, original_file_structure) + end + + it "should register group synchronize issues" do + group_issues = DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.issues_for_basename 'FolderWithGroupNotLinked' + + expect(group_issues).to match_array(['Group: FolderWithGroupNotLinked is not synchronized with file system (should be dummy/FolderWithGroupNotLinked).']) + end + + it "should register file synchronize issues" do + file_issues = DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.issues_for_basename 'Wowwww.m' + + expect(file_issues).to match_array(['File Reference: Wowwww.m is not synchronized with file system (should be dummy/SuchGroup/VeryChildGroup/Wowwww.m).']) + end + + it "should register unused files issues" do + unused_file_issues = DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.issues_for_basename 'image-not-in-xcodeproj.png' + + expect(unused_file_issues).to match_array(['Unused file not referenced by Xcode project: image-not-in-xcodeproj.png.']) + end + + it "should register sorting issues" do + group_sort_issues = DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.issues_for_basename 'AlreadySynced' + + expect(group_sort_issues).to match_array(['Group: AlreadySynced is not sorted.']) + end + + end + end describe "group_exclusions=" do @@ -207,7 +261,7 @@ def expected_group_structure it "should start fresh by removing any existing directory at work_root_pathname" do Pathname.any_instance.stub(:exist?).and_return(true) - expect(FileUtils).to receive(:rm_rf) + expect(Synx::FileManager).to receive(:rm_rf) DUMMY_SYNX_TEST_PROJECT.send(:work_root_pathname) end @@ -219,7 +273,7 @@ def expected_group_structure it "should be an idempotent operation but return the same value through memoization" do pathname = DUMMY_SYNX_TEST_PROJECT.send(:work_root_pathname) - expect(FileUtils).to_not receive(:rm_rf) + expect(Synx::FileManager).to_not receive(:rm_rf) expect_any_instance_of(Pathname).to_not receive(:exist?) expect_any_instance_of(Pathname).to_not receive(:mkpath) expect(DUMMY_SYNX_TEST_PROJECT.send(:work_root_pathname)).to be(pathname) @@ -237,4 +291,28 @@ def expected_group_structure expect(value).to eq(expected) end end + + describe "#exit_code" do + it "should return 0 if warn is not presented in options" do + DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.stub(:issues_count).and_return(1) + + DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new) + + expect(DUMMY_SYNX_TEST_PROJECT.exit_code).to eq(0) + end + + it "should return 0 if no issues were found" do + DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.stub(:issues_count).and_return(0) + DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new, :warn => :warning) + + expect(DUMMY_SYNX_TEST_PROJECT.exit_code).to eq(0) + end + + it "should return -1 if warn is set and issues were found" do + DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.stub(:issues_count).and_return(1) + DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new, :warn => :warning) + + expect(DUMMY_SYNX_TEST_PROJECT.exit_code).to eq(-1) + end + end end diff --git a/spec/synx/tabber_spec.rb b/spec/synx/tabber_spec.rb index a76d389..cf90186 100644 --- a/spec/synx/tabber_spec.rb +++ b/spec/synx/tabber_spec.rb @@ -63,6 +63,12 @@ expect(output.string).to eq("") end + it "should not print anything if warn is set" do + Synx::Tabber.options[:warn] = true + Synx::Tabber.puts("Hello, world.") + expect(output.string).to eq("") + end + it "should print colors if no_color is false or not present" do Synx::Tabber.puts("Hello, world.".red) expect(output.string).to eq("\e[0;31;49mHello, world.\e[0m\n") From 19d403510eb0b0f5230eaec6f318c56cb268cdbc Mon Sep 17 00:00:00 2001 From: Jakub Turek Date: Wed, 1 Feb 2017 09:40:09 +0100 Subject: [PATCH 02/10] Made error reporting prettier --- .../project/object/abstract_object.rb | 4 ++-- .../xcodeproj_ext/project/object/pbx_group.rb | 11 ++++++++++- spec/synx/project_spec.rb | 10 +++------- .../project/object/pbx_group_spec.rb | 16 +++++++++++++++- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/lib/synx/xcodeproj_ext/project/object/abstract_object.rb b/lib/synx/xcodeproj_ext/project/object/abstract_object.rb index 2ae92cb..64385fe 100644 --- a/lib/synx/xcodeproj_ext/project/object/abstract_object.rb +++ b/lib/synx/xcodeproj_ext/project/object/abstract_object.rb @@ -46,13 +46,13 @@ def track_sync_issues synced_relative_path = work_pathname.relative_path_from(project.work_root_pathname).to_s if current_relative_path != synced_relative_path - issue = "#{readable_type}: #{basename} is not synchronized with file system (should be #{synced_relative_path})." + issue = "#{readable_type} #{basename} is not synchronized with file system (current path: #{current_relative_path}, desired path: #{synced_relative_path})." project.sync_issues_repository.add_issue(issue, basename, :not_synchronized) end end def readable_type - isa.sub('PBX', '').split(/(?=[A-Z])/).join(' ') + isa.sub('PBX', '').split(/(?=[A-Z])/).join(' ').capitalize end def sync(group) diff --git a/lib/synx/xcodeproj_ext/project/object/pbx_group.rb b/lib/synx/xcodeproj_ext/project/object/pbx_group.rb index 5b47361..3079a07 100644 --- a/lib/synx/xcodeproj_ext/project/object/pbx_group.rb +++ b/lib/synx/xcodeproj_ext/project/object/pbx_group.rb @@ -51,7 +51,16 @@ def sort_by_name end if before_sorting != children.to_a - project.sync_issues_repository.add_issue "Group: #{basename} is not sorted.", real_path, :not_sorted + issue = "Group #{pretty_hierarchy_path} is not sorted alphabetically." + project.sync_issues_repository.add_issue(issue, real_path, :not_sorted) + end + end + + def pretty_hierarchy_path + if hierarchy_path.to_s.empty? + '/' + else + hierarchy_path.to_s end end diff --git a/spec/synx/project_spec.rb b/spec/synx/project_spec.rb index 74f400c..74a2644 100644 --- a/spec/synx/project_spec.rb +++ b/spec/synx/project_spec.rb @@ -102,10 +102,6 @@ def original_file_structure YAML::load_file(File.expand_path("../original_file_structure.yml", __FILE__)) end - def original_group_structure - YAML::load_file(File.expand_path("../original_group_structure.yml", __FILE__)) - end - describe "with no additional options" do before(:all) do @@ -195,13 +191,13 @@ def original_group_structure it "should register group synchronize issues" do group_issues = DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.issues_for_basename 'FolderWithGroupNotLinked' - expect(group_issues).to match_array(['Group: FolderWithGroupNotLinked is not synchronized with file system (should be dummy/FolderWithGroupNotLinked).']) + expect(group_issues).to match_array(['Group FolderWithGroupNotLinked is not synchronized with file system (current path: dummy, desired path: dummy/FolderWithGroupNotLinked).']) end it "should register file synchronize issues" do file_issues = DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.issues_for_basename 'Wowwww.m' - expect(file_issues).to match_array(['File Reference: Wowwww.m is not synchronized with file system (should be dummy/SuchGroup/VeryChildGroup/Wowwww.m).']) + expect(file_issues).to match_array(['File reference Wowwww.m is not synchronized with file system (current path: dummy/AlreadySynced/FolderNotInXcodeProj/Wowwww.m, desired path: dummy/SuchGroup/VeryChildGroup/Wowwww.m).']) end it "should register unused files issues" do @@ -213,7 +209,7 @@ def original_group_structure it "should register sorting issues" do group_sort_issues = DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.issues_for_basename 'AlreadySynced' - expect(group_sort_issues).to match_array(['Group: AlreadySynced is not sorted.']) + expect(group_sort_issues).to match_array(['Group /dummy/AlreadySynced is not sorted alphabetically.']) end end diff --git a/spec/synx/xcodeproj_ext/project/object/pbx_group_spec.rb b/spec/synx/xcodeproj_ext/project/object/pbx_group_spec.rb index 867f6cb..9e8c627 100644 --- a/spec/synx/xcodeproj_ext/project/object/pbx_group_spec.rb +++ b/spec/synx/xcodeproj_ext/project/object/pbx_group_spec.rb @@ -27,4 +27,18 @@ expect(top_group.groups_containing_forward_slash).to eq([child_1, child_1_2, child_2_2]) end end -end \ No newline at end of file + + describe "pretty_hierarchy_path" do + it "should return / in case hierarchy path is empty (top level)" do + main_group = DUMMY_SYNX_TEST_PROJECT.main_group + + expect(main_group.pretty_hierarchy_path).to eq('/') + end + + it "should return hierarchy path if it is not empty" do + dummy_group = DUMMY_SYNX_TEST_PROJECT.main_group.children.select { |g| g.basename == 'dummy' }.first + + expect(dummy_group.pretty_hierarchy_path).to eq('/dummy') + end + end +end From 7642ecceba254f3da36e766a4e55216f7e784f04 Mon Sep 17 00:00:00 2001 From: Jakub Turek Date: Wed, 1 Feb 2017 12:52:49 +0100 Subject: [PATCH 03/10] Refactored FileManager to a local scope instead of global --- lib/synx/file_manager.rb | 42 +++++++++---------- lib/synx/project.rb | 14 ++++--- .../project/object/abstract_object.rb | 4 ++ .../project/object/pbx_file_reference.rb | 2 +- .../xcodeproj_ext/project/object/pbx_group.rb | 2 +- spec/synx/file_manager_spec.rb | 20 ++++----- spec/synx/project_spec.rb | 8 +--- 7 files changed, 46 insertions(+), 46 deletions(-) diff --git a/lib/synx/file_manager.rb b/lib/synx/file_manager.rb index 4bd0b7c..7d24c62 100644 --- a/lib/synx/file_manager.rb +++ b/lib/synx/file_manager.rb @@ -3,36 +3,32 @@ module Synx class FileManager - @options = {} - - class << self - def rm_rf(list) - unless dry_run - FileUtils.rm_rf(list) - end - end + def initialize + @options = {} + end - def mv(src, dest) - unless dry_run - FileUtils.mv(src, dest) - end + def rm_rf(list) + unless dry_run + FileUtils.rm_rf(list) end + end - def reset - self.options = {} + def mv(src, dest) + unless dry_run + FileUtils.mv(src, dest) end + end - def options=(options = {}) - @options = options - end + def options=(options = {}) + @options = options + end - def options - @options - end + def options + @options + end - def dry_run - @options.key?(:warn) - end + def dry_run + @options.key?(:warn) end end diff --git a/lib/synx/project.rb b/lib/synx/project.rb index 639cee5..87d2c3c 100644 --- a/lib/synx/project.rb +++ b/lib/synx/project.rb @@ -9,7 +9,7 @@ class Project < Xcodeproj::Project DEFAULT_EXCLUSIONS = %W(/Libraries /Frameworks /Products /Pods) private_constant :DEFAULT_EXCLUSIONS - attr_accessor :delayed_groups_set_path, :group_exclusions, :prune, :sort_by_name, :warn_type + attr_accessor :delayed_groups_set_path, :group_exclusions, :prune, :sort_by_name, :warn_type, :file_manager def sync(options={}) set_options(options) @@ -53,7 +53,7 @@ def set_options(options) self.warn_type = validated_warn_type(options) Synx::Tabber.options = options - Synx::FileManager.options = options + file_manager.options = options sync_issues_repository.output = options[:output] unless options[:output].nil? end private :set_options @@ -70,8 +70,8 @@ def validated_warn_type(options) def transplant_work_project # Move the synced entries over Dir.glob(work_root_pathname + "*").each do |path| - Synx::FileManager.rm_rf(work_pathname_to_pathname(Pathname(path))) - Synx::FileManager.mv(path, root_pathname.to_s) + file_manager.rm_rf(work_pathname_to_pathname(Pathname(path))) + file_manager.mv(path, root_pathname.to_s) end end private :transplant_work_project @@ -86,7 +86,7 @@ def work_root_pathname else @work_root_pathname = Pathname(File.join(SYNXRONIZE_DIR, root_pathname.basename.to_s)) # Clean up any previous synx and start fresh - Synx::FileManager.rm_rf(@work_root_pathname.to_s) if @work_root_pathname.exist? + file_manager.rm_rf(@work_root_pathname.to_s) if @work_root_pathname.exist? @work_root_pathname.mkpath @work_root_pathname end @@ -134,6 +134,10 @@ def has_object_for_pathname?(pathname) end end + def file_manager + @file_manager ||= Synx::FileManager.new + end + def sync_issues_repository @sync_issues ||= Synx::IssueRegistry.new end diff --git a/lib/synx/xcodeproj_ext/project/object/abstract_object.rb b/lib/synx/xcodeproj_ext/project/object/abstract_object.rb index 64385fe..390c36e 100644 --- a/lib/synx/xcodeproj_ext/project/object/abstract_object.rb +++ b/lib/synx/xcodeproj_ext/project/object/abstract_object.rb @@ -59,6 +59,10 @@ def sync(group) raise NotImplementedError end + def file_manager + project.file_manager + end + end end end diff --git a/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb b/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb index 19725e6..51013eb 100644 --- a/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb +++ b/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb @@ -7,7 +7,7 @@ def sync(group) if should_sync? if should_move? track_sync_issues - Synx::FileManager.mv(real_path.to_s, work_pathname.to_s) + file_manager.mv(real_path.to_s, work_pathname.to_s) # TODO: move out to abstract_object self.source_tree = "" self.path = work_pathname.relative_path_from(parent.work_pathname).to_s diff --git a/lib/synx/xcodeproj_ext/project/object/pbx_group.rb b/lib/synx/xcodeproj_ext/project/object/pbx_group.rb index 3079a07..445153e 100644 --- a/lib/synx/xcodeproj_ext/project/object/pbx_group.rb +++ b/lib/synx/xcodeproj_ext/project/object/pbx_group.rb @@ -120,7 +120,7 @@ def handle_unused_file(file_pathname) elsif !project.prune || !is_file_to_prune destination = project.pathname_to_work_pathname(file_pathname.parent.realpath) destination.mkpath - Synx::FileManager.mv(file_pathname.realpath, destination) + file_manager.mv(file_pathname.realpath, destination) if is_file_to_prune Synx::Tabber.puts "#{file_pathname.basename} (source/image file that is not referenced by the Xcode project)".yellow else diff --git a/spec/synx/file_manager_spec.rb b/spec/synx/file_manager_spec.rb index 437cc0d..1d7b925 100644 --- a/spec/synx/file_manager_spec.rb +++ b/spec/synx/file_manager_spec.rb @@ -4,39 +4,39 @@ describe Synx::FileManager do - before(:each) do - Synx::FileManager.reset + let (:file_manager) do + Synx::FileManager.new end - describe "::rm_rf" do + describe "rm_rf" do before(:each) { FileUtils.stub(:rm_rf) } it "should delete file at given path" do - Synx::FileManager.rm_rf('/the_path') + file_manager.rm_rf('/the_path') expect(FileUtils).to have_received(:rm_rf).with('/the_path') end it "should not delete file if warn flag is set" do - Synx::FileManager.options[:warn] = 'warning' - Synx::FileManager.rm_rf('/the_path') + file_manager.options[:warn] = 'warning' + file_manager.rm_rf('/the_path') expect(FileUtils).to_not have_received(:rm_rf) end end - describe "::mv" do + describe "mv" do before(:each) { FileUtils.stub('mv') } it "should move file from source to destination" do - Synx::FileManager.mv('/src', '/dest') + file_manager.mv('/src', '/dest') expect(FileUtils).to have_received(:mv).with('/src', '/dest') end it "should not move files if warn flag is set" do - Synx::FileManager.options[:warn] = 'error' - Synx::FileManager.mv('/src', '/dest') + file_manager.options[:warn] = 'error' + file_manager.mv('/src', '/dest') expect(FileUtils).to_not have_received(:mv) end diff --git a/spec/synx/project_spec.rb b/spec/synx/project_spec.rb index 74a2644..1be452a 100644 --- a/spec/synx/project_spec.rb +++ b/spec/synx/project_spec.rb @@ -29,10 +29,6 @@ def except!(*keys) describe Synx::Project do - before(:all) { - Synx::FileManager.reset - } - describe "#sync" do def verify_group_structure(group, expected_structure) @@ -257,7 +253,7 @@ def original_file_structure it "should start fresh by removing any existing directory at work_root_pathname" do Pathname.any_instance.stub(:exist?).and_return(true) - expect(Synx::FileManager).to receive(:rm_rf) + expect(DUMMY_SYNX_TEST_PROJECT.file_manager).to receive(:rm_rf) DUMMY_SYNX_TEST_PROJECT.send(:work_root_pathname) end @@ -269,7 +265,7 @@ def original_file_structure it "should be an idempotent operation but return the same value through memoization" do pathname = DUMMY_SYNX_TEST_PROJECT.send(:work_root_pathname) - expect(Synx::FileManager).to_not receive(:rm_rf) + expect(DUMMY_SYNX_TEST_PROJECT.file_manager).to_not receive(:rm_rf) expect_any_instance_of(Pathname).to_not receive(:exist?) expect_any_instance_of(Pathname).to_not receive(:mkpath) expect(DUMMY_SYNX_TEST_PROJECT.send(:work_root_pathname)).to be(pathname) From ad5b807e570c1f6736111135e6fde504784fb166 Mon Sep 17 00:00:00 2001 From: Jakub Turek Date: Wed, 1 Feb 2017 13:57:19 +0100 Subject: [PATCH 04/10] Extracted an exclusive set of constants for dry run test to not interfere with original tests --- .gitignore | 1 + spec/spec_helper.rb | 8 +++++++- spec/synx/project_spec.rb | 16 +++++++--------- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index a57c77f..f02b441 100644 --- a/.gitignore +++ b/.gitignore @@ -44,3 +44,4 @@ DerivedData # Spec spec/test_dummy/ +spec/test_dry_run_dummy/ diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2aa0aba..f865c1a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -7,10 +7,16 @@ DUMMY_SYNX_PBXPROJ_PATH = File.join(DUMMY_SYNX_PROJECT_PATH, 'project.pbxproj') DUMMY_SYNX_TEST_PATH = File.expand_path('../test_dummy', __FILE__) DUMMY_SYNX_TEST_PROJECT_PATH = File.join(DUMMY_SYNX_TEST_PATH, 'dummy.xcodeproj') -DUMMY_SYNX_TEST_PBXPROJ_PATH = File.join(DUMMY_SYNX_TEST_PROJECT_PATH, 'project.pbxproj') FileUtils.rm_rf(DUMMY_SYNX_TEST_PATH) FileUtils.cp_r(DUMMY_SYNX_PATH, DUMMY_SYNX_TEST_PATH) DUMMY_SYNX_TEST_PROJECT = Synx::Project.open(DUMMY_SYNX_TEST_PROJECT_PATH) +DUMMY_SYNX_DRY_RUN_TEST_PATH = File.expand_path('../test_dry_run_dummy', __FILE__) +DUMMY_SYNX_DRY_RUN_TEST_PROJECT_PATH = File.join(DUMMY_SYNX_DRY_RUN_TEST_PATH, 'dummy.xcodeproj') +DUMMY_SYNX_DRY_RUN_TEST_PBXPROJ_PATH = File.join(DUMMY_SYNX_DRY_RUN_TEST_PROJECT_PATH, 'project.pbxproj') +FileUtils.rm_rf(DUMMY_SYNX_DRY_RUN_TEST_PATH) +FileUtils.cp_r(DUMMY_SYNX_PATH, DUMMY_SYNX_DRY_RUN_TEST_PATH) +DUMMY_SYNX_DRY_RUN_TEST_PROJECT = Synx::Project.open(DUMMY_SYNX_DRY_RUN_TEST_PROJECT_PATH) + RSpec.configure do |config| end diff --git a/spec/synx/project_spec.rb b/spec/synx/project_spec.rb index 1be452a..05a171c 100644 --- a/spec/synx/project_spec.rb +++ b/spec/synx/project_spec.rb @@ -171,39 +171,37 @@ def original_file_structure describe "with warn option enabled" do before(:all) do - FileUtils.rm_rf(DUMMY_SYNX_TEST_PATH) - FileUtils.cp_r(DUMMY_SYNX_PATH, DUMMY_SYNX_TEST_PATH) - DUMMY_SYNX_TEST_PROJECT.sync(:warn => 'warning', :output => StringIO.new, :prune => true) + DUMMY_SYNX_DRY_RUN_TEST_PROJECT.sync(:warn => 'warning', :output => StringIO.new, :prune => true) end it "should not modify the .pbxproj file" do - expect(FileUtils.identical?(DUMMY_SYNX_PBXPROJ_PATH, DUMMY_SYNX_TEST_PBXPROJ_PATH)).to be(true) + expect(FileUtils.identical?(DUMMY_SYNX_PBXPROJ_PATH, DUMMY_SYNX_DRY_RUN_TEST_PBXPROJ_PATH)).to be(true) end it "should not modify files structure" do - verify_file_structure(Pathname(DUMMY_SYNX_TEST_PROJECT_PATH).parent, original_file_structure) + verify_file_structure(Pathname(DUMMY_SYNX_DRY_RUN_TEST_PROJECT_PATH).parent, original_file_structure) end it "should register group synchronize issues" do - group_issues = DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.issues_for_basename 'FolderWithGroupNotLinked' + group_issues = DUMMY_SYNX_DRY_RUN_TEST_PROJECT.sync_issues_repository.issues_for_basename 'FolderWithGroupNotLinked' expect(group_issues).to match_array(['Group FolderWithGroupNotLinked is not synchronized with file system (current path: dummy, desired path: dummy/FolderWithGroupNotLinked).']) end it "should register file synchronize issues" do - file_issues = DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.issues_for_basename 'Wowwww.m' + file_issues = DUMMY_SYNX_DRY_RUN_TEST_PROJECT.sync_issues_repository.issues_for_basename 'Wowwww.m' expect(file_issues).to match_array(['File reference Wowwww.m is not synchronized with file system (current path: dummy/AlreadySynced/FolderNotInXcodeProj/Wowwww.m, desired path: dummy/SuchGroup/VeryChildGroup/Wowwww.m).']) end it "should register unused files issues" do - unused_file_issues = DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.issues_for_basename 'image-not-in-xcodeproj.png' + unused_file_issues = DUMMY_SYNX_DRY_RUN_TEST_PROJECT.sync_issues_repository.issues_for_basename 'image-not-in-xcodeproj.png' expect(unused_file_issues).to match_array(['Unused file not referenced by Xcode project: image-not-in-xcodeproj.png.']) end it "should register sorting issues" do - group_sort_issues = DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.issues_for_basename 'AlreadySynced' + group_sort_issues = DUMMY_SYNX_DRY_RUN_TEST_PROJECT.sync_issues_repository.issues_for_basename 'AlreadySynced' expect(group_sort_issues).to match_array(['Group /dummy/AlreadySynced is not sorted alphabetically.']) end From 134b4700939d150728aef00809f9231eb3bb26b1 Mon Sep 17 00:00:00 2001 From: Jakub Turek Date: Wed, 1 Feb 2017 14:02:55 +0100 Subject: [PATCH 05/10] Removed unused constant --- spec/spec_helper.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f865c1a..2f6e357 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,8 +3,7 @@ require 'pry' DUMMY_SYNX_PATH = File.expand_path('../dummy', __FILE__) -DUMMY_SYNX_PROJECT_PATH = File.join(DUMMY_SYNX_PATH, 'dummy.xcodeproj') -DUMMY_SYNX_PBXPROJ_PATH = File.join(DUMMY_SYNX_PROJECT_PATH, 'project.pbxproj') +DUMMY_SYNX_PBXPROJ_PATH = File.join(DUMMY_SYNX_PATH, 'dummy.xcodeproj/project.pbxproj') DUMMY_SYNX_TEST_PATH = File.expand_path('../test_dummy', __FILE__) DUMMY_SYNX_TEST_PROJECT_PATH = File.join(DUMMY_SYNX_TEST_PATH, 'dummy.xcodeproj') FileUtils.rm_rf(DUMMY_SYNX_TEST_PATH) From 26c0c6ece96bfb5d48c4a4963e49408a8fed21f7 Mon Sep 17 00:00:00 2001 From: Jakub Turek Date: Wed, 1 Feb 2017 14:38:17 +0100 Subject: [PATCH 06/10] Made synx process return 0 code if warn-type is warning --- lib/synx/project.rb | 4 ++-- spec/synx/project_spec.rb | 13 ++++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/synx/project.rb b/lib/synx/project.rb index 87d2c3c..2f167ad 100644 --- a/lib/synx/project.rb +++ b/lib/synx/project.rb @@ -143,11 +143,11 @@ def sync_issues_repository end def print_dry_run_issues - sync_issues_repository.print warn_type if warn_type + sync_issues_repository.print(warn_type.to_s) if warn_type end def exit_code - if warn_type and sync_issues_repository.issues_count > 0 + if warn_type.to_s == 'error' and sync_issues_repository.issues_count > 0 -1 else 0 diff --git a/spec/synx/project_spec.rb b/spec/synx/project_spec.rb index 05a171c..7bcf7f9 100644 --- a/spec/synx/project_spec.rb +++ b/spec/synx/project_spec.rb @@ -293,14 +293,21 @@ def original_file_structure it "should return 0 if no issues were found" do DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.stub(:issues_count).and_return(0) - DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new, :warn => :warning) + DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new, :warn => 'warning') expect(DUMMY_SYNX_TEST_PROJECT.exit_code).to eq(0) end - it "should return -1 if warn is set and issues were found" do + it "should return 0 if warn is set to warning" do DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.stub(:issues_count).and_return(1) - DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new, :warn => :warning) + DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new, :warn => 'warning') + + expect(DUMMY_SYNX_TEST_PROJECT.exit_code).to eq(0) + end + + it "should return -1 if warn is set to error and issues were found" do + DUMMY_SYNX_TEST_PROJECT.sync_issues_repository.stub(:issues_count).and_return(1) + DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new, :warn => 'error') expect(DUMMY_SYNX_TEST_PROJECT.exit_code).to eq(-1) end From afeffad003b0d513c2715234656b01d86fb44b35 Mon Sep 17 00:00:00 2001 From: Jakub Turek Date: Wed, 1 Feb 2017 16:36:44 +0100 Subject: [PATCH 07/10] Refactored the code to avoid dry run checks all the time --- lib/synx.rb | 2 +- lib/synx/file_manager.rb | 35 -------------------------- lib/synx/file_utils.rb | 13 ++++++++++ lib/synx/project.rb | 3 +-- spec/synx/file_manager_spec.rb | 45 ---------------------------------- spec/synx/project_spec.rb | 36 +++++++++++++++++++++++++++ 6 files changed, 51 insertions(+), 83 deletions(-) delete mode 100644 lib/synx/file_manager.rb create mode 100644 lib/synx/file_utils.rb delete mode 100644 spec/synx/file_manager_spec.rb diff --git a/lib/synx.rb b/lib/synx.rb index 4400b5a..06ae36c 100644 --- a/lib/synx.rb +++ b/lib/synx.rb @@ -2,7 +2,7 @@ require "synx/version" require "synx/project" require "synx/tabber" -require "synx/file_manager" +require "synx/file_utils" require "synx/issue_registry" require "colorize" diff --git a/lib/synx/file_manager.rb b/lib/synx/file_manager.rb deleted file mode 100644 index 7d24c62..0000000 --- a/lib/synx/file_manager.rb +++ /dev/null @@ -1,35 +0,0 @@ -require 'fileutils' - -module Synx - class FileManager - - def initialize - @options = {} - end - - def rm_rf(list) - unless dry_run - FileUtils.rm_rf(list) - end - end - - def mv(src, dest) - unless dry_run - FileUtils.mv(src, dest) - end - end - - def options=(options = {}) - @options = options - end - - def options - @options - end - - def dry_run - @options.key?(:warn) - end - - end -end diff --git a/lib/synx/file_utils.rb b/lib/synx/file_utils.rb new file mode 100644 index 0000000..42598d2 --- /dev/null +++ b/lib/synx/file_utils.rb @@ -0,0 +1,13 @@ +module Synx + class BlankFileUtils + + class << self + def rm_rf(list) + end + + def mv(src, dest) + end + end + + end +end diff --git a/lib/synx/project.rb b/lib/synx/project.rb index 2f167ad..5738b1d 100644 --- a/lib/synx/project.rb +++ b/lib/synx/project.rb @@ -53,7 +53,6 @@ def set_options(options) self.warn_type = validated_warn_type(options) Synx::Tabber.options = options - file_manager.options = options sync_issues_repository.output = options[:output] unless options[:output].nil? end private :set_options @@ -135,7 +134,7 @@ def has_object_for_pathname?(pathname) end def file_manager - @file_manager ||= Synx::FileManager.new + @file_manager ||= (warn_type ? Synx::BlankFileUtils : FileUtils) end def sync_issues_repository diff --git a/spec/synx/file_manager_spec.rb b/spec/synx/file_manager_spec.rb deleted file mode 100644 index 1d7b925..0000000 --- a/spec/synx/file_manager_spec.rb +++ /dev/null @@ -1,45 +0,0 @@ -require 'spec_helper' -require 'fileutils' -require 'pathname' - -describe Synx::FileManager do - - let (:file_manager) do - Synx::FileManager.new - end - - describe "rm_rf" do - before(:each) { FileUtils.stub(:rm_rf) } - - it "should delete file at given path" do - file_manager.rm_rf('/the_path') - - expect(FileUtils).to have_received(:rm_rf).with('/the_path') - end - - it "should not delete file if warn flag is set" do - file_manager.options[:warn] = 'warning' - file_manager.rm_rf('/the_path') - - expect(FileUtils).to_not have_received(:rm_rf) - end - end - - describe "mv" do - before(:each) { FileUtils.stub('mv') } - - it "should move file from source to destination" do - file_manager.mv('/src', '/dest') - - expect(FileUtils).to have_received(:mv).with('/src', '/dest') - end - - it "should not move files if warn flag is set" do - file_manager.options[:warn] = 'error' - file_manager.mv('/src', '/dest') - - expect(FileUtils).to_not have_received(:mv) - end - end - -end diff --git a/spec/synx/project_spec.rb b/spec/synx/project_spec.rb index 7bcf7f9..57593cf 100644 --- a/spec/synx/project_spec.rb +++ b/spec/synx/project_spec.rb @@ -312,4 +312,40 @@ def original_file_structure expect(DUMMY_SYNX_TEST_PROJECT.exit_code).to eq(-1) end end + + describe "#file_manager" do + before(:each) { + FileUtils.stub(:mv) + FileUtils.stub(:rm_rf) + DUMMY_SYNX_TEST_PROJECT.file_manager = nil + } + + it "should remove file if warning is not set" do + DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new, :warn => nil) + DUMMY_SYNX_TEST_PROJECT.file_manager.rm_rf('/the_path') + + expect(FileUtils).to have_received(:rm_rf).with('/the_path') + end + + it "should move file if warning is not set" do + DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new, :warn => nil) + DUMMY_SYNX_TEST_PROJECT.file_manager.mv('/the_path', '/the_other_path') + + expect(FileUtils).to have_received(:mv).with('/the_path', '/the_other_path') + end + + it "should not remove file if warning is set" do + DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new, :warn => 'error') + DUMMY_SYNX_TEST_PROJECT.file_manager.rm_rf('/the_path') + + expect(FileUtils).to_not have_received(:rm_rf) + end + + it "should not move file if warning is set" do + DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new, :warn => 'error') + DUMMY_SYNX_TEST_PROJECT.file_manager.mv('/the_path', '/the_other_path') + + expect(FileUtils).to_not have_received(:mv) + end + end end From 7c1f25db9eef357b981bf7ead97dc59a5d722054 Mon Sep 17 00:00:00 2001 From: Jakub Turek Date: Wed, 1 Feb 2017 18:26:35 +0100 Subject: [PATCH 08/10] Renamed file_manager to file_utils; Improved scanning performance --- lib/synx/issue_registry.rb | 12 ++++++++++++ lib/synx/project.rb | 16 ++++++++++------ .../project/object/abstract_object.rb | 4 ++-- .../project/object/pbx_file_reference.rb | 3 ++- .../xcodeproj_ext/project/object/pbx_group.rb | 4 ++-- spec/synx/project_spec.rb | 16 ++++++++-------- 6 files changed, 36 insertions(+), 19 deletions(-) diff --git a/lib/synx/issue_registry.rb b/lib/synx/issue_registry.rb index 5854dcf..1a68e0d 100644 --- a/lib/synx/issue_registry.rb +++ b/lib/synx/issue_registry.rb @@ -35,4 +35,16 @@ def print(type) end end + + class VisitedPathsRegistry + + class << self + + def paths + @paths ||= [] + end + + end + + end end diff --git a/lib/synx/project.rb b/lib/synx/project.rb index 5738b1d..9759af3 100644 --- a/lib/synx/project.rb +++ b/lib/synx/project.rb @@ -9,7 +9,7 @@ class Project < Xcodeproj::Project DEFAULT_EXCLUSIONS = %W(/Libraries /Frameworks /Products /Pods) private_constant :DEFAULT_EXCLUSIONS - attr_accessor :delayed_groups_set_path, :group_exclusions, :prune, :sort_by_name, :warn_type, :file_manager + attr_accessor :delayed_groups_set_path, :group_exclusions, :prune, :sort_by_name, :warn_type, :file_utils def sync(options={}) set_options(options) @@ -69,8 +69,8 @@ def validated_warn_type(options) def transplant_work_project # Move the synced entries over Dir.glob(work_root_pathname + "*").each do |path| - file_manager.rm_rf(work_pathname_to_pathname(Pathname(path))) - file_manager.mv(path, root_pathname.to_s) + file_utils.rm_rf(work_pathname_to_pathname(Pathname(path))) + file_utils.mv(path, root_pathname.to_s) end end private :transplant_work_project @@ -85,7 +85,7 @@ def work_root_pathname else @work_root_pathname = Pathname(File.join(SYNXRONIZE_DIR, root_pathname.basename.to_s)) # Clean up any previous synx and start fresh - file_manager.rm_rf(@work_root_pathname.to_s) if @work_root_pathname.exist? + file_utils.rm_rf(@work_root_pathname.to_s) if @work_root_pathname.exist? @work_root_pathname.mkpath @work_root_pathname end @@ -133,8 +133,8 @@ def has_object_for_pathname?(pathname) end end - def file_manager - @file_manager ||= (warn_type ? Synx::BlankFileUtils : FileUtils) + def file_utils + @file_utils ||= (warn_type.nil? ? FileUtils : Synx::BlankFileUtils) end def sync_issues_repository @@ -145,6 +145,10 @@ def print_dry_run_issues sync_issues_repository.print(warn_type.to_s) if warn_type end + def scanned_files + @scanned_files ||= [] + end + def exit_code if warn_type.to_s == 'error' and sync_issues_repository.issues_count > 0 -1 diff --git a/lib/synx/xcodeproj_ext/project/object/abstract_object.rb b/lib/synx/xcodeproj_ext/project/object/abstract_object.rb index 390c36e..43e3c27 100644 --- a/lib/synx/xcodeproj_ext/project/object/abstract_object.rb +++ b/lib/synx/xcodeproj_ext/project/object/abstract_object.rb @@ -59,8 +59,8 @@ def sync(group) raise NotImplementedError end - def file_manager - project.file_manager + def file_utils + project.file_utils end end diff --git a/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb b/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb index 51013eb..6e82822 100644 --- a/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb +++ b/lib/synx/xcodeproj_ext/project/object/pbx_file_reference.rb @@ -7,7 +7,8 @@ def sync(group) if should_sync? if should_move? track_sync_issues - file_manager.mv(real_path.to_s, work_pathname.to_s) + project.scanned_files << real_path.to_s + file_utils.mv(real_path.to_s, work_pathname.to_s) # TODO: move out to abstract_object self.source_tree = "" self.path = work_pathname.relative_path_from(parent.work_pathname).to_s diff --git a/lib/synx/xcodeproj_ext/project/object/pbx_group.rb b/lib/synx/xcodeproj_ext/project/object/pbx_group.rb index 445153e..67d08eb 100644 --- a/lib/synx/xcodeproj_ext/project/object/pbx_group.rb +++ b/lib/synx/xcodeproj_ext/project/object/pbx_group.rb @@ -71,7 +71,7 @@ def move_entries_not_in_xcodeproj Synx::Tabber.puts "#{basename}/".green Synx::Tabber.increase real_path.children.each do |entry_pathname| - unless project.has_object_for_pathname?(entry_pathname) + unless project.scanned_files.include?(entry_pathname.to_s) or project.has_object_for_pathname?(entry_pathname) handle_unused_entry(entry_pathname) end end @@ -120,7 +120,7 @@ def handle_unused_file(file_pathname) elsif !project.prune || !is_file_to_prune destination = project.pathname_to_work_pathname(file_pathname.parent.realpath) destination.mkpath - file_manager.mv(file_pathname.realpath, destination) + file_utils.mv(file_pathname.realpath, destination) if is_file_to_prune Synx::Tabber.puts "#{file_pathname.basename} (source/image file that is not referenced by the Xcode project)".yellow else diff --git a/spec/synx/project_spec.rb b/spec/synx/project_spec.rb index 57593cf..3455f8f 100644 --- a/spec/synx/project_spec.rb +++ b/spec/synx/project_spec.rb @@ -251,7 +251,7 @@ def original_file_structure it "should start fresh by removing any existing directory at work_root_pathname" do Pathname.any_instance.stub(:exist?).and_return(true) - expect(DUMMY_SYNX_TEST_PROJECT.file_manager).to receive(:rm_rf) + expect(DUMMY_SYNX_TEST_PROJECT.file_utils).to receive(:rm_rf) DUMMY_SYNX_TEST_PROJECT.send(:work_root_pathname) end @@ -263,7 +263,7 @@ def original_file_structure it "should be an idempotent operation but return the same value through memoization" do pathname = DUMMY_SYNX_TEST_PROJECT.send(:work_root_pathname) - expect(DUMMY_SYNX_TEST_PROJECT.file_manager).to_not receive(:rm_rf) + expect(DUMMY_SYNX_TEST_PROJECT.file_utils).to_not receive(:rm_rf) expect_any_instance_of(Pathname).to_not receive(:exist?) expect_any_instance_of(Pathname).to_not receive(:mkpath) expect(DUMMY_SYNX_TEST_PROJECT.send(:work_root_pathname)).to be(pathname) @@ -313,37 +313,37 @@ def original_file_structure end end - describe "#file_manager" do + describe "#file_utils" do before(:each) { FileUtils.stub(:mv) FileUtils.stub(:rm_rf) - DUMMY_SYNX_TEST_PROJECT.file_manager = nil + DUMMY_SYNX_TEST_PROJECT.file_utils = nil } it "should remove file if warning is not set" do DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new, :warn => nil) - DUMMY_SYNX_TEST_PROJECT.file_manager.rm_rf('/the_path') + DUMMY_SYNX_TEST_PROJECT.file_utils.rm_rf('/the_path') expect(FileUtils).to have_received(:rm_rf).with('/the_path') end it "should move file if warning is not set" do DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new, :warn => nil) - DUMMY_SYNX_TEST_PROJECT.file_manager.mv('/the_path', '/the_other_path') + DUMMY_SYNX_TEST_PROJECT.file_utils.mv('/the_path', '/the_other_path') expect(FileUtils).to have_received(:mv).with('/the_path', '/the_other_path') end it "should not remove file if warning is set" do DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new, :warn => 'error') - DUMMY_SYNX_TEST_PROJECT.file_manager.rm_rf('/the_path') + DUMMY_SYNX_TEST_PROJECT.file_utils.rm_rf('/the_path') expect(FileUtils).to_not have_received(:rm_rf) end it "should not move file if warning is set" do DUMMY_SYNX_TEST_PROJECT.sync(:output => StringIO.new, :warn => 'error') - DUMMY_SYNX_TEST_PROJECT.file_manager.mv('/the_path', '/the_other_path') + DUMMY_SYNX_TEST_PROJECT.file_utils.mv('/the_path', '/the_other_path') expect(FileUtils).to_not have_received(:mv) end From ef5b9e150bdd363f9777a42394eb72fefe84a12a Mon Sep 17 00:00:00 2001 From: Jakub Turek Date: Wed, 1 Feb 2017 18:51:46 +0100 Subject: [PATCH 09/10] Updated README --- README.md | 11 +++++++++++ bin/synx | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 3001100..4ddbc35 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,7 @@ Synx supports the following options: --no-sort-by-name disable sorting groups by name --quiet, -q silence all output --exclusion, -e EXCLUSION ignore an Xcode group while syncing + --warn-type, -w warning|error show warnings or errors only without modifying the project structure ``` For example, OCMock could have been organized using this command: @@ -53,6 +54,16 @@ For example, OCMock could have been organized using this command: if they had wanted not to sync the `/OCMock/Core Mocks` and `/OCMockTests` groups, and also remove (`-p`) any image/source files found by synx that weren't referenced by any groups in Xcode. +### Integration with Xcode + +Synx can be integrated with Xcode to check for synchronization issues after every build. In order to do that open your project settings, go to **Build Phases** tab, click **+** sign and select **New Run Script Phase**. Paste the following content into Run script window: + + synx -w warning ${PROJECT_FILE_PATH} + +You can also make Synx fail the build in case it finds any issues with a following script: + + synx -w error ${PROJECT_FILE_PATH} + ## Contributing We'd love to see your ideas for improving this library! The best way to contribute is by submitting a pull request. We'll do our best to respond to your patch as soon as possible. You can also submit a [new Github issue](https://github.com/venmo/synx/issues/new) if you find bugs or have questions. :octocat: diff --git a/bin/synx b/bin/synx index cd9de7d..d8dc655 100755 --- a/bin/synx +++ b/bin/synx @@ -12,7 +12,7 @@ Clamp do option "--no-sort-by-name", :flag, "disable sorting groups by name" option ["--quiet", "-q"], :flag, "silence all output" option ["--exclusion", "-e"], "EXCLUSION", "ignore an Xcode group while syncing", :multivalued => true - option ["--warn-type", "-w"], "warning|error", "show warnings or errors for files whose paths do not match the project structure. Using this option will not make any changes to the project. Use it as an Xcode build phase to alert the developer.", :attribute_name => :warn_type + option ["--warn-type", "-w"], "warning|error", "show warnings or errors only without modifying the project structure", :attribute_name => :warn_type option ["--version", "-v"], :flag, "shows synx version" do puts "Synx #{Synx::VERSION}" exit(0) From 5523893dcfae6b24c9f1cc23e8bc9062b28abe7a Mon Sep 17 00:00:00 2001 From: Jakub Turek Date: Wed, 1 Feb 2017 19:03:24 +0100 Subject: [PATCH 10/10] Truncated leftover class --- lib/synx/issue_registry.rb | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/lib/synx/issue_registry.rb b/lib/synx/issue_registry.rb index 1a68e0d..5854dcf 100644 --- a/lib/synx/issue_registry.rb +++ b/lib/synx/issue_registry.rb @@ -35,16 +35,4 @@ def print(type) end end - - class VisitedPathsRegistry - - class << self - - def paths - @paths ||= [] - end - - end - - end end