Add initialize_dup and initialize_clone #1910

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
3 participants
@carlosgaldino
Member

carlosgaldino commented Sep 16, 2012

This pull request isn't complete yet.

I opened because I need some feedback on how to change Kernel#dup and Kernel#clone since each method needs to call #initialize_dup and #initialize_clone, respectively.

Kernel#dup and #clone are defined in kernel/alpha.rb and the call to #initialize_dup should only occur on 1.9 mode. My question is, how should I proceed?

Copy the code for #clone and #dup to kernel/common/kernel19.rb and then change it so it will call the appropriate methods? Or is there a better solution?

The same question is valid for other classes that need to add specific behavior to #dup and #clone. Most of them are defined on kernel/bootstrap but on kernel/bootstrap I see that there are files specific to 1.9 mode so the fix should be split the method definition on both imaginary_class18.rb and imaginary_class19.rb and add the correct behavior on each file. Is that right?

Thanks.

P.S.: This will fix #1889.

ruby_version_is "1.9" do
describe "Kernel#initialize_clone" do
- it "needs to be reviewed for spec completeness"
+ it "calls #initialize_copy" do
+ ScratchPad.clear

This comment has been minimized.

Show comment Hide comment
@brixen

brixen Sep 22, 2012

Member

Setup code goes in a before action.

@brixen

brixen Sep 22, 2012

Member

Setup code goes in a before action.

@brixen

This comment has been minimized.

Show comment Hide comment
@brixen

brixen Sep 22, 2012

Member

This post has a discussion of language version-specific changes http://rubini.us/2011/10/18/contributing-to-rubinius/. Generally, you do put the methods in different files. However, we don't want to duplicate code unnecessarily.

In this case, it's a little bit complicated by having those methods defined in alpha.rb. That file has the bare minimum necessary to start loading code. However, we have analogous cases of dealing with this sort of version-specific behavior. When it involves things like the "kind" of objects or critical object reflection capabilities, we use the Rubinius::Type module. I'd suggest doing something like this:

diff --git a/kernel/alpha.rb b/kernel/alpha.rb
index fb59d54..4c2522a 100644
--- a/kernel/alpha.rb
+++ b/kernel/alpha.rb
@@ -206,9 +206,7 @@ module Kernel
 
     Rubinius.invoke_primitive :object_copy_object, copy, self
 
-    Rubinius.privately do
-      copy.initialize_copy self
-    end
+    Rubinius::Type.object_initialize_dup self, copy
     copy
   end
 
@@ -236,9 +234,7 @@ module Kernel
     Rubinius.invoke_primitive :object_copy_object, copy, self
     Rubinius.invoke_primitive :object_copy_singleton_class, copy, self
 
-    Rubinius.privately do
-      copy.initialize_copy self
-    end
+    Rubinius::Type.object_initialize_clone self, copy
 
     copy.freeze if frozen?
     copy
@@ -344,6 +340,20 @@ module Rubinius
       raise PrimitiveFailure, "MethodTable#alias primitive failed"
     end
   end
+
+  module Type
+    def self.object_initialize_dup(obj, copy)
+      Rubinius.privately do
+        copy.initialize_copy obj
+      end
+    end
+
+    def self.object_initialize_clone(obj, copy)
+      Rubinius.privately do
+        copy.initialize_copy obj
+      end
+    end
+  end
 end
 
 
@@ -734,4 +744,3 @@ end
 class Object
   include Kernel
 end
-

Define appropriate versions of Rubinius::Type.object_initialize_dup, etc. in bootstrap/type19.rb and add the appropriate Kernel, etc. methods for 1.9 mode.

Member

brixen commented Sep 22, 2012

This post has a discussion of language version-specific changes http://rubini.us/2011/10/18/contributing-to-rubinius/. Generally, you do put the methods in different files. However, we don't want to duplicate code unnecessarily.

In this case, it's a little bit complicated by having those methods defined in alpha.rb. That file has the bare minimum necessary to start loading code. However, we have analogous cases of dealing with this sort of version-specific behavior. When it involves things like the "kind" of objects or critical object reflection capabilities, we use the Rubinius::Type module. I'd suggest doing something like this:

diff --git a/kernel/alpha.rb b/kernel/alpha.rb
index fb59d54..4c2522a 100644
--- a/kernel/alpha.rb
+++ b/kernel/alpha.rb
@@ -206,9 +206,7 @@ module Kernel
 
     Rubinius.invoke_primitive :object_copy_object, copy, self
 
-    Rubinius.privately do
-      copy.initialize_copy self
-    end
+    Rubinius::Type.object_initialize_dup self, copy
     copy
   end
 
@@ -236,9 +234,7 @@ module Kernel
     Rubinius.invoke_primitive :object_copy_object, copy, self
     Rubinius.invoke_primitive :object_copy_singleton_class, copy, self
 
-    Rubinius.privately do
-      copy.initialize_copy self
-    end
+    Rubinius::Type.object_initialize_clone self, copy
 
     copy.freeze if frozen?
     copy
@@ -344,6 +340,20 @@ module Rubinius
       raise PrimitiveFailure, "MethodTable#alias primitive failed"
     end
   end
+
+  module Type
+    def self.object_initialize_dup(obj, copy)
+      Rubinius.privately do
+        copy.initialize_copy obj
+      end
+    end
+
+    def self.object_initialize_clone(obj, copy)
+      Rubinius.privately do
+        copy.initialize_copy obj
+      end
+    end
+  end
 end
 
 
@@ -734,4 +744,3 @@ end
 class Object
   include Kernel
 end
-

Define appropriate versions of Rubinius::Type.object_initialize_dup, etc. in bootstrap/type19.rb and add the appropriate Kernel, etc. methods for 1.9 mode.

@dbussink

This comment has been minimized.

Show comment Hide comment
@dbussink

dbussink Sep 27, 2012

Member

@carlosgaldino Do these suggestions help you get further with this?

Member

dbussink commented Sep 27, 2012

@carlosgaldino Do these suggestions help you get further with this?

@carlosgaldino

This comment has been minimized.

Show comment Hide comment
@carlosgaldino

carlosgaldino Oct 2, 2012

Member

Unfortunately I got busy last week and couldn't take a look on this.

I intend to take a look this week, though.

Carlos Galdino

On Thursday, September 27, 2012 at 5:21 AM, Dirkjan Bussink wrote:

