Closed
Conversation
This test was accidentally checking that the scheduled task was still in place in the Windows scheduled task list. Because the Win32::TaskScheduler#delete call seems to be asynchronous, as long as the test for scheduled task happened fast enough, it was still seen as present and the test 'passed'. In fact, puppet resource was scheduling the task to be removed, and it does get removed. This test began to fail when the vms slowed down enough that the broken verification step began to fail because it was no longer executing fast enough to see the transient task prior to its deletion completing. This change corrects the verification step to verify by asserting that the schtasks.exe list command failed (did not show the deleted task). And it wraps it in a timeout to allow it a few seconds for the scheduled tasks state to finalize.
The documentation of the each function used deprecated syntax.
…eduled-task-delete-acceptance (maint) Fix acceptance test for scheduled_task deletion
It seems that calling String#encode('UTF-16LE') sometimes adds only a single
NULL terminator in both ruby 1.9 and 2.0 on Windows. However, a wide string
is supposed to have two NULL terminators.
For reasons unknown, versions prior to 2012 seem to accept wide strings with
only a single NULL terminator, but in 2012 and 2012R2 the string is often times
contains a non-zero high byte, which causes Windows to read past the single
terminator, and attempt to access a path that does not exist.
Puppet::Util::Windows::File.replace_file now calls FFI to replace files in place on Windows. Puppet::Util::Windows::File.wide_string has now been moved to Puppet::Util::Windows::String.wide_string. This moves wide_string to lib/puppet/ubilt/windows/string.rb, adds specs concerning different encodings. In addition, we've removed Windows::API::WideString usages from everywhere it was found as it is known to cause issues with Windows 2012. Puppet::Util::Windows::Process.lookup_privilege_value in particular was changed to use LookupPrivilegeValueA (ANSI) as we control the privilege name being passed to it. Paired with Josh Cooper <josh@puppetlabs.com>
This commit simplifies the process for appending a NULL terminator to the wide string.
* pr/2261: (PUP-1389) Simplify logic for appending extra NULL (PUP-1389) ReplaceFile to FFI / move wide_string (PUP-1389) Ensure wide strings have double null terminators
Previously, we were appending a NULL to the wide encoded string to work around
a ruby bug fixed in 2.1.
However, we were calling `String#strip`, which makes a copy of the string, and
in the process only preserves the first NULL.
This commit embeds a NULL within the string itself, e.g. "bob\0". Since the
NULL is part of the string data, ruby will preserve it.
Also, `String#+` cannot combine strings with different encodings,
e.g. "a".encode('UTF-16LE') + "\0" results in:
incompatible character encodings: UTF-16LE and US-ASCII
which is why we need to encode the terminator. The actual message depends on
the default encoding, e.g. US-ASCII, UTF-8.
|
CLA signed by all contributors. |
Without this patch we're getting the following error on Ruby 2.1.0:
`can't modify frozen Symbol (Puppet::Error)`. This problem is caused by
our monkey patching which attempts to cache Symbol#to_proc return values
instead of creating new instances on every call.
As it turns out, the caching optimization is not necessary on Ruby 1.9.3
and later because Symbol#to_proc implements its own caching system in
these versions.
Here is the full error this patch avoids by not monkey patching
Symbol#to_proc on versions of Ruby prior to 1.9.3.
bundle exec rspec -fd spec/integration/agent/logging_spec.rb
/workspace/serious/src/puppet/lib/puppet/util/monkey_patches.rb:79:in `to_proc': Could not autoload puppet/type/component: can't modify frozen Symbol (Puppet::Error)
from /workspace/serious/src/puppet/lib/puppet/transaction/event.rb:15:in `<class:Event>'
from /workspace/serious/src/puppet/lib/puppet/transaction/event.rb:8:in `<top (required)>'
from /workspace/serious/src/puppet/lib/puppet/transaction.rb:13:in `require'
from /workspace/serious/src/puppet/lib/puppet/transaction.rb:13:in `<class:Transaction>'
from /workspace/serious/src/puppet/lib/puppet/transaction.rb:11:in `<top (required)>'
from /workspace/serious/src/puppet/lib/puppet/type/component.rb:4:in `require'
from /workspace/serious/src/puppet/lib/puppet/type/component.rb:4:in `<top (required)>'
from /workspace/serious/src/puppet/lib/puppet/util/autoload.rb:61:in `load'
from /workspace/serious/src/puppet/lib/puppet/util/autoload.rb:61:in `load_file'
from /workspace/serious/src/puppet/lib/puppet/util/autoload.rb:202:in `load'
from /workspace/serious/src/puppet/lib/puppet/metatype/manager.rb:159:in `type'
from /workspace/serious/src/puppet/lib/puppet/type.rb:2466:in `<top (required)>'
from /workspace/serious/src/puppet/lib/puppet.rb:175:in `require'
from /workspace/serious/src/puppet/lib/puppet.rb:175:in `<top (required)>'
from /workspace/serious/src/puppet/spec/spec_helper.rb:16:in `require'
from /workspace/serious/src/puppet/spec/spec_helper.rb:16:in `<top (required)>'
from /workspace/serious/src/puppet/spec/integration/agent/logging_spec.rb:2:in `require'
from /workspace/serious/src/puppet/spec/integration/agent/logging_spec.rb:2:in `<top (required)>'
from /workspace/serious/src/puppet/.bundle/gems/ruby/2.1.0/gems/rspec-core-2.11.1/lib/rspec/core/configuration.rb:780:in `load'
from /workspace/serious/src/puppet/.bundle/gems/ruby/2.1.0/gems/rspec-core-2.11.1/lib/rspec/core/configuration.rb:780:in `block in load_spec_files'
from /workspace/serious/src/puppet/.bundle/gems/ruby/2.1.0/gems/rspec-core-2.11.1/lib/rspec/core/configuration.rb:780:in `map'
from /workspace/serious/src/puppet/.bundle/gems/ruby/2.1.0/gems/rspec-core-2.11.1/lib/rspec/core/configuration.rb:780:in `load_spec_files'
from /workspace/serious/src/puppet/.bundle/gems/ruby/2.1.0/gems/rspec-core-2.11.1/lib/rspec/core/command_line.rb:22:in `run'
from /workspace/serious/src/puppet/.bundle/gems/ruby/2.1.0/gems/rspec-core-2.11.1/lib/rspec/core/runner.rb:69:in `run'
from /workspace/serious/src/puppet/.bundle/gems/ruby/2.1.0/gems/rspec-core-2.11.1/lib/rspec/core/runner.rb:8:in `block in autorun'
Ruby 2.1.0 includes RDoc 4.1.0 which added a requirement that subclasses of ClassModule implement 'add_prefix'. This requirement was added in this commit: ruby/rdoc@1701151 The rdoc docs for other classes mention that aref_prefix should be implemented by subclasses, but generally a base class implementation is provided. The rdoc doc for ClassModule hasn't been updated to reflect the new requirement. This patch simply implements the 'aref_prefix' method.
The TagSet class didn't include FormatSupport and didn't have a to_data_hash method, so it couldn't be serialized to msgpack.
Contributor
Author
|
reopened against stable: #2287 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
9 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The TagSet class didn't include FormatSupport and didn't have a
to_data_hash method, so it couldn't be serialized to msgpack.