Ruby 1.8.3 breaks Needle's logger?

ruby-talk
1

The problem was a hard coded "Format" constant in the Logger class that
defined the format. This was bad, but not fatal. The
ActiveRecord/Nitro/Needle/Glue, etc, folks made it worse when, instead
of subclassing, they redefined the Logger class and did things like
'remove_const "Format"', among other questionable decisions. I'm not
sure who wrote that code first but it was, unfortunately, copied and
used by several authors.

This issue was brought to the fore in Zed Shaw's rant, "The Chainsaw
Infanticide Logger Manuever" (ruby-talk:153380), which some folks missed
because the mailing list gateway was broken at the time. Anyway, after
that rant, myself and some other folks on IRC did some brainstorming and
came up with various ways to make the logging format more flexible. I
sent an email to NaHi about making a change, and he accepted the notion
in principle. After some friendly back and forth, he settled on his own
solution.

That solution included moving the 'Format' constant under a Formatter
class, and that's why some current code breaks. To call the code
'non-backwards-compatible' is a bit of a stretch, though, since the code
that is broken by this change is of, shall we say, highly questionable
design.

Regards,

Dan

···

-----Original Message-----
From: Yukihiro Matsumoto [mailto:matz@ruby-lang.org]
Sent: Wednesday, October 05, 2005 8:40 AM
To: ruby-talk ML
Subject: Re: Ruby 1.8.3 breaks Needle's logger?

Hi,

In message "Re: Ruby 1.8.3 breaks Needle's logger?" > on Wed, 5 Oct 2005 23:01:48 +0900, "Gavin Sinclair" > <gsinclair@gmail.com> writes:

>A non-backwards-compatible change in a minor release sounds
like a bad
>idea. 1.8.3 has been officially released, yeah? But I'd
like to know
>if this change was made intentionally, and whether such
changes will be
>made in future.

See [ruby-talk:159143]. It's a minor internal change, which
_normally_ other libraries are not suppose to depend.
Unfortunately the author did not know there are several
programs that depends on the internal.

              matz.

2

*Ahem*. For what it's worth, Needle *does* subclass Logger. :wink: And subclassing is no protection against a change in an internal API.

- Jamis

···

On Oct 5, 2005, at 9:14 AM, Berger, Daniel wrote:

The problem was a hard coded "Format" constant in the Logger class that
defined the format. This was bad, but not fatal. The
ActiveRecord/Nitro/Needle/Glue, etc, folks made it worse when, instead
of subclassing, they redefined the Logger class and did things like
'remove_const "Format"', among other questionable decisions. I'm not
sure who wrote that code first but it was, unfortunately, copied and
used by several authors.

3

Jamis Buck wrote:

···

On Oct 5, 2005, at 9:14 AM, Berger, Daniel wrote:

The problem was a hard coded "Format" constant in the Logger class that
defined the format. This was bad, but not fatal. The
ActiveRecord/Nitro/Needle/Glue, etc, folks made it worse when, instead
of subclassing, they redefined the Logger class and did things like
'remove_const "Format"', among other questionable decisions. I'm not
sure who wrote that code first but it was, unfortunately, copied and
used by several authors.

*Ahem*. For what it's worth, Needle *does* subclass Logger. :wink: And subclassing is no protection against a change in an internal API.

- Jamis

Well, in that case, I apologize to you Jamis, as I did not look at the Needle code. However, I have seen snippets of the Glue and ActiveSupport code that were affected because they abused open classes.

See Zed Shaw's rant for some more detail.

Regards,

Dan

4

The *real* problem is that Logger doesn't let us get at that format to change it as needed, right? Can we just patch it to allow that or did the change fix this problem?

James Edward Gray II

···

On Oct 5, 2005, at 10:18 AM, Jamis Buck wrote:

On Oct 5, 2005, at 9:14 AM, Berger, Daniel wrote:

The problem was a hard coded "Format" constant in the Logger class that
defined the format. This was bad, but not fatal. The
ActiveRecord/Nitro/Needle/Glue, etc, folks made it worse when, instead
of subclassing, they redefined the Logger class and did things like
'remove_const "Format"', among other questionable decisions. I'm not
sure who wrote that code first but it was, unfortunately, copied and
used by several authors.

*Ahem*. For what it's worth, Needle *does* subclass Logger. :wink: And subclassing is no protection against a change in an internal API.

5

Jamis Buck wrote:

The problem was a hard coded "Format" constant in the Logger class that
defined the format. This was bad, but not fatal. The
ActiveRecord/Nitro/Needle/Glue, etc, folks made it worse when, instead
of subclassing, they redefined the Logger class and did things like
'remove_const "Format"', among other questionable decisions. I'm not
sure who wrote that code first but it was, unfortunately, copied and
used by several authors.

*Ahem*. For what it's worth, Needle *does* subclass Logger. :wink: And subclassing is no protection against a change in an internal API.

- Jamis

Well, this is darn strange then:

djberge@~/local/src/ruby/needle-1.2.1/test-580>/opt/bin/ruby -v ALL-TESTS.rb
ruby 1.8.3 (2005-05-12) [sparc-solaris2.10]
Loaded suite ALL-TESTS
Started
.........................................................................................................................................................................
Finished in 2.039164 seconds.

