Skip to content

Add deprecation shim for the logging-gem logger API - #360

Merged
pcai merged 7 commits into
mainfrom
compat/logger-deprecation-shim
Sep 10, 2026
Merged

pcai merged 7 commits into
mainfrom
compat/logger-deprecation-shim

Conversation

@pcai

@pcai pcai commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

#358 replaced the unmaintained logging gem with stdlib Logger, which is a breaking change for anyone calling logging-specific methods like add_appenders on Connection#logger.

This keeps it non-breaking:

  • New WinRM::CompatLogger < ::Logger used as the default Connection#logger. It is a real stdlib Logger, so is_a?(Logger) checks and the whole standard API behave exactly as before.
  • add_appenders is still accepted but emits a one-time-per-instance deprecation warning and has no effect (stdlib loggers write to their log device; appenders have no equivalent).
  • conn.logger= injection and WINRM_LOG are untouched.
  • Changelog entry softened and README documents the migration path.
  • New tests/spec/compat_logger_spec.rb covers the shim.

With this in place the pending release no longer needs a major version bump for the logger change.

@pcai
pcai merged commit 6b002ff into main Sep 10, 2026
32 of 33 checks passed
@pcai
pcai deleted the compat/logger-deprecation-shim branch September 10, 2026 16:22
tas50 added a commit to tas50/chef-winrm that referenced this pull request Sep 15, 2026
The logging gem was the only reason this library carried a logging
dependency at all, and it was used for three things: reading a level out
of WINRM_LOG, building a logger for Connection, and building a second,
unrelated logger inside HttpTransport. Ruby's standard library covers all
three, so drop the dependency.

WinRM.default_log_level now parses WINRM_LOG, warning and falling back to
:warn on an unrecognized value rather than relying on the logging gem to
raise ArgumentError. The transport takes its logger from the connection
via options[:logger] instead of constructing its own, so a logger a
caller assigns to Connection#logger now reaches the transport too, which
it previously did not.

Connection#logger returns WinRM::CompatLogger, a stdlib Logger subclass
that still accepts add_appenders. It is the one logging-specific call
consumers actually used; it now emits a deprecation warning once per
instance and does nothing, so code written against the old API keeps
running instead of raising NoMethodError on upgrade.

Ported from WinRb#358 and WinRb#360.

Signed-off-by: Tim Smith <tim@mondoo.com>
tas50 added a commit to tas50/chef-winrm that referenced this pull request Sep 15, 2026
The logging gem was the only reason this library carried a logging
dependency at all, and it was used for three things: reading a level out
of WINRM_LOG, building a logger for Connection, and building a second,
unrelated logger inside HttpTransport. Ruby's standard library covers all
three, so drop the dependency.

WinRM.default_log_level now parses WINRM_LOG, warning and falling back to
:warn on an unrecognized value rather than relying on the logging gem to
raise ArgumentError. The transport takes its logger from the connection
via options[:logger] instead of constructing its own, so a logger a
caller assigns to Connection#logger now reaches the transport too, which
it previously did not.

Connection#logger returns WinRM::CompatLogger, a stdlib Logger subclass
that still accepts add_appenders. It is the one logging-specific call
consumers actually used; it now emits a deprecation warning once per
instance and does nothing, so code written against the old API keeps
running instead of raising NoMethodError on upgrade.

Ported from WinRb#358 and WinRb#360.

Signed-off-by: Tim Smith <tim@mondoo.com>
tpowell-progress pushed a commit to chef/chef-winrm that referenced this pull request Sep 18, 2026
* Fix the dead AppVeyor link in the certificate auth instructions

The README pointed at WinrmAppveyor.psm1 on the WinRb/WinRM master branch
for an example of minting a client-auth certificate with openssl. That
link is doubly dead: the branch is now main, and the file itself is gone
from both repos, deleted here in #33 and upstream in WinRb#364.
Name openssl directly instead.

Signed-off-by: Tim Smith <tim@mondoo.com>

