Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Methods defined in Rake namespaces are not marked as private #2316

Closed
razielgn opened this Issue Apr 27, 2013 · 3 comments

Comments

Projects
None yet
2 participants
Contributor

razielgn commented Apr 27, 2013

I've been trying to run gitlabhq master against Rubinius head (compiled less than a week ago).
A validator which should not respond to setup, actually responds to setup/0, causing an ArgumentError to be thrown, since one argument is passed through.
This happens on validators after rake tasks are loaded by Rails.

When running this under MRI 1.9.3, methods defined inside rake namespaces go into Object, but are marked private (don't know why exactly).
Rubinius does not mark them private. The triggering method is indeed setup (arity 0) defined in a rake task namespace.

You can find the complete stack trace here.

I'm really not sure about what I've discovered, can anyone shed some light on this?


Info:

rvm 1.19.5 (master)
rubinius 2.0.0.rc1 (1.9.3 c5bc086c yyyy-mm-dd JI) [x86_64-unknown-linux-gnu]
Linux 3.5.0-25-generic #39-Ubuntu SMP Mon Feb 25 18:26:58 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux

Steps to reproduce the problem:

git clone https://github.com/gitlabhq/gitlabhq
cd gitlabhq
bundle install 
cp config/database.yml.postgres config/database.yml
cp config/gitlab.yml.example config/gitlab.yml
bundle exec rake db:setup RAILS_ENV=test
Owner

dbussink commented May 1, 2013

I suspect this to be a slightly different variant of #2089, but is there a reason that gitlab adds this method to Object like this? When looking at the code I don't really see much of advantage of doing it the way it's done now over just defining that code inline, or extract it out to a proper project.

Would gitlab be open to a pull request that changes this?

Contributor

razielgn commented May 1, 2013

I totally agree, that code should be moved.
They do not plan to support anything other than MRI, but I'll try to bring up the issue anyway.

@dbussink dbussink was assigned May 2, 2013

@dbussink dbussink closed this in add25be May 3, 2013

Contributor

razielgn commented May 3, 2013

I confirm the commit fixes the problem I had. That's super awesome. 💚 💚 💚
I'm waiting for travis to update its nightlies, then I have a pull request ready for them.
Thanks!

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