From fb3dea78433fbb0b7da7f5d4399a42af936996a9 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Thu, 21 Nov 2019 17:07:41 +0800 Subject: [PATCH 1/2] (GH-199) Monkey Patch the Null Loader The Null Loader was removed in Puppet 6.11.0+ (and friends). This commit monkey patches the Null Loader back in if it doesn't exist. --- .../puppet_monkey_patches_puppetstrings.rb | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/lib/puppet-languageserver-sidecar/puppet_monkey_patches_puppetstrings.rb b/lib/puppet-languageserver-sidecar/puppet_monkey_patches_puppetstrings.rb index e1a2b0ff..78290e8d 100644 --- a/lib/puppet-languageserver-sidecar/puppet_monkey_patches_puppetstrings.rb +++ b/lib/puppet-languageserver-sidecar/puppet_monkey_patches_puppetstrings.rb @@ -69,6 +69,84 @@ def discover_paths(type, name_authority = Pcore::RUNTIME_NAME_AUTHORITY) end end +# The Null Loader was removed in Puppet 6.11.0+ (and friends) so monkey patch it back in! +# Last known source is at - https://github.com/puppetlabs/puppet/blob/6.10.1/lib/puppet/pops/loader/null_loader.rb +if defined?(::Puppet::Pops::Loader::NullLoader).nil? + # This came direct from Puppet so ignore Rubocop + # rubocop:disable Lint/UnusedMethodArgument + # rubocop:disable Style/ClassAndModuleChildren + # rubocop:disable Layout/SpaceAroundEqualsInParameterDefault + # rubocop:disable Style/StringLiterals + # rubocop:disable Style/TrivialAccessors + # rubocop:disable Style/DefWithParentheses + # The null loader is empty and delegates everything to its parent if it has one. + # + class ::Puppet::Pops::Loader::NullLoader < ::Puppet::Pops::Loader::Loader + attr_reader :loader_name + + # Construct a NullLoader, optionally with a parent loader + # + def initialize(parent_loader=nil, loader_name = "null-loader") + super(loader_name) + @parent = parent_loader + end + + # Has parent if one was set when constructed + def parent + @parent + end + + def find(typed_name) + if @parent.nil? + nil + else + @parent.find(typed_name) + end + end + + def load_typed(typed_name) + if @parent.nil? + nil + else + @parent.load_typed(typed_name) + end + end + + def loaded_entry(typed_name, check_dependencies = false) + if @parent.nil? + nil + else + @parent.loaded_entry(typed_name, check_dependencies) + end + end + + # Has no entries on its own - always nil + def get_entry(typed_name) + nil + end + + # Finds nothing, there are no entries + def find(name) + nil + end + + # Cannot store anything + def set_entry(typed_name, value, origin = nil) + nil + end + + def to_s() + "(NullLoader '#{loader_name}')" + end + end + # rubocop:enable Lint/UnusedMethodArgument + # rubocop:enable Style/ClassAndModuleChildren + # rubocop:enable Layout/SpaceAroundEqualsInParameterDefault + # rubocop:enable Style/StringLiterals + # rubocop:enable Style/TrivialAccessors + # rubocop:enable Style/DefWithParentheses +end + # While this is not a monkey patch, but a new class, this class is used purely to # enumerate the paths of puppet "things" that aren't already covered as part of the # usual loaders. It is implemented as a null loader as it can't actually _load_ From 074ad0bc52a504106cbf2d3807082d7df4a42ec5 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Fri, 22 Nov 2019 13:56:34 +0800 Subject: [PATCH 2/2] (GH-199) Update stack trace tests for Puppet 6.11.0 The Puppet call stack has changed behaviour for both exceptions and succesful calls. This commit updates the tests for expect different stack traces for exceptions, and updates the generate stack trace method to ignore call sites with invalid line numbers (e.g. 0) --- .../debug_session/hook_handlers.rb | 6 +++++- .../puppet-debugserver/end_to_end_spec.rb | 18 ++++++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/lib/puppet-debugserver/debug_session/hook_handlers.rb b/lib/puppet-debugserver/debug_session/hook_handlers.rb index 565c7a9f..56948d62 100644 --- a/lib/puppet-debugserver/debug_session/hook_handlers.rb +++ b/lib/puppet-debugserver/debug_session/hook_handlers.rb @@ -256,7 +256,11 @@ def process_breakpoint_hook(reason, args) end break_description = break_display_text if break_description.empty? - stack_trace = Puppet::Pops::PuppetStack.stacktrace + # Due to a modification to the way stack traces are treated in Puppet 6.11.0, the stack + # now includes entries for files in Line 0, which doesn't exist. These indicate that a file + # has started to be processed/parsed/compiled. So we just ignore them + # See https://tickets.puppetlabs.com/browse/PUP-10150 for more infomation + stack_trace = Puppet::Pops::PuppetStack.stacktrace.reject { |item| item[1].zero? } # Due to https://github.com/puppetlabs/puppet/commit/0f96dd918b6184261bc2219e5e68e246ffbeac10 # Prior to Puppet 4.8.0, stacktrace is in reverse order stack_trace.reverse! if Gem::Version.new(Puppet.version) < Gem::Version.new('4.8.0') diff --git a/spec/debugserver/integration/puppet-debugserver/end_to_end_spec.rb b/spec/debugserver/integration/puppet-debugserver/end_to_end_spec.rb index 9a7f9dd7..bbf29f3c 100644 --- a/spec/debugserver/integration/puppet-debugserver/end_to_end_spec.rb +++ b/spec/debugserver/integration/puppet-debugserver/end_to_end_spec.rb @@ -32,6 +32,12 @@ @debug_stderr.close } + def modified_puppet_stack_trace + # Due to a modification to the way stack traces are treated in Puppet 6.11.0, the stack size is different + # See https://tickets.puppetlabs.com/browse/PUP-10150 for more infomation + @modified_puppet_stack_trace ||= Gem::Version.create(Puppet.version) >= Gem::Version.create('6.11.0') + end + context 'Processing an empty manifest with no breakpoints' do let(:manifest_file) { File.join($fixtures_dir, 'environments', 'testfixtures', 'manifests', 'empty.pp') } let(:noop) { true } @@ -92,10 +98,14 @@ result = @client.data_from_request_seq_id(@client.current_seq_id) # As we're only in the root, only two frames should be available. The error and where it was called from expect(result['success']).to be true - expect(result['body']['stackFrames'].count).to eq(2) - expect(result['body']['stackFrames'][0]).to include('line' => 3) - expect(result['body']['stackFrames'][1]).to include('line' => 3) - + if modified_puppet_stack_trace + expect(result['body']['stackFrames'].count).to eq(1) + expect(result['body']['stackFrames'][0]).to include('line' => 3) + else + expect(result['body']['stackFrames'].count).to eq(2) + expect(result['body']['stackFrames'][0]).to include('line' => 3) + expect(result['body']['stackFrames'][1]).to include('line' => 3) + end # continue_request @client.send_data(@client.continue_request(@client.next_seq_id, thread_id)) expect(@client).to receive_message_with_request_id_within_timeout([@client.current_seq_id, 5])