From 72eda0a471950505d76fc5d997feea97653abcc4 Mon Sep 17 00:00:00 2001 From: Nathan Kleyn Date: Tue, 20 Jan 2015 09:04:05 +0000 Subject: [PATCH] Use Tarjan's strongly connected components algorithm within the graph. Using the TSort module, the Graph has been refactored to deal with templates from the beginning, and take care of things like linking and sorting in a more transparent way. It now includes Enumerable, making it much easier to deal with. --- examples/test-static-project-2/Shantyfile | 2 +- .../test-static-project-3/Shantyfile | 2 +- lib/shanty/discoverer.rb | 2 +- lib/shanty/graph.rb | 74 +++++++++++-------- lib/shanty/mixins/acts_as_link_graph_node.rb | 10 --- lib/shanty/project.rb | 4 +- lib/shanty/project_template.rb | 10 ++- lib/shanty/task_env.rb | 8 +- lib/shanty/task_sets/basic.rb | 6 +- spec/lib/shanty/graph_spec.rb | 54 ++++++-------- 10 files changed, 83 insertions(+), 89 deletions(-) diff --git a/examples/test-static-project-2/Shantyfile b/examples/test-static-project-2/Shantyfile index 772b132..f28e046 100644 --- a/examples/test-static-project-2/Shantyfile +++ b/examples/test-static-project-2/Shantyfile @@ -1 +1 @@ -parent 'test-static-project' +parent 'examples/test-static-project' diff --git a/examples/test-static-project-2/test-static-project-3/Shantyfile b/examples/test-static-project-2/test-static-project-3/Shantyfile index b624ad9..e1d5155 100644 --- a/examples/test-static-project-2/test-static-project-3/Shantyfile +++ b/examples/test-static-project-2/test-static-project-3/Shantyfile @@ -1,4 +1,4 @@ -parent 'test-static-project-2' +parent 'examples/test-static-project-2' after_create do # This will run after the project has been created. diff --git a/lib/shanty/discoverer.rb b/lib/shanty/discoverer.rb index 5470218..bc2f54d 100644 --- a/lib/shanty/discoverer.rb +++ b/lib/shanty/discoverer.rb @@ -25,7 +25,7 @@ def discover_all private def create_project(*args) - ProjectTemplate.new(*args) + ProjectTemplate.new(Dir.pwd, *args) end end end diff --git a/lib/shanty/graph.rb b/lib/shanty/graph.rb index 08721ba..8d56ad7 100644 --- a/lib/shanty/graph.rb +++ b/lib/shanty/graph.rb @@ -1,4 +1,6 @@ require 'algorithms' +require 'forwardable' +require 'tsort' module Shanty # Public: Represents the link graph of projects in the repository. This class is @@ -7,19 +9,22 @@ module Shanty # projects need to be build by combining a Git diff with the dependency graph # to resolve to a list of projects required to be built. class Graph - attr_reader :projects + extend Forwardable + include TSort, Enumerable + + def_delegators :@projects, :<<, :length, :add, :remove + def_delegators :sorted_projects, :each, :values, :[] # Public: Initialise a ProjectLinkGraph. # - # projects - An array of project instances to take and link together into - # a graph structure of dependencies. - def initialize(projects) - @projects = sort_projects(link_projects(projects)) + # project_templates - An array of project templates to take, instantiate and + # link together into a graph structure of dependencies. + def initialize(project_templates) @project_path_trie = Containers::Trie.new + @project_templates = project_templates + @projects = projects_by_path.values - @projects.each do |project| - @project_path_trie[project.path] = project - end + link_projects end # Public: All the changed projects in the current repository. @@ -27,7 +32,7 @@ def initialize(projects) # Returns an Array of Project subclasses, one for each project in the # repository. def changed - @projects.select(&:changed?) + select(&:changed?) end # Public: Returns a project, if any, with the given name. @@ -36,7 +41,7 @@ def changed # # Returns an instance of a Project subclass if found, otherwise nil. def by_name(name) - @projects.find { |project| project.name == name } + find { |project| project.name == name } end # Public: Returns all projects of the given types. @@ -46,7 +51,7 @@ def by_name(name) # Returns an Array of Project subclasses, one for each project in the # repository. def all_of_type(*types) - @projects.select { |project| types.include?(project.class) } + select { |project| types.include?(project.class) } end # Public: Returns all the changed projects of the given types. @@ -84,6 +89,26 @@ def owner_of_file(path) private + def sorted_projects + @sorted_projects ||= tsort + end + + def tsort_each_node + @projects.each { |p| yield p } + end + + def tsort_each_child(project) + projects_by_path[project.path].parents.each { |p| yield p } + end + + def projects_by_path + @projects_by_path ||= Hash[@project_templates.map do |project_template| + project = project_template.type.new(project_template) + @project_path_trie[project.path] = project + [project.path, project] + end] + end + # Private: Given a list of projects, construct the parent/child # relationships between them given a list of their parents/children by name # as defined on the project instances. @@ -91,31 +116,18 @@ def owner_of_file(path) # projects - An array of Project subclasses to link together. # # Returns an Array of linked projects. - def link_projects(projects) - projects_by_name = projects.each_with_object({}) { |project, acc| acc[project.name] = project } + def link_projects + projects_by_path.each_value do |project| + project.parents_by_path.each do |parent_path| + parent_dependency = projects_by_path[parent_path] + if parent_dependency.nil? + fail("Cannot find project at path #{parent_path}, which was specified as a dependency for #{project}") + end - projects.each do |project| - parent_dependencies = project.parents_by_name.map { |parent_name| projects_by_name[parent_name] }.compact - - parent_dependencies.each do |parent_dependency| project.add_parent(parent_dependency) parent_dependency.add_child(project) end end - - projects - end - - # Private: Given a list of Project subclasses, sort them by their distance - # from the root node (that is, the topmost node). This order is important - # because it matches the order in which things should be build to avoid - # missing dependencies. - # - # projects - An array of Project subclasses to sort. - # - # Returns a sorted Array of Project subclasses. - def sort_projects(projects) - projects.sort { |a, b| a.distance_from_root <=> b.distance_from_root } end end end diff --git a/lib/shanty/mixins/acts_as_link_graph_node.rb b/lib/shanty/mixins/acts_as_link_graph_node.rb index 960f6b9..b09e4b3 100644 --- a/lib/shanty/mixins/acts_as_link_graph_node.rb +++ b/lib/shanty/mixins/acts_as_link_graph_node.rb @@ -66,16 +66,6 @@ def all_children def all_parents parents + parents.map(&:all_parents).flatten end - - # Public: Calculate the maximum number of traverses that need to be made to - # reach the root from this node. - # - # Returns a Fixnum representing the traverses to the root. Note that a return - # value of 0 means this is the root (ie. it has no parents). - def distance_from_root - return 0 if parents.empty? - parents.map(&:distance_from_root).max + 1 - end end end end diff --git a/lib/shanty/project.rb b/lib/shanty/project.rb index 8f8cdb6..35198e1 100644 --- a/lib/shanty/project.rb +++ b/lib/shanty/project.rb @@ -7,7 +7,7 @@ class Project include Mixins::ActsAsLinkGraphNode include Mixins::Callbacks - attr_accessor :name, :path, :options, :parents_by_name, :changed + attr_accessor :name, :path, :options, :parents_by_path, :changed attr_reader :changed alias_method :changed?, :changed @@ -19,7 +19,7 @@ def initialize(project_template) @name = project_template.name @path = project_template.path @options = project_template.options - @parents_by_name = project_template.parents + @parents_by_path = project_template.parents @changed = false project_template.plugins.each do |plugin| diff --git a/lib/shanty/project_template.rb b/lib/shanty/project_template.rb index 0748d22..f052c03 100644 --- a/lib/shanty/project_template.rb +++ b/lib/shanty/project_template.rb @@ -7,11 +7,12 @@ class ProjectTemplate extend Mixins::AttrCombinedAccessor attr_combined_accessor :name, :type, :plugins, :parents, :options - attr_reader :path + attr_reader :root, :path - def initialize(path, args = {}) + def initialize(root, path, args = {}) fail 'Path to project must be a directory.' unless File.directory?(path) + @root = root @path = path @name = File.basename(path) @type = args[:type] || StaticProject @@ -27,7 +28,7 @@ def execute_shantyfile return unless File.exist?(shantyfile_path) - eval(File.read(shantyfile_path)) + instance_eval(File.read(shantyfile_path), shantyfile_path) end def plugin(plugin) @@ -35,7 +36,8 @@ def plugin(plugin) end def parent(parent) - @parents << parent + # Will make the parent path relative to the root if (and only if) it is relative. + @parents << File.expand_path(parent, @root) end def option(key, value) diff --git a/lib/shanty/task_env.rb b/lib/shanty/task_env.rb index ab54b35..1047ea9 100644 --- a/lib/shanty/task_env.rb +++ b/lib/shanty/task_env.rb @@ -57,13 +57,9 @@ def construct_project_graph Discoverer.new.discover_all end - projects = project_templates.map do |project_template| - project_template.type.new(project_template) - end - - graph = Graph.new(projects) + graph = Graph.new(project_templates) - Mutator.new.apply_mutations(graph) + # Mutator.new.apply_mutations(graph) end end end diff --git a/lib/shanty/task_sets/basic.rb b/lib/shanty/task_sets/basic.rb index 179bbe0..c1a3bf4 100644 --- a/lib/shanty/task_sets/basic.rb +++ b/lib/shanty/task_sets/basic.rb @@ -15,7 +15,7 @@ def init(_options, _task_env) option :changed, type: :boolean, desc: 'tasks.common.options.changed' option :types, type: :array, desc: 'tasks.common.options.types' def projects(options, task_env) - task_env.graph.projects.each do |project| + task_env.graph.each do |project| next if options.changed? && !project.changed? puts project end @@ -26,7 +26,7 @@ def projects(options, task_env) option :watch, type: :boolean, desc: 'tasks.common.options.watch' option :types, type: :array, desc: 'tasks.common.options.types' def build(options, task_env) - task_env.graph.projects.each do |project| + task_env.graph.each do |project| next if options.changed? && !project.changed? fail I18n.t('tasks.build.failed', project: project) unless project.publish(:build) end @@ -37,7 +37,7 @@ def build(options, task_env) option :watch, type: :boolean, desc: 'tasks.common.options.watch' option :types, type: :array, desc: 'tasks.common.options.types' def test(options, task_env) - task_env.graph.projects.each do |project| + task_env.graph.each do |project| next if options.changed? && !project.changed? fail I18n.t('tasks.test.failed', project: project) unless project.publish(:test) end diff --git a/spec/lib/shanty/graph_spec.rb b/spec/lib/shanty/graph_spec.rb index cb00d18..d8dd726 100644 --- a/spec/lib/shanty/graph_spec.rb +++ b/spec/lib/shanty/graph_spec.rb @@ -8,48 +8,42 @@ # Allows all classes to be refereneced without the module name module Shanty RSpec.describe Graph do + let(:root) { File.expand_path(File.join(__dir__, '..', '..', '..')) } let(:project_paths) do [ - File.join('examples', 'test-static-project-2', 'test-static-project-3'), - File.join('examples', 'test-static-project-2'), - File.join('examples', 'test-static-project') + File.join(root, 'examples', 'test-static-project-2', 'test-static-project-3'), + File.join(root, 'examples', 'test-static-project-2'), + File.join(root, 'examples', 'test-static-project') ] end - let(:project_templates) { project_paths.map { |project_path| ProjectTemplate.new(project_path) } } - let(:projects) { project_templates.map { |project_template| StaticProject.new(project_template) } } - let(:graph) { Graph.new(projects) } + let(:project_templates) { project_paths.map { |project_path| ProjectTemplate.new(root, project_path) } } + let(:graph) { Graph.new(project_templates) } - describe '#projects' do + describe 'enumerable methods' do it 'returns projects linked together with parent relationships' do - projects = graph.projects - - expect(projects[0].parents).to eql([]) - expect(projects[1].parents).to eql([projects[0]]) - expect(projects[2].parents).to eql([projects[1]]) + expect(graph[0].parents.map(&:path)).to eql([]) + expect(graph[1].parents.map(&:path)).to eql([project_paths[2]]) + expect(graph[2].parents.map(&:path)).to eql([project_paths[1]]) end it 'returns projects linked together with child relationships' do - projects = graph.projects - - expect(projects[0].children).to eql([projects[1]]) - expect(projects[1].children).to eql([projects[2]]) - expect(projects[2].children).to eql([]) + expect(graph[0].children.map(&:path)).to eql([project_paths[1]]) + expect(graph[1].children.map(&:path)).to eql([project_paths[0]]) + expect(graph[2].children.map(&:path)).to eql([]) end - it 'returns projects sorted from roots to leaves' do - graph_projects = graph.projects - - expect(projects[0]).to equal(graph_projects[2]) - expect(projects[1]).to equal(graph_projects[1]) - expect(projects[2]).to equal(graph_projects[0]) + it "returns projects sorted using Tarjan's strongly connected components algorithm" do + expect(graph[0].path).to equal(project_paths[2]) + expect(graph[1].path).to equal(project_paths[1]) + expect(graph[2].path).to equal(project_paths[0]) end end describe '#changed' do it 'returns only projects where #changed? is true' do - projects.first.changed = true + graph.first.changed = true - expect(graph.changed).to contain_exactly(projects.first) + expect(graph.changed).to contain_exactly(graph.first) end end @@ -59,7 +53,7 @@ module Shanty end it 'returns the correct project when finding a name that does exist' do - expect(graph.by_name(projects.first.name)).to equal(projects.first) + expect(graph.by_name(graph.first.name)).to equal(graph.first) end end @@ -73,13 +67,13 @@ module Shanty end it 'returns the correct projects when matching types are given' do - expect(graph.all_of_type(StaticProject)).to match_array(projects) + expect(graph.all_of_type(StaticProject)).to match_array(graph) end end describe '#changed_of_type' do before do - projects.first.changed = true + graph.first.changed = true end it 'returns an empty array when no types are given' do @@ -91,7 +85,7 @@ module Shanty end it 'returns the correct projects when matching types are given' do - expect(graph.changed_of_type(StaticProject)).to contain_exactly(projects.first) + expect(graph.changed_of_type(StaticProject)).to contain_exactly(graph.first) end end @@ -101,7 +95,7 @@ module Shanty end it 'returns the correct project that owns a given folder' do - expect(graph.owner_of_file(project_paths.first)).to equal(projects.first) + expect(graph.owner_of_file(project_paths.first).path).to equal(project_paths.first) end end end