From 191967530cba3b073c37854e2bf5b9c8500d77d9 Mon Sep 17 00:00:00 2001 From: Sven Fuchs Date: Tue, 2 Jun 2020 19:48:45 +0200 Subject: [PATCH 1/5] optionally merge without flattening the tree --- lib/travis/yml/configs/config/base.rb | 25 +++++++++++++++++++++++++ lib/travis/yml/configs/configs.rb | 1 + 2 files changed, 26 insertions(+) diff --git a/lib/travis/yml/configs/config/base.rb b/lib/travis/yml/configs/config/base.rb index 655d516ff..ad94d8314 100644 --- a/lib/travis/yml/configs/config/base.rb +++ b/lib/travis/yml/configs/config/base.rb @@ -51,6 +51,31 @@ def imports end end memoize :imports + + def merge + return {} if errored? || circular? || !matches? + # skip all but the last duplicate + order_duplicates(flatten2) if root? + imports.reverse.map(&:merge).inject(part) do |lft, rgt| + Support::Merge.new(lft.to_h, rgt.to_h).apply + end + end + + def flatten2 + return [] if errored? || circular? || !matches? + configs = sort([self].compact + imports.map(&:flatten2).flatten) + configs.uniq(&:to_s) + end + + def order_duplicates(configs) + p configs.select(&:skip?).size + configs.select(&:skip?).each do |lft| + rgt = configs.detect { |rgt| lft.to_s == rgt.to_s && !rgt.skip? } + p lft, rgt + # configs[i] = configs.delete(rgt) + end + end + # Flattening the tree should result in a unique array of configs # ordered by the order resulting in walking the tree depth-first. # However, we load the tree breadth-first and load times vary. diff --git a/lib/travis/yml/configs/configs.rb b/lib/travis/yml/configs/configs.rb index 8a0e840e2..c16a1667c 100644 --- a/lib/travis/yml/configs/configs.rb +++ b/lib/travis/yml/configs/configs.rb @@ -91,6 +91,7 @@ def fetch def merge doc = Yml.apply(Yml::Parts::Merge.new(configs).apply.to_h, opts) if opts[:merge_normalized] + doc ||= Yml.load([ctx.fetch.config.merge], opts) if opts[:merge_mode_2] || true doc ||= Yml.load(configs.map(&:part), opts) @config = except(doc.serialize, *DROP) msgs.concat(doc.msgs) From 1bd661b4f1bbc5c623f5828ba128796290d1eece Mon Sep 17 00:00:00 2001 From: Sven Fuchs Date: Mon, 3 Aug 2020 15:01:16 +0200 Subject: [PATCH 2/5] extract Config::Order --- .ruby-version | 2 +- Gemfile.lock | 4 +- lib/travis/yml/configs/config/base.rb | 37 ++++---- lib/travis/yml/configs/config/order.rb | 38 +++++++++ spec/travis/yml/configs/config/order_spec.rb | 89 ++++++++++++++++++++ 5 files changed, 147 insertions(+), 23 deletions(-) create mode 100644 lib/travis/yml/configs/config/order.rb create mode 100644 spec/travis/yml/configs/config/order_spec.rb diff --git a/.ruby-version b/.ruby-version index 338a5b5d8..57cf282eb 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.6.6 +2.6.5 diff --git a/Gemfile.lock b/Gemfile.lock index 4c8e76001..33091df5f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -155,7 +155,7 @@ DEPENDENCIES webmock RUBY VERSION - ruby 2.6.6p146 + ruby 2.6.5p114 BUNDLED WITH - 2.0.1 + 2.1.4 diff --git a/lib/travis/yml/configs/config/base.rb b/lib/travis/yml/configs/config/base.rb index ad94d8314..f5e764b0f 100644 --- a/lib/travis/yml/configs/config/base.rb +++ b/lib/travis/yml/configs/config/base.rb @@ -1,5 +1,6 @@ require 'forwardable' require 'travis/yml/helper/obj' +require 'travis/yml/configs/config/order' require 'travis/yml/configs/errors' require 'travis/yml/configs/ref' @@ -32,6 +33,10 @@ def config @config ||= {} end + def config=(config) + @config = config + end + def load(&block) @on_loaded = block if root? end @@ -53,27 +58,15 @@ def imports memoize :imports def merge - return {} if errored? || circular? || !matches? - # skip all but the last duplicate - order_duplicates(flatten2) if root? + return {} if skip? || errored? || circular? || !matches? + order_duplicates if root? imports.reverse.map(&:merge).inject(part) do |lft, rgt| Support::Merge.new(lft.to_h, rgt.to_h).apply end end - def flatten2 - return [] if errored? || circular? || !matches? - configs = sort([self].compact + imports.map(&:flatten2).flatten) - configs.uniq(&:to_s) - end - - def order_duplicates(configs) - p configs.select(&:skip?).size - configs.select(&:skip?).each do |lft| - rgt = configs.detect { |rgt| lft.to_s == rgt.to_s && !rgt.skip? } - p lft, rgt - # configs[i] = configs.delete(rgt) - end + def order_duplicates + Order.new(self).run end # Flattening the tree should result in a unique array of configs @@ -112,6 +105,10 @@ def sort(configs) end end + def errored? + !!@errored + end + def circular? parents.map(&:to_s).include?(to_s) end @@ -155,6 +152,10 @@ def skip @skip = true end + def unskip + @skip = false + end + def skip? !!@skip end @@ -209,10 +210,6 @@ def required? !parent&.api? || !travis_yml? end - def errored? - !!@errored - end - def secure?(obj) case obj when Hash diff --git a/lib/travis/yml/configs/config/order.rb b/lib/travis/yml/configs/config/order.rb new file mode 100644 index 000000000..853c7def7 --- /dev/null +++ b/lib/travis/yml/configs/config/order.rb @@ -0,0 +1,38 @@ +module Travis + module Yml + module Configs + module Config + class Order < Obj.new(:config) + # make sure we keep the last duplicate, no matter in which order + # configs have been loaded + def run + order_duplicates(configs.reverse) + end + + def configs + @configs ||= flatten(config) + end + + def flatten(config) + return [] if config.errored? || config.circular? || !config.matches? + [config] + config.imports.map { |config| flatten(config) }.flatten + end + + def order_duplicates(configs) + configs.each.with_index do |lft, ix| + next unless lft.skip? + rgt = configs[(ix + 1)..-1].detect { |rgt| lft.to_s == rgt.to_s && !rgt.skip? } + swap(lft, rgt) if rgt + end + end + + def swap(lft, rgt) + lft.unskip + rgt.skip + lft.config, rgt.config = rgt.config, nil + end + end + end + end + end +end diff --git a/spec/travis/yml/configs/config/order_spec.rb b/spec/travis/yml/configs/config/order_spec.rb new file mode 100644 index 000000000..a80d63783 --- /dev/null +++ b/spec/travis/yml/configs/config/order_spec.rb @@ -0,0 +1,89 @@ +describe Travis::Yml::Configs::Config::Order, 'keep the last duplicate' do + let(:const) do + Class.new(Obj.new(:source, config: nil, circular: false, errored: false, matches: true)) do + def imports + @imports ||= [] + end + + def skip? + !!@skip + end + + def skip + @skip = true + end + + def unskip + @skip = false + end + + alias circular? circular + alias errored? errored + alias matches? matches + + def to_s + source + end + end + end + + let(:config) { const.new('.travis.yml') } + + describe 'flat' do + before do + 0.upto(3) do |ix| + config.imports << const.new('one.yml', ix).tap do |config| + config.skip if skips[ix] + end + end + described_class.new(config).run + end + + describe 'unskipped 1st position' do + let(:skips) { [false, true, true, true] } + it { expect(config.imports.map(&:skip?)).to eq [true, true, true, false] } + end + + describe 'unskipped 2nd position' do + let(:skips) { [true, false, true, true] } + it { expect(config.imports.map(&:skip?)).to eq [true, true, true, false] } + end + + describe 'unskipped 3rd position' do + let(:skips) { [true, true, false, true] } + it { expect(config.imports.map(&:skip?)).to eq [true, true, true, false] } + end + + describe 'unskipped 4th position' do + let(:skips) { [true, true, true, false] } + it { expect(config.imports.map(&:skip?)).to eq [true, true, true, false] } + end + end + + describe 'nested' do + let(:nested) { config.imports.map { |config| config.imports }.flatten } + + before do + config.imports << const.new('one.yml') + config.imports << const.new('two.yml') + + 0.upto(1) do |ix| + config.imports[ix].imports << const.new('nested.yml').tap do |config| + config.skip if skips[ix] + end + end + + described_class.new(config).run + end + + describe 'unskipped 1st position' do + let(:skips) { [false, true] } + it { expect(nested.map(&:skip?)).to eq [true, false] } + end + + describe 'unskipped 2nd position' do + let(:skips) { [true, false] } + it { expect(nested.map(&:skip?)).to eq [true, false] } + end + end +end From fe54a6918db8c63f1edccc5f610f8c684b2dadb3 Mon Sep 17 00:00:00 2001 From: Sven Fuchs Date: Fri, 30 Oct 2020 19:30:03 +0100 Subject: [PATCH 3/5] silence logger during tests --- lib/travis/yml/configs/model/repos.rb | 2 +- lib/travis/yml/remote_vcs/client.rb | 2 +- lib/travis/yml/remote_vcs/repository.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/travis/yml/configs/model/repos.rb b/lib/travis/yml/configs/model/repos.rb index 98c64e40d..525bb6753 100644 --- a/lib/travis/yml/configs/model/repos.rb +++ b/lib/travis/yml/configs/model/repos.rb @@ -32,7 +32,7 @@ def []=(slug, repo) end def fetch(slug, provider) - logger.info "Get Repo for #{slug} #{provider}" + logger.info "Get Repo for #{slug} #{provider}" unless ENV['ENV'] == 'test' Travis::Repo.new(slug, provider).fetch end diff --git a/lib/travis/yml/remote_vcs/client.rb b/lib/travis/yml/remote_vcs/client.rb index 9d5754d72..328005c66 100644 --- a/lib/travis/yml/remote_vcs/client.rb +++ b/lib/travis/yml/remote_vcs/client.rb @@ -23,7 +23,7 @@ def http_options def request(method, name) resp = connection.send(method) { |req| yield(req) } - logger.info("RemoteVcs response #{resp.inspect}") + logger.info("RemoteVcs response #{resp.inspect}") unless ENV['ENV'] == 'test' JSON.parse(resp.body) end diff --git a/lib/travis/yml/remote_vcs/repository.rb b/lib/travis/yml/remote_vcs/repository.rb index 66088e22a..e108ab82e 100644 --- a/lib/travis/yml/remote_vcs/repository.rb +++ b/lib/travis/yml/remote_vcs/repository.rb @@ -5,7 +5,7 @@ module Yml module RemoteVcs class Repository < Client def content(id:, path:, ref:) - logger.info("RemoteVcs Repository #{id}, #{path}") + logger.info("RemoteVcs Repository #{id}, #{path}") unless ENV['ENV'] == 'test' request(:get, __method__) do |req| req.url "repos/#{id}/contents/#{path}" req.params['ref'] = ref From c1db1479a57a8ad01fe0b4a440a48dee853a6972 Mon Sep 17 00:00:00 2001 From: Sven Fuchs Date: Fri, 30 Oct 2020 19:55:49 +0100 Subject: [PATCH 4/5] revert accidental changes to ruby version --- .ruby-version | 2 +- Gemfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.ruby-version b/.ruby-version index 57cf282eb..338a5b5d8 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.6.5 +2.6.6 diff --git a/Gemfile.lock b/Gemfile.lock index 33091df5f..4c8e76001 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -155,7 +155,7 @@ DEPENDENCIES webmock RUBY VERSION - ruby 2.6.5p114 + ruby 2.6.6p146 BUNDLED WITH - 2.1.4 + 2.0.1 From 607bad92524113461a24511be5885aa645b96766 Mon Sep 17 00:00:00 2001 From: Sven Fuchs Date: Mon, 2 Nov 2020 12:10:29 +0100 Subject: [PATCH 5/5] add spec, remove bogus reverse --- lib/travis/yml/configs/config/base.rb | 2 +- spec/travis/yml/configs/import_spec.rb | 80 ++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/lib/travis/yml/configs/config/base.rb b/lib/travis/yml/configs/config/base.rb index f5e764b0f..d2e40bea8 100644 --- a/lib/travis/yml/configs/config/base.rb +++ b/lib/travis/yml/configs/config/base.rb @@ -60,7 +60,7 @@ def imports def merge return {} if skip? || errored? || circular? || !matches? order_duplicates if root? - imports.reverse.map(&:merge).inject(part) do |lft, rgt| + imports.map(&:merge).inject(part) do |lft, rgt| Support::Merge.new(lft.to_h, rgt.to_h).apply end end diff --git a/spec/travis/yml/configs/import_spec.rb b/spec/travis/yml/configs/import_spec.rb index bfc464566..05afca6dd 100644 --- a/spec/travis/yml/configs/import_spec.rb +++ b/spec/travis/yml/configs/import_spec.rb @@ -399,4 +399,84 @@ def self.imports(sources) ) end end + + describe do + let(:repo) { { id: 1, github_id: 1, slug: 'owner/repo', token: repo_token, private: true } } + + let(:travis_yml) do + <<~yml + script: + - ./travis_yml + import: + - source: one.yml + mode: #{mode} + - source: two.yml + mode: #{mode} + yml + end + + let(:one) do + <<~yml + script: + - ./one + import: + - source: nested.yml + mode: #{mode} + yml + end + + let(:two) do + <<~yml + script: + - ./two + import: + - source: nested.yml + mode: #{mode} + yml + end + + let(:nested) do + <<~yml + script: + - ./nested + yml + end + + before { stub_repo(repo[:slug], internal: true, body: repo) } + before { stub_content(repo[:id], '.travis.yml', travis_yml) } + before { stub_content(repo[:id], 'one.yml', one) } + before { stub_content(repo[:id], 'two.yml', two) } + before { stub_content(repo[:id], 'nested.yml', nested) } + before { configs.tap(&:load) } + + describe 'deep_merge_append' do + let(:mode) { :deep_merge_append } + + it do + expect(configs.config).to eq( + script: %w( + ./nested + ./two + ./one + ./travis_yml + ) + ) + end + end + + describe 'deep_merge_prepend' do + let(:mode) { :deep_merge_prepend } + + it do + expect(configs.config).to eq( + script: %w( + ./travis_yml + ./one + ./two + ./nested + ) + ) + end + end + end end