Bug #17767


`Cloned ENV` inconsistently returns `ENV` or `self`

Added by kachick (Kenichi Kamiya) over 3 years ago. Updated almost 2 years ago.

Target version:
ruby -v:
ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [x86_64-darwin20]



Is this an expected behavior? 😅

$ ruby -v
ruby 3.0.0p0 (2020-12-25 revision 95aff21468) [x86_64-darwin20]
cloned_env = ENV.clone

p ENV.each_key{}.equal?(ENV) #=> true
p cloned_env.each_key{}.equal?(cloned_env) #=> true


err = ENV.fetch('TEST') rescue $!
p err.receiver.equal?(ENV) #=> true
err = cloned_env.fetch('TEST') rescue $!
p err.receiver.equal?(cloned_env) #=> false

p!{ false }.equal?(ENV) #=> true

cloned_env['TEST'] = 'TRUE'
p!{ false }.equal?(cloned_env) #=> false

I guess ruby don't care Cloned ENV objects.
Yes, anyone basically do not write these code.
But the behaviors looks weird to me. 😓

Should block to clone?
Should return ENV?
Should return self?

I think they return self makes consistently behaviors and does not break compatibilities.
So I have created this PR 😅

Updated by jeremyevans0 (Jeremy Evans) over 3 years ago

I don't think it makes sense to have cloned ENV objects. I propose the following behavior:

  • ENV.dup/ENV.clone/ENV.clone(freeze: nil)/ENV.clone(freeze: false) should return ENV, since the same singleton storage is used (the environ table).
  • ENV.clone(freeze: true) should raise TypeError, the same as ENV.freeze.

ENV.dup previously returned a useless instance of Object, since all ENV behavior is defined on a singleton class.

I've submitted a pull request implementing this proposal:

Updated by nobu (Nobuyoshi Nakada) over 3 years ago

jeremyevans0 (Jeremy Evans) wrote in #note-1:

I don't think it makes sense to have cloned ENV objects.

It feels reasonable.

Updated by nobu (Nobuyoshi Nakada) about 3 years ago

Cloned ENV accesses/modifies the process global environment variables, as well as ENV itself, however the codes we found expect these are different.
So I propose ENV.clone and ENV.dup should raise an exception.

Updated by jeremyevans0 (Jeremy Evans) about 3 years ago

At the May 2021 developer meeting, it was decided that ENV.dup should raise and ENV.clone should warn, so I closed my previous pull request and added a pull request for that behavior:

Actions #5

Updated by jeremyevans (Jeremy Evans) about 3 years ago

  • Status changed from Open to Closed

Applied in changeset git|117310bdc00236c0a7676616ce25b5106775dabc.

Make ENV.clone warn and ENV.dup raise

ENV.dup returned a plain Object, since all of ENV's behavior is
defined in ENV's singleton class. So using dup makes no sense.

ENV.clone works and is used in some gems, but it doesn't do what
the user expects, since modifying ENV.clone also modifies ENV.
Add a deprecation warning pointing the user to use ENV.to_h

This also undefines some private initialize* methods in ENV,
since they are not needed.

Fixes [Bug #17767]

Updated by nobu (Nobuyoshi Nakada) about 2 years ago

Now can/should we make ENV.clone an error too?

Updated by Dan0042 (Daniel DeLorme) almost 2 years ago

I realize I'm late, but instead of "pointing the user to use ENV.to_h" it would be more helpful to just have dup/clone be aliases of to_h. It's the obvious behavior that everyone using ENV.clone was expecting in the first place.


Also available in: Atom PDF