Hi, today I''ve finally started fixing some failing specs in the core libraries of IronRuby (well actually I spent most of the time reviewing the smaller changes between MRI 1.8 and 1.9 and reading some parts of the source code of IronRuby since it''s been quite some time since the last time I contributed). I still have to push my changes on a remote repository but in the meantime you can find them in the diff attached to this email. 304459b Implement various fixes to Kernel and Array related to the trust status of Object instances. e77abb5 Fix: Kernel#taint and Kernel#untaint raise an exception when trying to modify the taint status of a frozen object. 3f0760b Fix: Array#pop raises a RuntimeError on a frozen array. 9e4defb Implement Array#pop(n). 1ba0628 Fix: Object#=~ returns nil matching any object. 19e8250 Update the unsigned version of App.config to pick the right library paths for 1.9. A few question / notes: - If nothing has changed since the last time, I guess you can''t still merge changes coming from the community that don''t apply to parts of IronRuby outside Libraries.LCA_RESTRICTED, right? - I noticed that the contents of .gitignore have been changed in the master branch and now it contains only one line. I wonder if this change is intentional, now I get a lot of garbage (e.g. the output of the compilation) in the list of unstaged changes. - Many specs of Array, Hash and String fail because MRI 1.9 now raises a RuntimeError instead of a TypeError when trying to modify a frozen object. The change in http://github.com/ironruby/ironruby/blob/d0fcc0a132c1ca4f07a11369293d1c5801a6a0d3/Languages/Ruby/Ruby/Runtime/RubyExceptions.cs#L49 is trivial though. - The checks I added when modiying taintness or trustiness on frozen objects should be moved inside RubyContext, respectively to SetObjectTaint() SetObjectTrustiness(). - It would be nice to have a TrustObjectBy<T>(RubyContext, Object) method in RubyContext. - If you are wondering why I haven''t implemented the last three items myself, see my first question :-) Thanks, Daniele -- Daniele Alessandri http://clorophilla.net/ http://twitter.com/JoL1hAHN -------------- next part -------------- A non-text attachment was scrubbed... Name: corelib19_20100820A.diff Type: application/octet-stream Size: 28721 bytes Desc: not available URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20100820/3cf5beeb/attachment-0001.obj>
Thanks for the patch! Jim, could you look at that? [Inline] -----Original Message----- From: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at rubyforge.org] On Behalf Of Daniele Alessandri Sent: Friday, August 20, 2010 12:00 PM To: ironruby-core at rubyforge.org Subject: [Ironruby-core] Review: miscellaneous fixes Hi, today I''ve finally started fixing some failing specs in the core libraries of IronRuby (well actually I spent most of the time reviewing the smaller changes between MRI 1.8 and 1.9 and reading some parts of the source code of IronRuby since it''s been quite some time since the last time I contributed). I still have to push my changes on a remote repository but in the meantime you can find them in the diff attached to this email. 304459b Implement various fixes to Kernel and Array related to the trust status of Object instances. [Better fix would be: return stream.String.TaintBy(format); This would take care of these two failures as well: fails:Array#pack returns a tainted string when the format is tainted fails:Array#pack returns a tainted string when the format is tainted even if the given format is empty Note that context.TaintObjectBy should only be used on objects that are not statically typed to a type that implements IRubyObjectState. MutableString does. ] e77abb5 Fix: Kernel#taint and Kernel#untaint raise an exception when trying to modify the taint status of a frozen object. [This is better done internally. See your question below ?] 3f0760b Fix: Array#pop raises a RuntimeError on a frozen array. 9e4defb Implement Array#pop(n). 1ba0628 Fix: Object#=~ returns nil matching any object. 19e8250 Update the unsigned version of App.config to pick the right library paths for 1.9. A few question / notes: - If nothing has changed since the last time, I guess you can''t still merge changes coming from the community that don''t apply to parts of IronRuby outside Libraries.LCA_RESTRICTED, right? [Nothing changed yet.] - I noticed that the contents of .gitignore have been changed in the master branch and now it contains only one line. I wonder if this change is intentional, now I get a lot of garbage (e.g. the output of the compilation) in the list of unstaged changes. [Jim?] - Many specs of Array, Hash and String fail because MRI 1.9 now raises a RuntimeError instead of a TypeError when trying to modify a frozen object. The change in http://github.com/ironruby/ironruby/blob/d0fcc0a132c1ca4f07a11369293d1c5801a6a0d3/Languages/Ruby/Ruby/Runtime/RubyExceptions.cs#L49 is trivial though. [Fixed] - The checks I added when modiying taintness or trustiness on frozen objects should be moved inside RubyContext, respectively to SetObjectTaint() SetObjectTrustiness(). [Fixed as well.] - It would be nice to have a TrustObjectBy<T>(RubyContext, Object) method in RubyContext. [I''m not sure this is needed. Instead I modified TaintObjectBy to copy trustiness as well - it seems that most of the time "trust" and "taint" are both copied. This makes the changes in KernelOps.cs unnecessary (Clone, Trust, Untrust, Taint, Untaint). And also the change in IListOps.Repetition and Compact. ] - If you are wondering why I haven''t implemented the last three items myself, see my first question :-) Thanks, Daniele -- Daniele Alessandri http://clorophilla.net/ http://twitter.com/JoL1hAHN -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20100822/b28b6544/attachment.html>
I''ve already fixed this in my fork ... just reverted the mistake change: http://github.com/jschementi/ironruby/commit/271532b3e9bd25a00153cf1c633c582b5c7101d1 ~Jimmy On Sat, Aug 21, 2010 at 8:43 PM, Tomas Matousek < Tomas.Matousek at microsoft.com> wrote:> Thanks for the patch! > > Jim, could you look at that? > > [Inline] > > -----Original Message----- > From: ironruby-core-bounces at rubyforge.org [ > mailto:ironruby-core-bounces at rubyforge.org<ironruby-core-bounces at rubyforge.org>] > On Behalf Of Daniele Alessandri > Sent: Friday, August 20, 2010 12:00 PM > To: ironruby-core at rubyforge.org > Subject: [Ironruby-core] Review: miscellaneous fixes > > Hi, > today I''ve finally started fixing some failing specs in the core libraries > of IronRuby (well actually I spent most of the time reviewing the smaller > changes between MRI 1.8 and 1.9 and reading some parts of the source code of > IronRuby since it''s been quite some time since the last time I contributed). > I still have to push my changes on a remote repository but in the meantime > you can find them in the diff attached to this email. > > 304459b Implement various fixes to Kernel and Array related to the trust > status of Object instances. > > [Better fix would be: > return stream.String.TaintBy(format); > > This would take care of these two failures as well: > fails:Array#pack returns a tainted string when the format is tainted > fails:Array#pack returns a tainted string when the format is tainted even > if the given format is empty > > Note that context.TaintObjectBy should only be used on objects that are not > statically typed to a type that implements IRubyObjectState. MutableString > does. > ] > > e77abb5 Fix: Kernel#taint and Kernel#untaint raise an exception when trying > to modify the taint status of a frozen object. > > [This is better done internally. See your question below J] > > 3f0760b Fix: Array#pop raises a RuntimeError on a frozen array. > 9e4defb Implement Array#pop(n). > 1ba0628 Fix: Object#=~ returns nil matching any object. > 19e8250 Update the unsigned version of App.config to pick the right library > paths for 1.9. > > A few question / notes: > - If nothing has changed since the last time, I guess you can''t still merge > changes coming from the community that don''t apply to parts of IronRuby > outside Libraries.LCA_RESTRICTED, right? > > [Nothing changed yet.] > > - I noticed that the contents of .gitignore have been changed in the master > branch and now it contains only one line. I wonder if this change is > intentional, now I get a lot of garbage (e.g. the output of the compilation) > in the list of unstaged changes. > > [Jim?] > > - Many specs of Array, Hash and String fail because MRI 1.9 now raises a > RuntimeError instead of a TypeError when trying to modify a frozen object. > The change in > > http://github.com/ironruby/ironruby/blob/d0fcc0a132c1ca4f07a11369293d1c5801a6a0d3/Languages/Ruby/Ruby/Runtime/RubyExceptions.cs#L49 > is trivial though. > > [Fixed] > > - The checks I added when modiying taintness or trustiness on frozen > objects should be moved inside RubyContext, respectively to > SetObjectTaint() SetObjectTrustiness(). > > [Fixed as well.] > > - It would be nice to have a TrustObjectBy<T>(RubyContext, Object) method > in RubyContext. > > [I''m not sure this is needed. Instead I modified TaintObjectBy to copy > trustiness as well - it seems that most of the time "trust" and "taint" are > both copied. > This makes the changes in KernelOps.cs unnecessary (Clone, Trust, Untrust, > Taint, Untaint). And also the change in IListOps.Repetition and Compact. > ] > > - If you are wondering why I haven''t implemented the last three items > myself, see my first question :-) > > Thanks, > Daniele > > -- > Daniele Alessandri > http://clorophilla.net/ > http://twitter.com/JoL1hAHN > > > _______________________________________________ > Ironruby-core mailing list > Ironruby-core at rubyforge.org > http://rubyforge.org/mailman/listinfo/ironruby-core > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/ironruby-core/attachments/20100821/359afb5f/attachment.html>
On Sun, Aug 22, 2010 at 02:43, Tomas Matousek <Tomas.Matousek at microsoft.com> wrote: I have rebased my local branch on top of your latest changes and removed the unneeded commits, now everything is available on my remote repository in a separate topic branch (see http://github.com/nrk/ironruby/tree/corelib19). The three commits "survived" are: 793effd Implement Array#pop(n). 503002f Fix: Object#=~ returns nil matching any object. 7a6275b Update the unsigned version of App.config to pick the right library paths for 1.9. See http://github.com/nrk/ironruby/compare/014e640ed27c28ff97fb...793effd4d75f39f36ff3 There are also new changes in that branch, but I''m going to post a new message with a separate diff for those.> 304459b Implement various fixes to Kernel and Array related to the trust > status of Object instances. > > [Better fix would be: > return stream.String.TaintBy(format);Fix applied... and it actually lead me to notice a bug related to operations on the trust status of MutableString instances (see below):>>> "".untrust=> "">>> "".untrust.untrust(ir):1:in `untrust'': can''t modify frozen object (RuntimeError) from (ir):1>>> "".untrust.trust(ir):1:in `trust'': can''t modify frozen object (RuntimeError) from (ir):1>>> "".trust.untrust=> ""> Note that context.TaintObjectBy should only be used on objects that are not > statically typed to a type that implements IRubyObjectState. MutableString > does.Ah, right!>> - If nothing has changed since the last time, I guess you can''t still merge >> changes coming from the community that don''t apply to parts of IronRuby >> outside Libraries.LCA_RESTRICTED, right?> [Nothing changed yet.]I think I still need to find a decent workflow when working on stuff that can''t be merged back by you because of the current constraints, maybe I''ll just limit myself to do some experiments in a further branch and report back just like I did in my previous mail.> [I''m not sure this is needed. Instead I modified TaintObjectBy to copy > trustiness as well - it seems that most of the time "trust" and "taint" are > both copied. > This makes the changes in KernelOps.cs unnecessary (Clone, Trust, Untrust, > Taint, Untaint). And also the change in IListOps.Repetition and Compact. > ]If trust and taint are both copied then yes, I agree that''s the way to go. I just wasn''t sure about this behaviour so I thought that keeping them separate (with maybe a new method in RubyContext such as CopyFlags or anything along this line) might have been safer. -- Daniele Alessandri http://clorophilla.net/ http://twitter.com/JoL1hAHN
I''ve just merged your changes into the main repo. Sorry for the delay, we had some technical problems with our sync server. Tomas -----Original Message----- From: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at rubyforge.org] On Behalf Of Daniele Alessandri Sent: Sunday, August 22, 2010 2:45 AM To: ironruby-core at rubyforge.org Subject: Re: [Ironruby-core] Review: miscellaneous fixes On Sun, Aug 22, 2010 at 02:43, Tomas Matousek <Tomas.Matousek at microsoft.com> wrote: I have rebased my local branch on top of your latest changes and removed the unneeded commits, now everything is available on my remote repository in a separate topic branch (see http://github.com/nrk/ironruby/tree/corelib19). The three commits "survived" are: 793effd Implement Array#pop(n). 503002f Fix: Object#=~ returns nil matching any object. 7a6275b Update the unsigned version of App.config to pick the right library paths for 1.9. See http://github.com/nrk/ironruby/compare/014e640ed27c28ff97fb...793effd4d75f39f36ff3 There are also new changes in that branch, but I''m going to post a new message with a separate diff for those.> 304459b Implement various fixes to Kernel and Array related to the > trust status of Object instances. > > [Better fix would be: > return stream.String.TaintBy(format);Fix applied... and it actually lead me to notice a bug related to operations on the trust status of MutableString instances (see below):>>> "".untrust=> "">>> "".untrust.untrust(ir):1:in `untrust'': can''t modify frozen object (RuntimeError) from (ir):1>>> "".untrust.trust(ir):1:in `trust'': can''t modify frozen object (RuntimeError) from (ir):1>>> "".trust.untrust=> ""> Note that context.TaintObjectBy should only be used on objects that > are not statically typed to a type that implements IRubyObjectState. > MutableString does.Ah, right!>> - If nothing has changed since the last time, I guess you can''t still >> merge changes coming from the community that don''t apply to parts of >> IronRuby outside Libraries.LCA_RESTRICTED, right?> [Nothing changed yet.]I think I still need to find a decent workflow when working on stuff that can''t be merged back by you because of the current constraints, maybe I''ll just limit myself to do some experiments in a further branch and report back just like I did in my previous mail.> [I''m not sure this is needed. Instead I modified TaintObjectBy to copy > trustiness as well - it seems that most of the time "trust" and > "taint" are both copied. > This makes the changes in KernelOps.cs unnecessary (Clone, Trust, > Untrust, Taint, Untaint). And also the change in IListOps.Repetition and Compact. > ]If trust and taint are both copied then yes, I agree that''s the way to go. I just wasn''t sure about this behaviour so I thought that keeping them separate (with maybe a new method in RubyContext such as CopyFlags or anything along this line) might have been safer. -- Daniele Alessandri http://clorophilla.net/ http://twitter.com/JoL1hAHN _______________________________________________ Ironruby-core mailing list Ironruby-core at rubyforge.org http://rubyforge.org/mailman/listinfo/ironruby-core