@carlosgaldino (https://github.com/carlosgaldino) Do these suggestions help you get further with this?


Reply to this email directly or view it on GitHub (#1910 (comment)).

Member

carlosgaldino commented Oct 2, 2012

Unfortunately I got busy last week and couldn't take a look on this.

I intend to take a look this week, though.

Carlos Galdino

On Thursday, September 27, 2012 at 5:21 AM, Dirkjan Bussink wrote:

@carlosgaldino (https://github.com/carlosgaldino) Do these suggestions help you get further with this?


Reply to this email directly or view it on GitHub (#1910 (comment)).

@dbussink

This comment has been minimized.

Show comment Hide comment
@dbussink

dbussink Nov 11, 2012

Member

Since there hasn't been much activity here, I've implemented the feature.

Member

dbussink commented Nov 11, 2012

Since there hasn't been much activity here, I've implemented the feature.

dbussink added a commit that referenced this pull request Nov 11, 2012

@dbussink dbussink closed this Nov 11, 2012

@carlosgaldino

This comment has been minimized.

Show comment Hide comment
@carlosgaldino

carlosgaldino Nov 12, 2012

Member

@dbussink Yeah, I'm sorry. I've been busy lately and didn't have any time.

Member

carlosgaldino commented Nov 12, 2012

@dbussink Yeah, I'm sorry. I've been busy lately and didn't have any time.

brixen added a commit that referenced this pull request Jan 16, 2018

Update rubygems to 2.7.4.
Updating rubygems-update
Successfully installed rubygems-update-2.7.4
Installing RubyGems 2.7.4

=== 2.6.14 / 2017-10-09

Security fixes:

* Whitelist classes and symbols that are in loaded YAML.
  See CVE-2017-0903 for full details.
  Fix by Aaron Patterson.

=== 2.6.13 / 2017-08-27

Security fixes:

* Fix a DNS request hijacking vulnerability. (CVE-2017-0902)
  Discovered by Jonathan Claudius, fix by Samuel Giddins.
* Fix an ANSI escape sequence vulnerability. (CVE-2017-0899)
  Discovered by Yusuke Endoh, fix by Evan Phoenix.
* Fix a DOS vulnerability in the `query` command. (CVE-2017-0900)
  Discovered by Yusuke Endoh, fix by Samuel Giddins.
* Fix a vulnerability in the gem installer that allowed a malicious gem
  to overwrite arbitrary files. (CVE-2017-0901)
  Discovered by Yusuke Endoh, fix by Samuel Giddins.

=== 2.6.12 / 2017-04-30

Bug fixes:

* Fix test_self_find_files_with_gemfile to sort expected files. Pull
  request #1880 by Kazuaki Matsuo.
* Fix issue for MinGW / MSYS2 builds and testing. Pull request #1879 by
  MSP-Greg.
* Fix gem open to open highest version number rather than lowest. Pull
  request #1877 by Tim Pope.
* Add a test for requiring a default spec as installed by the ruby
  installer. Pull request #1899 by Samuel Giddins.
* Fix broken --exact parameter to gem command. Pull request #1873 by Jason
  Frey.
* [Installer] Generate backwards-compatible binstubs. Pull request #1904
  by Samuel Giddins.
* Fix pre-existing source recognition on add action. Pull request #1883 by
  Jonathan Claudius.
* Prevent negative IDs in output of #inspect. Pull request #1908 by Vít
  Ondruch.
* Allow Gem.finish_resolve to respect already-activated specs. Pull
  request #1910 by Samuel Giddins.

=== 2.6.11 / 2017-03-16

Bug fixes:

* Fixed broken tests on ruby-head. Pull request #1841 by
  SHIBATA Hiroshi.
* Update vendored Molinillo to 0.5.7. Pull request #1859 by Samuel
  Giddins.
* Avoid activating Ruby 2.5 default gems when possible. Pull request #1843
  by Samuel Giddins.
* Use improved resolver sorting algorithm. Pull request #1856 by
  Samuel Giddins.

=== 2.6.10 / 2017-01-23

Bug fixes:

* Fix `require` calling the wrong `gem` method when it is overridden.
  Pull request #1822 by Samuel Giddins.

=== 2.6.9 / 2017-01-20

Bug fixes:

* Allow initializing versions with empty strings. Pull request #1767 by
  Luis Sagastume.
* Fix TypeError on 2.4. Pull request #1788 by Nobuyoshi Nakada.
* Don't output mkmf.log message if compilation didn't fail. Pull request
  #1808 by Jeremy Evans.
* Fixed broken links and overzealous URL encoding in gem server. Pull
  request #1809 by Nicole Orchard.
* Update vendored Molinillo to 0.5.5. Pull request #1812 by Samuel
  Giddins.
* RakeBuilder: avoid frozen string issue. Pull request #1819 by Olle
  Jonsson.

=== 2.6.8 / 2016-10-29

Bug fixes:

* Improve SSL verification failure message. Pull request #1751
  by Eric Hodel.
* Ensure `to_spec` falls back on prerelease specs. Pull request
  #1755 by André Arko.
* Update vendored Molinillo to 0.5.3. Pull request #1763 by
  Samuel Giddins.

=== 2.6.7 / 2016-09-26

Bug fixes:

* Install native extensions in the correct location when using the
  `--user-install` flag. Pull request #1683 by Noah Kantrowitz.
* When calling `Gem.sources`, load sources from `configuration`
  if present, else use the default sources. Pull request #1699
  by Luis Sagastume.
* Fail gracefully when attempting to redirect without a Location.
  Pull request #1711 by Samuel Giddins.
* Update vendored Molinillo to 0.5.1. Pull request #1714 by
  Samuel Giddins.

=== 2.6.6 / 2016-06-22

Bug fixes:

* Sort installed versions to make sure we install the latest version when
  running `gem update --system`. As a one-time fix, run
  `gem update --system=2.6.6`. Pull request #1601 by David Radcliffe.

------------------------------------------------------------------------------

RubyGems installed the following executables:
	/source/rubinius/rubinius/bin/gem

RubyGems system software updated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment