Never close $stdout

[quixoten]

This change was causing issues in practice.

---

RFC @abrandoned

[localshred]

👍

[quixoten]

@localshred, I just removed the "close old logger" code all together. There's not a straightforward way to handle this, so we'll leave it up to the user to close old loggers if they're changing them using initialize_logger which should be rare.

My previous attempt was checking the new loggers target, rather than the old one, so it would still happily close $stdout if the new logger was using a file for a target if the previous one used $stdout.

[quixoten]

As far as I can tell, the Logger class doesn't make its target easily accessible.

[localshred]

I wouldn't ever close $stdout, since that's global to the process.

[quixoten]

@localshred, agreed. The issue I'm running into is that I can't get to the old logger's target to make sure it's not $stdout before closing it.

[mmmries]

Just ran into this, couldn't run rake db:migrate because puts would fail. Glad you guys are already on it. 👍

[liveh2o]

That might be a misattribution, @hqmq. I think you meant Scumbag @quixoten :-)

[mmmries]

@abrandoned sorry, I thought it was your PR that introduced the bug, generating new meme... stand by

[localshred]

This image makes me infinitely happy. I think we have a new meme on our hands. 👍 💯 🏆 🎆

[mmmries]

[liveh2o]

Hahahah

[mmmries]

Looks like there is still an issue with this set of changes:

NameError: uninitialized constant Logger
  const_missing at /srv/amigo/shared/bundle/jruby/1.9/gems/protobuf-3.3.1/lib/protobuf/cli.rb:26
            CLI at /srv/amigo/shared/bundle/jruby/1.9/gems/protobuf-3.3.1/lib/protobuf/cli.rb:26
       Protobuf at /srv/amigo/shared/bundle/jruby/1.9/gems/protobuf-3.3.1/lib/protobuf/cli.rb:8
         (root) at /srv/amigo/shared/bundle/jruby/1.9/gems/protobuf-3.3.1/lib/protobuf/cli.rb:7
        require at /srv/amigo/shared/bundle/jruby/1.9/gems/protobuf-3.3.1/bin/rpc_server:1
         (root) at /srv/amigo/shared/bundle/jruby/1.9/gems/protobuf-3.3.1/bin/rpc_server:1
           load at /srv/amigo/shared/bundle/jruby/1.9/bin/rpc_server:23
         (root) at /srv/amigo/shared/bundle/jruby/1.9/bin/rpc_server:23

I think you need to require 'logger' from the standard library in order to use the Logger::INFO constant.

[liveh2o]

Fail.

[liveh2o]

Is that the only issue, @hqmq? If so, we can patch this quickly. Otherwise, we should probably take a quick pass through and double check things.

[mmmries]

@liveh2o that was as far as I got trying to deploy with it, I rolled back to 3.2.1 for now in amigo. I think we should probably take a close look before releasing another patch version (ie. actually try running the code on a real server)