* Rename changelog.md to the canonical CHANGELOG.md

GitHub, RubyGems and most tooling look for the uppercase spelling and
will not surface a lowercase changelog.md. Rename it and update the two
references in the copilot instructions.

Renamed via an intermediate filename so the change is recorded correctly
on case-insensitive filesystems.

Ported from WinRb/WinRM 2827ae1.

Signed-off-by: Tim Smith <tim@mondoo.com>

* Run the unit workflow on pushes to main, not master

The unit workflow's push trigger still named master, a branch this
repository does not have. Unit tests therefore ran on pull requests but
never on merge, so main had no post-merge verification. The lint
workflow already triggers on main; this brings unit into line.

Ported from WinRb/WinRM 915d688.

Signed-off-by: Tim Smith <tim@mondoo.com>

* Close orphaned shells via at_exit when finalizers cannot spawn threads

Ruby 3.1 forbids allocating a thread inside a finalizer, so the original
`Thread.new { close_shell }` finalizer raised ThreadError during garbage
collection. That was worked around by making the finalizer a no-op, which
stopped the crash but leaked the remote shell: an unclosed shell survived
on the WinRM server until its idle timeout expired, consuming one of the
server's limited shell slots in the meantime.

Attempt the thread as before, and when the interpreter refuses with
"can't alloc thread", register an at_exit block that closes the shell at
process exit instead. Unrecognized ThreadErrors still propagate, and a
failure inside the deferred close warns rather than raising, since an
exception from at_exit would obscure the program's real exit status.

Adds coverage for all four paths, which the no-op version had none of.

Ported from WinRb#354.

Signed-off-by: Tim Smith <tim@mondoo.com>

* Replace the logging gem with the Ruby standard library Logger

The logging gem was the only reason this library carried a logging
dependency at all, and it was used for three things: reading a level out
of WINRM_LOG, building a logger for Connection, and building a second,
unrelated logger inside HttpTransport. Ruby's standard library covers all
three, so drop the dependency.

WinRM.default_log_level now parses WINRM_LOG, warning and falling back to
:warn on an unrecognized value rather than relying on the logging gem to
raise ArgumentError. The transport takes its logger from the connection
via options[:logger] instead of constructing its own, so a logger a
caller assigns to Connection#logger now reaches the transport too, which
it previously did not.

Connection#logger returns WinRM::CompatLogger, a stdlib Logger subclass
that still accepts add_appenders. It is the one logging-specific call
consumers actually used; it now emits a deprecation warning once per
instance and does nothing, so code written against the old API keeps
running instead of raising NoMethodError on upgrade.

Ported from WinRb#358 and WinRb#360.

Signed-off-by: Tim Smith <tim@mondoo.com>

* Exclude the broken rubyntlm 0.6.7

rubyntlm 0.6.7 extracted the session key derivation methods into
Net::NTLM::Client::SessionCrypto but left CLIENT_TO_SERVER_SEALING and the
other three key constants behind in Net::NTLM::Client::Session. Ruby
resolves a constant referenced in a method body through the lexical scope
where the method was written and that scope's ancestors, not through the
ancestors of the class that later includes the module, so Session is never
searched. Every call to client_seal_key, server_seal_key, client_sign_key
and server_sign_key raises NameError.

The negotiate transport reaches those methods through seal_message and
unseal_message, so with 0.6.7 installed every NTLM and Negotiate
connection fails. Negotiate is the default transport, so this breaks
essentially all usage. 0.6.5 and 0.6.6 are unaffected, and the dependency
now excludes only 0.6.7 so a fixed 0.6.8 is picked up automatically.

Nothing in the unit suite touched that code path, so the breakage showed
up only in the integration suite, which needs a live Windows host. Add a
contract spec covering all four derivations; it fails in seconds against
0.6.7 and passes against 0.6.5 and 0.6.6.

Signed-off-by: Tim Smith <tim@mondoo.com>

---------

Signed-off-by: Tim Smith <tim@mondoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant