Bug #15478: [RFC] erb: remove deprecation warnings from ERB.new

• Status: Closed
• Assignee: k0kubun (Takashi Kokubun)
• Target version: -
• Backport: 2.4: UNKNOWN, 2.5: UNKNOWN, 2.6: UNKNOWN

[normalperson (Eric Wong)]

erb: remove deprecation warnings from ERB.new

There's too much code relying on the old ordering; so any
deprecation or removal of the old way is too annoying
and only serves to make users angry.

[k0kubun (Takashi Kokubun)]

• Status changed from Open to Feedback

Stop showing warnings for legacy_trim_mode and legacy_eoutvar is fine. But why should we stop deprecating nil and 0 for safe_level? If we do so, this would be suddenly broken in Ruby 2.8 or Ruby 3.0. So this change does not make sense for safe migration.

Note that we already showed this deprecation warning on warnlevel == 2 on Ruby 2.6.

[k0kubun (Takashi Kokubun)]

There's too much code relying on the old ordering

Also please provide real-world use cases for this. I saw many projects are rewritten to Ruby 2.6 interface before even Ruby 2.6 is officially released. We should fix such projects instead of removing deprecation warning where it's actually necessary.

[normalperson (Eric Wong)]

[e-mail] wrote:

Stop showing warnings for legacy_trim_mode and legacy_eoutvar is fine.

OK.

But why should we stop deprecating nil and 0 for
safe_level? If we do so, this would be suddenly broken in Ruby
2.8 or Ruby 3.0. So this change does not make sense for safe
migration.

nil and 0 $SAFE is already no-op, already, right? I don't care
about $SAFE, so I would expect people will pass nil or 0 when
they don't care (because they need to pass the other legacy
variables).

Note that we already showed this deprecation warning on warnlevel == 2
on Ruby 2.6.

Yeah, so if $SAFE already warns, I don't think ERB should also need
to warn.

Also please provide real-world use cases for this.

I hit this at:
https://80x24.org/olddoc.git/tree/lib/oldweb.rb?h=v1.5.1#n272

I saw many projects are rewritten to Ruby 2.6 interface before
even Ruby 2.6 is officially released. We should fix such
projects instead of removing deprecation warning where it's
actually necessary.

Most code repositories are NOT public; and we can never know all
or even most of them. By now, I believe deprecating
existing/stable APIs gracefully in scripting languages is
impossible without either making users angry or wasting their
time with churn.

It's much easier to deprecate C-API stuff because only
developers see such warnings.

If anything, such warning should go to syslog (or an
out-of-the-way OS-level logging mechanism). Stderr is too
annoying to ordinary users (not programmers).

[k0kubun (Takashi Kokubun)]

nil and 0 $SAFE is already no-op, already, right? I don't care about $SAFE, so I would expect people will pass nil or 0 when they don't care (because they need to pass the other legacy variables).

Note that we already showed this deprecation warning on warnlevel == 2 on Ruby 2.6.

Yeah, so if $SAFE already warns, I don't think ERB should also need to warn.

I think you misunderstand that we're warning here only for $SAFE deprecation. Once $SAFE is eliminated, why would we leave the second argument for setting safe_level? The purpose of this warning is to eliminate the second argument of ERB.new. Passing nil and 0 will be harmful when the argument does not exist anymore.

Also note that I actually meant that it's fine to combine multiple warnings to one line by "Stop showing warnings for legacy_trim_mode and legacy_eoutvar is fine", and still the third/fourth arguments are expected to go away.

I hit this at:
https://80x24.org/olddoc.git/tree/lib/oldweb.rb?h=v1.5.1#n272

Thanks. Shall I send a patch to fix it?

Most code repositories are NOT public; and we can never know all or even most of them. By now, I believe deprecating existing/stable APIs gracefully in scripting languages is impossible without either making users angry or wasting their time with churn.

I get your point. I actually faced this by myself at my company's private code.

At the same time, it should be only acceptable on a major version upgrade as long as we provide a way to gracefully migrate from old interface to new interface and notify users gradually. Starting from warn level 2 in Ruby 2.6, decreasing it to 1 in Ruby 2.7, and changing the behavior on Ruby 3.0 should be fine in that sense.

If anything, such warning should go to syslog (or an out-of-the-way OS-level logging mechanism). Stderr is too annoying to ordinary users (not programmers).

I agree that non programmers wouldn't see syslog and it would not annoy them. But do you think programmers always see syslog when they execute Ruby? I don't think so because I only see it when something strange happens.

I believe annoying users by suggesting to fix the usage first is far better than immediately breaking it at some point with just ArgumentError which has almost no clue to fix it immediately.

[k0kubun (Takashi Kokubun)]

• Status changed from Feedback to Third Party's Issue
• File olddoc-erb-ruby26.patch olddoc-erb-ruby26.patch added

I don't know how to send a patch to your git repository, so I attached it in this ticket.

I will still welcome your opinion on this thread, but at least I'm going to eliminate the second argument once $SAFE is removed from Ruby itself, and I don't think it's a good idea to avoid warning users in a noticeable way prior to that.

My suggestion to fix the issue on other closed codes would be postponing $SAFE deprecation to Ruby 4 or later in another ticket. If it's changed so, I'm fine to keep the warn level to 2 during Ruby 2.x.

[normalperson (Eric Wong)]

[e-mail] wrote:

Issue #15478 has been updated by k0kubun (Takashi Kokubun).

nil and 0 $SAFE is already no-op, already, right? I don't care about $SAFE, so I would expect people will pass nil or 0 when they don't care (because they need to pass the other legacy variables).

Note that we already showed this deprecation warning on warnlevel == 2 on Ruby 2.6.

Yeah, so if $SAFE already warns, I don't think ERB should also need to warn.

I think you misunderstand that we're warning here only for
$SAFE deprecation. Once $SAFE is eliminated, why would we
leave the second argument for setting safe_level?

Leave it as a placeholder for compatibility.

The purpose of this warning is to eliminate the second argument of
ERB.new. Passing nil and 0 will be harmful when the argument
does not exist anymore.

Why would safe_level=(0|nil) ever be harmful? It's the default
security level and it'll be the same when $SAFE is eliminated.

Warning on safe_level=(1|2|etc) I can understand.

Also note that I actually meant that it's fine to combine
multiple warnings to one line by "Stop showing warnings for
legacy_trim_mode and legacy_eoutvar is fine", and still the
third/fourth arguments are expected to go away.

No, removing any args is a bad idea because it screws
up existing users and my main point.

I understand $SAFE removal since it's fake security
and already broken. But it's OK to ignore 0/nil.

I hit this at:
https://80x24.org/olddoc.git/tree/lib/oldweb.rb?h=v1.5.1#n272

Thanks. Shall I send a patch to fix it?

Thanks, but I already had a dirty patch which
I just pushed out (connection still flaky).
https://80x24.org/olddoc-public/[e-mail]/

Most code repositories are NOT public; and we can never know
all or even most of them. By now, I believe deprecating
existing/stable APIs gracefully in scripting languages is
impossible without either making users angry or wasting
their time with churn.

I get your point. I actually faced this by myself at my
company's private code.

At the same time, it should be only acceptable on a major
version upgrade as long as we provide a way to gracefully
migrate from old interface to new interface and notify users
gradually. Starting from warn level 2 in Ruby 2.6, decreasing
it to 1 in Ruby 2.7, and changing the behavior on Ruby 3.0
should be fine in that sense.

Why change existing behavior at all? We can support both old
and new-style kwargs without much overhead. There needs to
be no breaking changes for most users, no breaking changes
for Ruby 3, none for 4, etc.

And 3 years deprecation is way too fast for users on LTS systems.

If anything, such warning should go to syslog (or an out-of-the-way OS-level logging mechanism). Stderr is too annoying to ordinary users (not programmers).

I agree that non programmers wouldn't see syslog and it would not annoy them. But do you think programmers always see syslog when they execute Ruby? I don't think so because I only see it when something strange happens.

I believe annoying users by suggesting to fix the usage first
is far better than immediately breaking it at some point with
just ArgumentError which has almost no clue to fix it
immediately.

Of course we never break anything immediately. We don't
need to change or break 99% of ERB use cases which never
change $SAFE.

[k0kubun (Takashi Kokubun)]

• Status changed from Third Party's Issue to Assigned

Why change existing behavior at all?

That's because leaving a legacy code might confuse those who read the code and decreases the maintainability. I like to keep implementation simple so that we don't lose the speed of implementing new features. Otherwise Ruby implementation will stop evolving in the future for maintaining unnecessary legacy stuffs. That's why I'm pushing unnecessary things towards removal.

However,

We can support both old and new-style kwargs without much overhead.

this will become even more valid especially when we have real keyword arguments (I forgot the ticket number), even while we would not pass Hash as safe_level.

As my opinion is not such stronger than you, I'll revert my change for decreasing the warn level from 2 to 1 in Ruby 2.7 and make the behavior the same as Ruby 2.6, believing that warn level 2 would not annoy non programmers.

[k0kubun (Takashi Kokubun)]

• Status changed from Assigned to Closed

Applied in changeset trunk|r66631.

---

Revert "erb.rb: print deprecation warning with warn level 1"

This reverts commit b5569b9ab2ad5e0e4a997df7eb73e97ecbacc9dd.

The deprecation is indefinitely postponed.

[Bug #15478]

[k0kubun (Takashi Kokubun)]

for $SAFE deprecation, I partially applied your change in r66632 as well, in addition to reverting my change for Ruby 2.7.

[normalperson (Eric Wong)]

[e-mail] wrote:

Status changed from Third Party's Issue to Assigned

Thanks for fixing.

Why change existing behavior at all?

That's because leaving a legacy code might confuse those who
read the code and decreases the maintainability. I like to
keep implementation simple so that we don't lose the speed of
implementing new features. Otherwise Ruby implementation will
stop evolving in the future for maintaining unnecessary legacy
stuffs. That's why I'm pushing unnecessary things towards
removal.

I don't believe forcing users to change their code and usages
is worth the trouble. As far as maintaintability goes, we have
tests to enforce this behavior.

The burden should be on the few of us, the developers.
Not thousands of users.

These warnings and annoyances gives users an excuse to abandon
Ruby for something new (I saw this often where I used to work).

Fwiw, I have many sh/perl5/awk scripts from decades ago which
still work today without modification. I believe Ruby is a
better language, but I also long for the UI/API stability of
older languages.

I don't know how to send a patch to your git repository, so I attached it in this ticket.

Thanks. I'm not sure if my version check or Method#parameters check
is better than my version or if it's even worth the trouble to push
out either patch+release for a limited-scope project olddoc.

Improving email/public-inbox integration with cgit and other
repository viewers is my goal for 2019. So hopefully I can make
things easier for you and others to figure out in the future
(without relying on centralized or proprietary services)