169 tests, 350 assertions, 0 failures, 0 errors

Yet, with 1.8.2 I see 4 test failures out of tc_logger.rb (which I just reported on the RubyForge project page).

Also, why "Logger < ::Logger" ?

Regards,

Dan

···

On Oct 5, 2005, at 9:14 AM, Berger, Daniel wrote:

6

James Edward Gray II wrote:

The *real* problem is that Logger doesn't let us get at that format to change it as needed, right? Can we just patch it to allow that or did the change fix this problem?

James Edward Gray II

Yes, that's what was fixed.

Dan

7

Whether you reopen or subclass Logger has little bearing; either way would have broken with 1.8.3. How you would solve this problem (or have avoided it)?

Regards,
jeremy

···

On Oct 5, 2005, at 8:23 AM, Daniel Berger wrote:

Jamis Buck wrote:

On Oct 5, 2005, at 9:14 AM, Berger, Daniel wrote:

The problem was a hard coded "Format" constant in the Logger class that
defined the format. This was bad, but not fatal. The
ActiveRecord/Nitro/Needle/Glue, etc, folks made it worse when, instead
of subclassing, they redefined the Logger class and did things like
'remove_const "Format"', among other questionable decisions. I'm not
sure who wrote that code first but it was, unfortunately, copied and
used by several authors.

*Ahem*. For what it's worth, Needle *does* subclass Logger. :wink: And subclassing is no protection against a change in an internal API.

Well, in that case, I apologize to you Jamis, as I did not look at the Needle code. However, I have seen snippets of the Glue and ActiveSupport code that were affected because they abused open classes.

8

Jamis Buck wrote:

The problem was a hard coded "Format" constant in the Logger class that
defined the format. This was bad, but not fatal. The
ActiveRecord/Nitro/Needle/Glue, etc, folks made it worse when, instead
of subclassing, they redefined the Logger class and did things like
'remove_const "Format"', among other questionable decisions. I'm not
sure who wrote that code first but it was, unfortunately, copied and
used by several authors.

*Ahem*. For what it's worth, Needle *does* subclass Logger. :wink: And subclassing is no protection against a change in an internal API.
- Jamis

Well, this is darn strange then:

djberge@~/local/src/ruby/needle-1.2.1/test-580>/opt/bin/ruby -v ALL-TESTS.rb
ruby 1.8.3 (2005-05-12) [sparc-solaris2.10]
Loaded suite ALL-TESTS
Started
.........................................................................................................................................................................
Finished in 2.039164 seconds.

169 tests, 350 assertions, 0 failures, 0 errors

Yet, with 1.8.2 I see 4 test failures out of tc_logger.rb (which I just reported on the RubyForge project page).

Hmmm:

chuckles:~/Projects/needle/trunk> ruby -v test/ALL-TESTS.rb
ruby 1.8.2 (2004-12-25) [powerpc-darwin8.0.0]
Loaded suite test/ALL-TESTS
Started
...........................................................................................................................................................................
Finished in 0.967888 seconds.

171 tests, 352 assertions, 0 failures, 0 errors

Maybe I need to package another Needle release...but I don't think trunk has anything that hasn't been released yet. I haven't worked on Needle in ages. Could also be a problem with Ruby on solaris... not having a solaris box to test on, there's no way for me to test that.

As for the clean bill of slate on 1.8.3, blame my incomplete coverage of logger, probably. I haven't yet tried 1.8.3 (being a rails-for-my-meat-and-potatoes guy). However, unless things are miraculously watching over Needle's code, I suspect the logs it generates will be malformed...but that's just a guess based on what I've seen of the changes in 1.8.3's logger.

Also, why "Logger < ::Logger" ?

Ah, but don't leave out the context:

   module Needle
     class Logger << ::Logger
     end
   end

Needle::Logger is a subclass of the global ::Logger.

- Jamis

Regards,

Dan

Cheers,

Jamis

···

On Oct 5, 2005, at 2:27 PM, Daniel Berger wrote:

On Oct 5, 2005, at 9:14 AM, Berger, Daniel wrote:

9

[...]

Whether you reopen or subclass Logger has little bearing; either
way would have broken with 1.8.3. How you would solve this problem
(or have avoided it)?

It's Free Software. One possibility is to take the code, fix it,
put it in a custom namespace, e.g. Needle::Logger and distribute
it with your project or as a new package.

Then, when some time in the future the standard logger fulfills
your needs, just delegate from Needle::Logger to Logger.

···

On Wednesday 05 October 2005 18:43, Jeremy Kemper wrote:

--
Stefan

10

I think reimplementing Logger is a far more questionable design decision.

Regards,
jeremy

···

On Oct 5, 2005, at 10:09 AM, Stefan Lang wrote:

On Wednesday 05 October 2005 18:43, Jeremy Kemper wrote:
[...]

Whether you reopen or subclass Logger has little bearing; either
way would have broken with 1.8.3. How you would solve this problem
(or have avoided it)?

It's Free Software. One possibility is to take the code, fix it,
put it in a custom namespace, e.g. Needle::Logger and distribute
it with your project or as a new package.

Then, when some time in the future the standard logger fulfills
your needs, just delegate from Needle::Logger to Logger.